From cdc4ed5ae275bea7ec4d930ec3e26530b1eac1f4 Mon Sep 17 00:00:00 2001 From: mpl Date: Fri, 28 Mar 2014 19:45:22 +0100 Subject: [PATCH] camput init: ring/key related fixes 1) Removed exec call to gpg, because it automatically looks in .gnupg/, which we don't use anymore as a default. 2) Now taking into account global --secret-keyring flag. This flag is now in osutil. 3) New or modified funcs in osutil 4) Made sure --gpgkey works too. 5) Cleaned up error messages and hints. Context: http://camlistore.org/issue/364 http://camlistore.org/issue/368 Change-Id: I2e51032ed0597da656db100d72f5588b37308e1a --- cmd/camput/init.go | 117 ++++++++++++++++-------------- pkg/client/config.go | 30 +++----- pkg/jsonsign/keys.go | 5 +- pkg/jsonsign/sign.go | 6 +- pkg/jsonsign/signhandler/sig.go | 2 +- pkg/osutil/paths.go | 62 ++++++++++++++-- server/camlistored/camlistored.go | 2 +- 7 files changed, 139 insertions(+), 85 deletions(-) diff --git a/cmd/camput/init.go b/cmd/camput/init.go index 45a2e5019..462393813 100644 --- a/cmd/camput/init.go +++ b/cmd/camput/init.go @@ -18,14 +18,14 @@ package main import ( "encoding/json" - "errors" "flag" "fmt" "log" "os" - "os/exec" + "path/filepath" "camlistore.org/pkg/blob" + "camlistore.org/pkg/client/android" "camlistore.org/pkg/cmdmain" "camlistore.org/pkg/jsonsign" "camlistore.org/pkg/osutil" @@ -33,16 +33,18 @@ import ( ) type initCmd struct { - newKey bool - gpgkey string - noconfig bool + newKey bool // whether to create a new GPG ring and key. + noconfig bool // whether to generate a client config file. + keyId string // GPG key ID to use. + secretRing string // GPG secret ring file to use. } func init() { cmdmain.RegisterCommand("init", func(flags *flag.FlagSet) cmdmain.CommandRunner { cmd := new(initCmd) - flags.BoolVar(&cmd.newKey, "newkey", false, "Automatically generate a new identity in a new secret ring.") - flags.StringVar(&cmd.gpgkey, "gpgkey", "", "GPG key to use for signing (overrides $GPGKEY environment)") + flags.BoolVar(&cmd.newKey, "newkey", false, + fmt.Sprintf("Automatically generate a new identity in a new secret ring at %s", osutil.DefaultSecretRingFile())) + flags.StringVar(&cmd.keyId, "gpgkey", "", "GPG key ID to use for signing (overrides $GPGKEY environment)") flags.BoolVar(&cmd.noconfig, "noconfig", false, "Stop after creating the public key blob, and do not try and create a config file.") return cmd }) @@ -64,57 +66,57 @@ func (c *initCmd) Examples() []string { } } -// keyId returns the current keyId. It checks, in this order, -// the --gpgkey flag, the GPGKEY env var, and the default -// identity secret ring. -func (c *initCmd) keyId(secRing string) (string, error) { - if k := c.gpgkey; k != "" { - return k, nil +// initSecretRing sets c.secretRing. It tries, in this order, the --secret-keyring flag, +// the CAMLI_SECRET_RING env var, then defaults to the operating system dependent location +// otherwise. +// It returns an error if the file does not exist. +func (c *initCmd) initSecretRing() error { + if secretRing, ok := osutil.ExplicitSecretRingFile(); ok { + c.secretRing = secretRing + } else { + if android.OnAndroid() { + panic("on android, so CAMLI_SECRET_RING should have been defined, or --secret-keyring used.") + } + c.secretRing = osutil.SecretRingFile() + } + if _, err := os.Stat(c.secretRing); err != nil { + hint := "\nA GPG key is required, please use 'camput init --newkey'.\n\nOr if you know what you're doing, you can set the global camput flag --secret-keyring, or the CAMLI_SECRET_RING env var, to use your own GPG ring. And --gpgkey= or GPGKEY to select which key ID to use." + return fmt.Errorf("Could not use secret ring file %v: %v.\n%v", c.secretRing, err, hint) + } + return nil +} + +// initKeyId sets c.keyId. It checks, in this order, the --gpgkey flag, the GPGKEY env var, +// and in the default identity secret ring. +func (c *initCmd) initKeyId() error { + if k := c.keyId; k != "" { + return nil } if k := os.Getenv("GPGKEY"); k != "" { - return k, nil + c.keyId = k + return nil } - k, err := jsonsign.KeyIdFromRing(secRing) + k, err := jsonsign.KeyIdFromRing(c.secretRing) if err != nil { - log.Printf("No suitable gpg key was found in %v: %v", secRing, err) - } else { - if k != "" { - log.Printf("Re-using identity with keyId %q found in file %s", k, secRing) - return k, nil - } + hint := "You can set --gpgkey= or the GPGKEY env var to select which key ID to use.\n" + return fmt.Errorf("No suitable gpg key was found in %v: %v.\n%v", c.secretRing, err, hint) } - - // TODO: run and parse gpg --list-secret-keys and see if there's just one and suggest that? Or show - // a list of them? - return "", errors.New("Initialization requires your public GPG key.\nYou can set --gpgkey= or set $GPGKEY in your environment. Run gpg --list-secret-keys to find their key IDs.\nOr you can create a new secret ring and key with 'camput init --newkey'.") + c.keyId = k + log.Printf("Re-using identity with keyId %q found in file %s", c.keyId, c.secretRing) + return nil } -func (c *initCmd) getPublicKeyArmoredFromFile(secretRingFileName, keyId string) (b []byte, err error) { - entity, err := jsonsign.EntityFromSecring(keyId, secretRingFileName) - if err == nil { - pubArmor, err := jsonsign.ArmoredPublicKey(entity) - if err == nil { - return []byte(pubArmor), nil - } - } - b, err = exec.Command("gpg", "--export", "--armor", keyId).Output() +func (c *initCmd) getPublicKeyArmored() ([]byte, error) { + entity, err := jsonsign.EntityFromSecring(c.keyId, c.secretRing) if err != nil { - return nil, fmt.Errorf("Error running gpg to export public key %q: %v", keyId, err) + return nil, fmt.Errorf("Could not find keyId %v in ring %v: %v", c.keyId, c.secretRing, err) } - if len(b) == 0 { - return nil, fmt.Errorf("gpg export of public key %q was empty.", keyId) - } - return b, nil -} - -func (c *initCmd) getPublicKeyArmored(keyId string) (b []byte, err error) { - file := osutil.IdentitySecretRing() - b, err = c.getPublicKeyArmoredFromFile(file, keyId) + pubArmor, err := jsonsign.ArmoredPublicKey(entity) if err != nil { - return nil, fmt.Errorf("failed to export armored public key ID %q from %v: %v", keyId, file, err) + return nil, fmt.Errorf("failed to export armored public key ID %q from %v: %v", c.keyId, c.secretRing, err) } - return b, nil + return []byte(pubArmor), nil } func (c *initCmd) RunCommand(args []string) error { @@ -122,26 +124,27 @@ func (c *initCmd) RunCommand(args []string) error { return cmdmain.ErrUsage } - if c.newKey && c.gpgkey != "" { + if c.newKey && c.keyId != "" { log.Fatal("--newkey and --gpgkey are mutually exclusive") } - var keyId string var err error - secRing := osutil.IdentitySecretRing() if c.newKey { - keyId, err = jsonsign.GenerateNewSecRing(secRing) + c.secretRing = osutil.DefaultSecretRingFile() + c.keyId, err = jsonsign.GenerateNewSecRing(c.secretRing) if err != nil { return err } } else { - keyId, err = c.keyId(secRing) - if err != nil { + if err := c.initSecretRing(); err != nil { + return err + } + if err := c.initKeyId(); err != nil { return err } } - pubArmor, err := c.getPublicKeyArmored(keyId) + pubArmor, err := c.getPublicKeyArmored() if err != nil { return err } @@ -159,7 +162,9 @@ func (c *initCmd) RunCommand(args []string) error { if err == nil { log.Fatalf("Config file %q already exists; quitting without touching it.", configFilePath) } - + if err := os.MkdirAll(filepath.Dir(configFilePath), 0700); err != nil { + return err + } if f, err := os.OpenFile(configFilePath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0600); err == nil { defer f.Close() m := &clientconfig.Config{ @@ -170,7 +175,7 @@ func (c *initCmd) RunCommand(args []string) error { Auth: "localhost", }, }, - Identity: keyId, + Identity: c.keyId, IgnoredFiles: []string{".DS_Store"}, } @@ -183,6 +188,8 @@ func (c *initCmd) RunCommand(args []string) error { log.Fatalf("Error writing to %q: %v", configFilePath, err) } log.Printf("Wrote %q; modify as necessary.", configFilePath) + } else { + return fmt.Errorf("could not write client config file %v: %v", configFilePath, err) } return nil } diff --git a/pkg/client/config.go b/pkg/client/config.go index 90e65bf18..8a659af70 100644 --- a/pkg/client/config.go +++ b/pkg/client/config.go @@ -36,15 +36,12 @@ import ( "camlistore.org/pkg/types/clientconfig" ) -// These, if set, override the JSON config file +// If set, flagServer overrides the JSON config file // ~/.config/camlistore/client-config.json -// (i.e. osutil.UserClientConfigPath()) "server" and "password" keys. +// (i.e. osutil.UserClientConfigPath()) "server" key. // -// A main binary must call AddFlags to expose these. -var ( - flagServer string - flagSecretRing string -) +// A main binary must call AddFlags to expose it. +var flagServer string func AddFlags() { defaultPath := "/x/y/z/we're/in-a-test" @@ -52,7 +49,7 @@ func AddFlags() { defaultPath = osutil.UserClientConfigPath() } flag.StringVar(&flagServer, "server", "", "Camlistore server prefix. If blank, the default from the \"server\" field of "+defaultPath+" is used. Acceptable forms: https://you.example.com, example.com:1345 (https assumed), or http://you.example.com/alt-root") - flag.StringVar(&flagSecretRing, "secret-keyring", "", "GnuPG secret keyring file to use.") + osutil.AddSecretRingFlag() } // ExplicitServer returns the blobserver given in the flags, if any. @@ -94,7 +91,7 @@ func parseConfig() { config = &clientconfig.Config{ Identity: cfg.OptionalString("identity", ""), - IdentitySecretRing: cfg.OptionalString("identitySecretRing", osutil.IdentitySecretRing()), + IdentitySecretRing: cfg.OptionalString("identitySecretRing", ""), IgnoredFiles: cfg.OptionalList("ignoredFiles"), } serversList := make(map[string]*clientconfig.Server) @@ -150,7 +147,7 @@ func convertToMultiServers(conf jsonconfig.Obj) (jsonconfig.Obj, error) { }, }, "identity": conf.OptionalString("identity", ""), - "identitySecretRing": conf.OptionalString("identitySecretRing", osutil.IdentitySecretRing()), + "identitySecretRing": conf.OptionalString("identitySecretRing", ""), } if ignoredFiles := conf.OptionalList("ignoredFiles"); ignoredFiles != nil { var list []interface{} @@ -330,22 +327,19 @@ func (c *Client) SetupAuthFromString(a string) error { } // SecretRingFile returns the filename to the user's GPG secret ring. -// The value comes from either a command-line flag, the +// The value comes from either the --secret-keyring flag, the // CAMLI_SECRET_RING environment variable, the client config file's // "identitySecretRing" value, or the operating system default location. func (c *Client) SecretRingFile() string { - if flagSecretRing != "" { - return flagSecretRing - } - if e := os.Getenv("CAMLI_SECRET_RING"); e != "" { - return e + if secretRing, ok := osutil.ExplicitSecretRingFile(); ok { + return secretRing } if android.OnAndroid() { - panic("CAMLI_SECRET_RING should have been defined when on android") + panic("on android, so CAMLI_SECRET_RING should have been defined, or --secret-keyring used.") } configOnce.Do(parseConfig) if config.IdentitySecretRing == "" { - return osutil.IdentitySecretRing() + return osutil.SecretRingFile() } return config.IdentitySecretRing } diff --git a/pkg/jsonsign/keys.go b/pkg/jsonsign/keys.go index dc421cf0a..e4629623c 100644 --- a/pkg/jsonsign/keys.go +++ b/pkg/jsonsign/keys.go @@ -95,11 +95,12 @@ func openArmoredPublicKeyFile(reader io.ReadCloser) (*packet.PublicKey, error) { return pk, nil } -// EntityFromSecring returns the openpgp Entity from keyFile that matches keyId. If empty, keyFile defaults to osutil.IdentitySecretRing(). +// EntityFromSecring returns the openpgp Entity from keyFile that matches keyId. +// If empty, keyFile defaults to osutil.SecretRingFile(). func EntityFromSecring(keyId, keyFile string) (*openpgp.Entity, error) { keyId = strings.ToUpper(keyId) if keyFile == "" { - keyFile = osutil.IdentitySecretRing() + keyFile = osutil.SecretRingFile() } secring, err := os.Open(keyFile) if err != nil { diff --git a/pkg/jsonsign/sign.go b/pkg/jsonsign/sign.go index affdec923..aa9dbee0d 100644 --- a/pkg/jsonsign/sign.go +++ b/pkg/jsonsign/sign.go @@ -42,7 +42,7 @@ type FileEntityFetcher struct { } func FlagEntityFetcher() *FileEntityFetcher { - return &FileEntityFetcher{File: osutil.IdentitySecretRing()} + return &FileEntityFetcher{File: osutil.SecretRingFile()} } type CachingEntityFetcher struct { @@ -117,7 +117,7 @@ type SignRequest struct { // SecretKeyringPath is only used if EntityFetcher is nil, // in which case SecretKeyringPath is used if non-empty. - // As a final resort, we default to osutil.IdentitySecretRing(). + // As a final resort, we default to osutil.SecretRingFile(). SecretKeyringPath string } @@ -125,7 +125,7 @@ func (sr *SignRequest) secretRingPath() string { if sr.SecretKeyringPath != "" { return sr.SecretKeyringPath } - return osutil.IdentitySecretRing() + return osutil.SecretRingFile() } func (sr *SignRequest) Sign() (signedJSON string, err error) { diff --git a/pkg/jsonsign/signhandler/sig.go b/pkg/jsonsign/signhandler/sig.go index df4f45c25..3287e81a6 100644 --- a/pkg/jsonsign/signhandler/sig.go +++ b/pkg/jsonsign/signhandler/sig.go @@ -66,7 +66,7 @@ func (h *Handler) secretRingPath() string { if h.secretRing != "" { return h.secretRing } - return osutil.IdentitySecretRing() + return osutil.SecretRingFile() } func init() { diff --git a/pkg/osutil/paths.go b/pkg/osutil/paths.go index 9bc47ec2e..da984ebea 100644 --- a/pkg/osutil/paths.go +++ b/pkg/osutil/paths.go @@ -17,6 +17,7 @@ limitations under the License. package osutil import ( + "flag" "log" "os" "path/filepath" @@ -108,6 +109,10 @@ func CamliConfigDir() string { return p } failInTests() + return camliConfigDir() +} + +func camliConfigDir() string { if runtime.GOOS == "windows" { return filepath.Join(os.Getenv("APPDATA"), "Camlistore") } @@ -125,14 +130,61 @@ func UserClientConfigPath() string { return filepath.Join(CamliConfigDir(), "client-config.json") } -// IdentitySecretRing returns the path to the default GPG -// secret keyring. It is overriden by the CAMLI_SECRET_RING -// environment variable. -func IdentitySecretRing() string { +// If set, flagSecretRing overrides the JSON config file +// ~/.config/camlistore/client-config.json +// (i.e. UserClientConfigPath()) "identitySecretRing" key. +var ( + flagSecretRing string + secretRingFlagAdded bool +) + +func AddSecretRingFlag() { + flag.StringVar(&flagSecretRing, "secret-keyring", "", "GnuPG secret keyring file to use.") + secretRingFlagAdded = true +} + +// ExplicitSecretRingFile returns the path to the user's GPG secret ring +// file and true if it was ever set through the --secret-keyring flag or +// the CAMLI_SECRET_RING var. It returns "", false otherwise. +// Use of this function requires the program to call AddSecretRingFlag, +// and before flag.Parse is called. +func ExplicitSecretRingFile() (string, bool) { + if !secretRingFlagAdded { + panic("proper use of ExplicitSecretRingFile requires exposing flagSecretRing with AddSecretRingFlag") + } + if flagSecretRing != "" { + return flagSecretRing, true + } + if e := os.Getenv("CAMLI_SECRET_RING"); e != "" { + return e, true + } + return "", false +} + +// DefaultSecretRingFile returns the path to the default GPG secret +// keyring. It is not influenced by any flag or CAMLI* env var. +func DefaultSecretRingFile() string { + return filepath.Join(camliConfigDir(), "identity-secring.gpg") +} + +// identitySecretRing returns the path to the default GPG +// secret keyring. It is still affected by CAMLI_CONFIG_DIR. +func identitySecretRing() string { + return filepath.Join(CamliConfigDir(), "identity-secring.gpg") +} + +// SecretRingFile returns the path to the user's GPG secret ring file. +// The value comes from either the --secret-keyring flag (if previously +// registered with AddSecretRingFlag), or the CAMLI_SECRET_RING environment +// variable, or the operating system default location. +func SecretRingFile() string { + if flagSecretRing != "" { + return flagSecretRing + } if e := os.Getenv("CAMLI_SECRET_RING"); e != "" { return e } - return filepath.Join(CamliConfigDir(), "identity-secring.gpg") + return identitySecretRing() } // DefaultTLSCert returns the path to the default TLS certificate diff --git a/server/camlistored/camlistored.go b/server/camlistored/camlistored.go index 583f8a393..5eca127eb 100644 --- a/server/camlistored/camlistored.go +++ b/server/camlistored/camlistored.go @@ -249,7 +249,7 @@ func newDefaultConfigFile(path string) error { } var keyId string - secRing := osutil.IdentitySecretRing() + secRing := osutil.SecretRingFile() _, err := os.Stat(secRing) switch { case err == nil: