From 668d3ea6c72578cdfd162e0ac2eaab0850b39373 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 18 Nov 2021 18:44:58 -0800 Subject: [PATCH] Modify errs.Wrap() with bad request to send messages to users. --- api/rekey.go | 4 ++-- api/revoke.go | 2 +- api/ssh.go | 12 ++++++------ api/sshRekey.go | 4 ++-- api/sshRenew.go | 2 +- api/sshRevoke.go | 2 +- api/utils.go | 4 ++-- authority/provisioner/jwk.go | 2 +- authority/provisioner/sign_ssh_options.go | 3 ++- authority/provisioner/x5c.go | 2 +- authority/ssh.go | 8 ++++---- authority/tls.go | 9 ++++++--- authority/tls_test.go | 2 +- 13 files changed, 30 insertions(+), 26 deletions(-) diff --git a/api/rekey.go b/api/rekey.go index 2b60eabc..b7958844 100644 --- a/api/rekey.go +++ b/api/rekey.go @@ -18,7 +18,7 @@ func (s *RekeyRequest) Validate() error { return errs.BadRequest("missing csr") } if err := s.CsrPEM.CertificateRequest.CheckSignature(); err != nil { - return errs.Wrap(http.StatusBadRequest, err, "invalid csr") + return errs.BadRequestErr(err, "invalid csr") } return nil @@ -33,7 +33,7 @@ func (h *caHandler) Rekey(w http.ResponseWriter, r *http.Request) { var body RekeyRequest if err := ReadJSON(r.Body, &body); err != nil { - WriteError(w, errs.Wrap(http.StatusBadRequest, err, "error reading request body")) + WriteError(w, errs.BadRequestErr(err, "error reading request body")) return } diff --git a/api/revoke.go b/api/revoke.go index f3f47ebb..44d52cb9 100644 --- a/api/revoke.go +++ b/api/revoke.go @@ -49,7 +49,7 @@ func (r *RevokeRequest) Validate() (err error) { func (h *caHandler) Revoke(w http.ResponseWriter, r *http.Request) { var body RevokeRequest if err := ReadJSON(r.Body, &body); err != nil { - WriteError(w, errs.Wrap(http.StatusBadRequest, err, "error reading request body")) + WriteError(w, errs.BadRequestErr(err, "error reading request body")) return } diff --git a/api/ssh.go b/api/ssh.go index 315b3e83..43ee6b98 100644 --- a/api/ssh.go +++ b/api/ssh.go @@ -250,7 +250,7 @@ type SSHBastionResponse struct { func (h *caHandler) SSHSign(w http.ResponseWriter, r *http.Request) { var body SSHSignRequest if err := ReadJSON(r.Body, &body); err != nil { - WriteError(w, errs.Wrap(http.StatusBadRequest, err, "error reading request body")) + WriteError(w, errs.BadRequestErr(err, "error reading request body")) return } @@ -262,7 +262,7 @@ func (h *caHandler) SSHSign(w http.ResponseWriter, r *http.Request) { publicKey, err := ssh.ParsePublicKey(body.PublicKey) if err != nil { - WriteError(w, errs.Wrap(http.StatusBadRequest, err, "error parsing publicKey")) + WriteError(w, errs.BadRequestErr(err, "error parsing publicKey")) return } @@ -270,7 +270,7 @@ func (h *caHandler) SSHSign(w http.ResponseWriter, r *http.Request) { if body.AddUserPublicKey != nil { addUserPublicKey, err = ssh.ParsePublicKey(body.AddUserPublicKey) if err != nil { - WriteError(w, errs.Wrap(http.StatusBadRequest, err, "error parsing addUserPublicKey")) + WriteError(w, errs.BadRequestErr(err, "error parsing addUserPublicKey")) return } } @@ -394,7 +394,7 @@ func (h *caHandler) SSHFederation(w http.ResponseWriter, r *http.Request) { func (h *caHandler) SSHConfig(w http.ResponseWriter, r *http.Request) { var body SSHConfigRequest if err := ReadJSON(r.Body, &body); err != nil { - WriteError(w, errs.Wrap(http.StatusBadRequest, err, "error reading request body")) + WriteError(w, errs.BadRequestErr(err, "error reading request body")) return } if err := body.Validate(); err != nil { @@ -426,7 +426,7 @@ func (h *caHandler) SSHConfig(w http.ResponseWriter, r *http.Request) { func (h *caHandler) SSHCheckHost(w http.ResponseWriter, r *http.Request) { var body SSHCheckPrincipalRequest if err := ReadJSON(r.Body, &body); err != nil { - WriteError(w, errs.Wrap(http.StatusBadRequest, err, "error reading request body")) + WriteError(w, errs.BadRequestErr(err, "error reading request body")) return } if err := body.Validate(); err != nil { @@ -465,7 +465,7 @@ func (h *caHandler) SSHGetHosts(w http.ResponseWriter, r *http.Request) { func (h *caHandler) SSHBastion(w http.ResponseWriter, r *http.Request) { var body SSHBastionRequest if err := ReadJSON(r.Body, &body); err != nil { - WriteError(w, errs.Wrap(http.StatusBadRequest, err, "error reading request body")) + WriteError(w, errs.BadRequestErr(err, "error reading request body")) return } if err := body.Validate(); err != nil { diff --git a/api/sshRekey.go b/api/sshRekey.go index 4e29b043..8d2ba5ee 100644 --- a/api/sshRekey.go +++ b/api/sshRekey.go @@ -39,7 +39,7 @@ type SSHRekeyResponse struct { func (h *caHandler) SSHRekey(w http.ResponseWriter, r *http.Request) { var body SSHRekeyRequest if err := ReadJSON(r.Body, &body); err != nil { - WriteError(w, errs.Wrap(http.StatusBadRequest, err, "error reading request body")) + WriteError(w, errs.BadRequestErr(err, "error reading request body")) return } @@ -51,7 +51,7 @@ func (h *caHandler) SSHRekey(w http.ResponseWriter, r *http.Request) { publicKey, err := ssh.ParsePublicKey(body.PublicKey) if err != nil { - WriteError(w, errs.Wrap(http.StatusBadRequest, err, "error parsing publicKey")) + WriteError(w, errs.BadRequestErr(err, "error parsing publicKey")) return } diff --git a/api/sshRenew.go b/api/sshRenew.go index d28b57b5..5dfd5983 100644 --- a/api/sshRenew.go +++ b/api/sshRenew.go @@ -37,7 +37,7 @@ type SSHRenewResponse struct { func (h *caHandler) SSHRenew(w http.ResponseWriter, r *http.Request) { var body SSHRenewRequest if err := ReadJSON(r.Body, &body); err != nil { - WriteError(w, errs.Wrap(http.StatusBadRequest, err, "error reading request body")) + WriteError(w, errs.BadRequestErr(err, "error reading request body")) return } diff --git a/api/sshRevoke.go b/api/sshRevoke.go index c6ebe99d..cfc25f04 100644 --- a/api/sshRevoke.go +++ b/api/sshRevoke.go @@ -48,7 +48,7 @@ func (r *SSHRevokeRequest) Validate() (err error) { func (h *caHandler) SSHRevoke(w http.ResponseWriter, r *http.Request) { var body SSHRevokeRequest if err := ReadJSON(r.Body, &body); err != nil { - WriteError(w, errs.Wrap(http.StatusBadRequest, err, "error reading request body")) + WriteError(w, errs.BadRequestErr(err, "error reading request body")) return } diff --git a/api/utils.go b/api/utils.go index fa56ed6b..a7f4bf58 100644 --- a/api/utils.go +++ b/api/utils.go @@ -93,7 +93,7 @@ func ProtoJSONStatus(w http.ResponseWriter, m proto.Message, status int) { // pointed by v. func ReadJSON(r io.Reader, v interface{}) error { if err := json.NewDecoder(r).Decode(v); err != nil { - return errs.Wrap(http.StatusBadRequest, err, "error decoding json") + return errs.BadRequestErr(err, "error decoding json") } return nil } @@ -103,7 +103,7 @@ func ReadJSON(r io.Reader, v interface{}) error { func ReadProtoJSON(r io.Reader, m proto.Message) error { data, err := io.ReadAll(r) if err != nil { - return errs.Wrap(http.StatusBadRequest, err, "error reading request body") + return errs.BadRequestErr(err, "error reading request body") } return protojson.Unmarshal(data, m) } diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go index 56768fb7..137915c8 100644 --- a/authority/provisioner/jwk.go +++ b/authority/provisioner/jwk.go @@ -228,7 +228,7 @@ func (p *JWK) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, // Use options in the token. if opts.CertType != "" { if certType, err = sshutil.CertTypeFromString(opts.CertType); err != nil { - return nil, errs.Wrap(http.StatusBadRequest, err, "jwk.AuthorizeSSHSign") + return nil, errs.BadRequestErr(err, err.Error()) } } if opts.KeyID != "" { diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index 878d3d02..78d5dd31 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -9,6 +9,7 @@ import ( "time" "github.com/pkg/errors" + "github.com/smallstep/certificates/errs" "go.step.sm/crypto/keyutil" "golang.org/x/crypto/ssh" ) @@ -55,7 +56,7 @@ type SignSSHOptions struct { // Validate validates the given SignSSHOptions. func (o SignSSHOptions) Validate() error { if o.CertType != "" && o.CertType != SSHUserCert && o.CertType != SSHHostCert { - return errors.Errorf("unknown certType %s", o.CertType) + return errs.BadRequest("unknown certificate type '%s'", o.CertType) } return nil } diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go index a05f39c7..8710acb5 100644 --- a/authority/provisioner/x5c.go +++ b/authority/provisioner/x5c.go @@ -271,7 +271,7 @@ func (p *X5C) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, // Use options in the token. if opts.CertType != "" { if certType, err = sshutil.CertTypeFromString(opts.CertType); err != nil { - return nil, errs.Wrap(http.StatusBadRequest, err, "x5c.AuthorizeSSHSign") + return nil, errs.BadRequestErr(err, err.Error()) } } if opts.KeyID != "" { diff --git a/authority/ssh.go b/authority/ssh.go index 5e03ee9e..9c5405c4 100644 --- a/authority/ssh.go +++ b/authority/ssh.go @@ -151,7 +151,7 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi // Validate given options. if err := opts.Validate(); err != nil { - return nil, errs.Wrap(http.StatusBadRequest, err, "authority.SignSSH") + return nil, err } // Set backdate with the configured value @@ -194,8 +194,8 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi certificate, err := sshutil.NewCertificate(cr, certOptions...) if err != nil { if _, ok := err.(*sshutil.TemplateError); ok { - return nil, errs.NewErr(http.StatusBadRequest, err, - errs.WithMessage(err.Error()), + return nil, errs.ApplyOptions( + errs.BadRequestErr(err, err.Error()), errs.WithKeyVal("signOptions", signOpts), ) } @@ -208,7 +208,7 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi // Use SignSSHOptions to modify the certificate validity. It will be later // checked or set if not defined. if err := opts.ModifyValidity(certTpl); err != nil { - return nil, errs.Wrap(http.StatusBadRequest, err, "authority.SignSSH") + return nil, errs.BadRequestErr(err, err.Error()) } // Use provisioner modifiers. diff --git a/authority/tls.go b/authority/tls.go index 4a5f2fdf..716d8956 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -76,7 +76,10 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Sign opts := []interface{}{errs.WithKeyVal("csr", csr), errs.WithKeyVal("signOptions", signOpts)} if err := csr.CheckSignature(); err != nil { - return nil, errs.Wrap(http.StatusBadRequest, err, "authority.Sign; invalid certificate request", opts...) + return nil, errs.ApplyOptions( + errs.BadRequestErr(err, "invalid certificate request"), + opts..., + ) } // Set backdate with the configured value @@ -114,8 +117,8 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Sign cert, err := x509util.NewCertificate(csr, certOptions...) if err != nil { if _, ok := err.(*x509util.TemplateError); ok { - return nil, errs.NewErr(http.StatusBadRequest, err, - errs.WithMessage(err.Error()), + return nil, errs.ApplyOptions( + errs.BadRequestErr(err, err.Error()), errs.WithKeyVal("csr", csr), errs.WithKeyVal("signOptions", signOpts), ) diff --git a/authority/tls_test.go b/authority/tls_test.go index 409c0582..03beb5c1 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -256,7 +256,7 @@ func TestAuthority_Sign(t *testing.T) { csr: csr, extraOpts: extraOpts, signOpts: signOpts, - err: errors.New("authority.Sign; invalid certificate request"), + err: errors.New("invalid certificate request"), code: http.StatusBadRequest, } },