From 17b17fc8bb690d1f6344e5af1c62b3b37166bc48 Mon Sep 17 00:00:00 2001 From: Niall Sheridan Date: Sun, 15 Jan 2017 21:50:38 +0000 Subject: Add more context to errors --- client/client.go | 19 ++++++++++--------- client/keys.go | 8 +++++--- cmd/cashierd/main.go | 38 +++++++++++++++++++++----------------- server/auth/github/github.go | 2 +- server/auth/github/github_test.go | 8 +++----- server/auth/google/google.go | 2 +- server/auth/google/google_test.go | 7 +++---- server/config/config.go | 22 +++++++++++----------- 8 files changed, 55 insertions(+), 51 deletions(-) diff --git a/client/client.go b/client/client.go index b13c4cb..382c53d 100644 --- a/client/client.go +++ b/client/client.go @@ -11,6 +11,7 @@ import ( "time" "github.com/nsheridan/cashier/lib" + "github.com/pkg/errors" "golang.org/x/crypto/ssh" "golang.org/x/crypto/ssh/agent" ) @@ -27,7 +28,7 @@ func InstallCert(a agent.Agent, cert *ssh.Certificate, key Key) error { LifetimeSecs: uint32(lifetime), } if err := a.Add(pubcert); err != nil { - return fmt.Errorf("error importing certificate: %s", err) + return errors.Wrap(err, "unable to add cert to ssh agent") } privkey := agent.AddedKey{ PrivateKey: key, @@ -35,7 +36,7 @@ func InstallCert(a agent.Agent, cert *ssh.Certificate, key Key) error { LifetimeSecs: uint32(lifetime), } if err := a.Add(privkey); err != nil { - return fmt.Errorf("error importing key: %s", err) + return errors.Wrap(err, "unable to add private key to ssh agent") } return nil } @@ -48,7 +49,7 @@ func send(s []byte, token, ca string, ValidateTLSCertificate bool) (*lib.SignRes client := &http.Client{Transport: transport} u, err := url.Parse(ca) if err != nil { - return nil, err + return nil, errors.Wrap(err, "unable to parse CA url") } u.Path = path.Join(u.Path, "/sign") req, err := http.NewRequest("POST", u.String(), bytes.NewReader(s)) @@ -68,7 +69,7 @@ func send(s []byte, token, ca string, ValidateTLSCertificate bool) (*lib.SignRes defer resp.Body.Close() c := &lib.SignResponse{} if err := json.NewDecoder(resp.Body).Decode(c); err != nil { - return nil, err + return nil, errors.Wrap(err, "unable to decode server response") } return c, nil } @@ -84,22 +85,22 @@ func Sign(pub ssh.PublicKey, token string, conf *Config) (*ssh.Certificate, erro ValidUntil: time.Now().Add(validity), }) if err != nil { - return nil, err + return nil, errors.Wrap(err, "unable to create sign request") } resp, err := send(s, token, conf.CA, conf.ValidateTLSCertificate) if err != nil { - return nil, err + return nil, errors.Wrap(err, "error sending request to CA") } if resp.Status != "ok" { - return nil, fmt.Errorf("error: %s", resp.Response) + return nil, fmt.Errorf("bad response from CA: %s", resp.Response) } k, _, _, _, err := ssh.ParseAuthorizedKey([]byte(resp.Response)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "unable to parse response") } cert, ok := k.(*ssh.Certificate) if !ok { - return nil, fmt.Errorf("did not receive a certificate from server") + return nil, fmt.Errorf("did not receive a valid certificate from server") } return cert, nil } diff --git a/client/keys.go b/client/keys.go index 3d2fb31..73983a8 100644 --- a/client/keys.go +++ b/client/keys.go @@ -8,6 +8,8 @@ import ( "crypto/rsa" "fmt" + "github.com/pkg/errors" + "golang.org/x/crypto/ed25519" "golang.org/x/crypto/ssh" ) @@ -68,7 +70,7 @@ func generateECDSAKey(size int) (Key, error) { case 521: curve = elliptic.P521() default: - return nil, fmt.Errorf("Unsupported key size: %d. Valid sizes are '256', '384', '521'", size) + return nil, fmt.Errorf("Unsupported ECDSA key size: %d. Valid sizes are '256', '384', '521'", size) } return ecdsa.GenerateKey(curve, rand.Reader) } @@ -101,8 +103,8 @@ func GenerateKey(options ...func(*options)) (Key, ssh.PublicKey, error) { privkey, err = generateRSAKey(config.size) } if err != nil { - return nil, nil, err + return nil, nil, errors.Wrapf(err, "unable to generate %s key-pair", config.keytype) } pubkey, err = ssh.NewPublicKey(privkey.Public()) - return privkey, pubkey, err + return privkey, pubkey, errors.Wrap(err, "error parsing public key") } diff --git a/cmd/cashierd/main.go b/cmd/cashierd/main.go index 83627ad..fc03171 100644 --- a/cmd/cashierd/main.go +++ b/cmd/cashierd/main.go @@ -5,7 +5,6 @@ import ( "crypto/tls" "encoding/hex" "encoding/json" - "errors" "flag" "fmt" "html/template" @@ -17,6 +16,8 @@ import ( "strconv" "strings" + "github.com/pkg/errors" + "go4.org/wkfs" "golang.org/x/crypto/acme/autocert" "golang.org/x/oauth2" @@ -125,7 +126,7 @@ func (a *appContext) login(w http.ResponseWriter, r *http.Request) (int, error) } // parseKey retrieves and unmarshals the signing request. -func parseKey(r *http.Request) (*lib.SignRequest, error) { +func extractKey(r *http.Request) (*lib.SignRequest, error) { var s lib.SignRequest if err := json.NewDecoder(r.Body).Decode(&s); err != nil { return nil, err @@ -154,23 +155,25 @@ func signHandler(a *appContext, w http.ResponseWriter, r *http.Request) (int, er } // Sign the pubkey and issue the cert. - req, err := parseKey(r) + req, err := extractKey(r) if err != nil { - return http.StatusInternalServerError, err + return http.StatusBadRequest, errors.Wrap(err, "unable to extract key from request") } username := a.authprovider.Username(token) a.authprovider.Revoke(token) // We don't need this anymore. cert, err := a.sshKeySigner.SignUserKey(req, username) if err != nil { - return http.StatusInternalServerError, err + return http.StatusInternalServerError, errors.Wrap(err, "error signing key") } if err := a.certstore.SetCert(cert); err != nil { log.Printf("Error recording cert: %v", err) } - json.NewEncoder(w).Encode(&lib.SignResponse{ + if err := json.NewEncoder(w).Encode(&lib.SignResponse{ Status: "ok", Response: lib.GetPublicKey(cert), - }) + }); err != nil { + return http.StatusInternalServerError, errors.Wrap(err, "error encoding response") + } return http.StatusOK, nil } @@ -219,7 +222,7 @@ func listRevokedCertsHandler(a *appContext, w http.ResponseWriter, r *http.Reque } rl, err := a.sshKeySigner.GenerateRevocationList(revoked) if err != nil { - return http.StatusInternalServerError, err + return http.StatusInternalServerError, errors.Wrap(err, "unable to generate KRL") } w.Header().Set("Content-Type", "application/octet-stream") w.Write(rl) @@ -258,7 +261,7 @@ func revokeCertHandler(a *appContext, w http.ResponseWriter, r *http.Request) (i r.ParseForm() for _, id := range r.Form["cert_id"] { if err := a.certstore.Revoke(id); err != nil { - return http.StatusInternalServerError, err + return http.StatusInternalServerError, errors.Wrap(err, "unable to revoke") } } http.Redirect(w, r, "/admin/certs", http.StatusSeeOther) @@ -292,7 +295,7 @@ func newState() string { func readConfig(filename string) (*config.Config, error) { f, err := os.Open(filename) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to parse config file") } defer f.Close() return config.ReadConfig(f) @@ -301,11 +304,11 @@ func readConfig(filename string) (*config.Config, error) { func loadCerts(certFile, keyFile string) (tls.Certificate, error) { key, err := wkfs.ReadFile(keyFile) if err != nil { - return tls.Certificate{}, err + return tls.Certificate{}, errors.Wrap(err, "error reading TLS private key") } cert, err := wkfs.ReadFile(certFile) if err != nil { - return tls.Certificate{}, err + return tls.Certificate{}, errors.Wrap(err, "error reading TLS certificate") } return tls.X509KeyPair(cert, key) } @@ -338,14 +341,15 @@ func main() { if conf.Server.HTTPLogFile != "" { logfile, err = os.OpenFile(conf.Server.HTTPLogFile, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0640) if err != nil { - log.Fatal(err) + log.Printf("unable to open %s for writing. logging to stdout", conf.Server.HTTPLogFile) + logfile = os.Stderr } } laddr := fmt.Sprintf("%s:%d", conf.Server.Addr, conf.Server.Port) l, err := net.Listen("tcp", laddr) if err != nil { - log.Fatal(err) + log.Fatal(errors.Wrapf(err, "unable to listen on %s:%d", conf.Server.Addr, conf.Server.Port)) } tlsConfig := &tls.Config{} @@ -364,7 +368,7 @@ func main() { tlsConfig.Certificates = make([]tls.Certificate, 1) tlsConfig.Certificates[0], err = loadCerts(conf.Server.TLSCert, conf.Server.TLSKey) if err != nil { - log.Fatal(err) + log.Fatal(errors.Wrap(err, "unable to create TLS listener")) } } l = tls.NewListener(l, tlsConfig) @@ -373,7 +377,7 @@ func main() { if conf.Server.User != "" { log.Print("Dropping privileges...") if err := drop.DropPrivileges(conf.Server.User); err != nil { - log.Fatal(err) + log.Fatal(errors.Wrap(err, "unable to drop privileges")) } } @@ -388,7 +392,7 @@ func main() { log.Fatalf("Unknown provider %s\n", conf.Auth.Provider) } if err != nil { - log.Fatal(err) + log.Fatal(errors.Wrapf(err, "unable to use provider '%s'", conf.Auth.Provider)) } certstore, err := store.New(conf.Server.Database) diff --git a/server/auth/github/github.go b/server/auth/github/github.go index 24a4bbf..7628526 100644 --- a/server/auth/github/github.go +++ b/server/auth/github/github.go @@ -32,7 +32,7 @@ func New(c *config.Auth) (auth.Provider, error) { uw[u] = true } if c.ProviderOpts["organization"] == "" && len(uw) == 0 { - return nil, errors.New("github_opts organization and the users whitelist must not be both empty") + return nil, errors.New("either GitHub organization or users whitelist must be specified") } return &Config{ config: &oauth2.Config{ diff --git a/server/auth/github/github_test.go b/server/auth/github/github_test.go index c0b26a4..b0c97d2 100644 --- a/server/auth/github/github_test.go +++ b/server/auth/github/github_test.go @@ -29,11 +29,9 @@ func TestNew(t *testing.T) { func TestNewEmptyOrganization(t *testing.T) { organization = "" - a := assert.New(t) - - _, err := newGithub() - a.EqualError(err, "github_opts organization and the users whitelist must not be both empty") - + if _, err := newGithub(); err == nil { + t.Error("creating a provider without an organization set should return an error") + } organization = "exampleorg" } diff --git a/server/auth/google/google.go b/server/auth/google/google.go index 08a4083..643ecfe 100644 --- a/server/auth/google/google.go +++ b/server/auth/google/google.go @@ -34,7 +34,7 @@ func New(c *config.Auth) (auth.Provider, error) { uw[u] = true } if c.ProviderOpts["domain"] == "" && len(uw) == 0 { - return nil, errors.New("google_opts domain and the users whitelist must not be both empty") + return nil, errors.New("either Google Apps domain or users whitelist must be specified") } return &Config{ diff --git a/server/auth/google/google_test.go b/server/auth/google/google_test.go index b80c4bf..781cf6f 100644 --- a/server/auth/google/google_test.go +++ b/server/auth/google/google_test.go @@ -28,12 +28,11 @@ func TestNew(t *testing.T) { } func TestNewWithoutDomain(t *testing.T) { - a := assert.New(t) - domain = "" - _, err := newGoogle() - a.EqualError(err, "google_opts domain and the users whitelist must not be both empty") + if _, err := newGoogle(); err == nil { + t.Error("creating a provider without a domain set should return an error") + } domain = "example.com" } diff --git a/server/config/config.go b/server/config/config.go index 5f3f458..f2598a0 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -1,7 +1,6 @@ package config import ( - "errors" "fmt" "io" "log" @@ -12,6 +11,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/mitchellh/mapstructure" "github.com/nsheridan/cashier/server/helpers/vault" + "github.com/pkg/errors" "github.com/spf13/viper" ) @@ -156,14 +156,14 @@ func setFromVault(c *Config) error { } v, err := vault.NewClient(c.Vault.Address, c.Vault.Token) if err != nil { - return err + return errors.Wrap(err, "vault error") } - var errors error + var errs error get := func(value string) string { if strings.HasPrefix(value, "/vault/") { s, err := v.Read(value) if err != nil { - errors = multierror.Append(errors, err) + errs = multierror.Append(errs, err) } return s } @@ -180,12 +180,12 @@ func setFromVault(c *Config) error { c.AWS.AccessKey = get(c.AWS.AccessKey) c.AWS.SecretKey = get(c.AWS.SecretKey) } - return errors + return errors.Wrap(errs, "errors reading from vault") } // Unmarshal the config into a *Config func decode() (*Config, error) { - var errors error + var errs error config := &Config{} configPieces := map[string]interface{}{ "auth": &config.Auth, @@ -200,21 +200,21 @@ func decode() (*Config, error) { continue } if err := mapstructure.WeakDecode(conf[0], val); err != nil { - errors = multierror.Append(errors, err) + errs = multierror.Append(errs, err) } } - return config, errors + return config, errs } // ReadConfig parses a hcl configuration file into a Config struct. func ReadConfig(r io.Reader) (*Config, error) { viper.SetConfigType("hcl") if err := viper.ReadConfig(r); err != nil { - return nil, err + return nil, errors.Wrap(err, "unable to read config") } config, err := decode() if err != nil { - return nil, err + return nil, errors.Wrap(err, "unable to parse config") } if err := setFromVault(config); err != nil { return nil, err @@ -222,7 +222,7 @@ func ReadConfig(r io.Reader) (*Config, error) { setFromEnvironment(config) convertDatastoreConfig(config) if err := verifyConfig(config); err != nil { - return nil, err + return nil, errors.Wrap(err, "unable to verify config") } return config, nil } -- cgit v1.2.3