From 613c99f00f8fb156e732f80d352c3371da025d55 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Thu, 24 Mar 2022 13:10:49 +0100 Subject: [PATCH] Fix linting issues --- api/utils.go | 8 +++--- authority/admin/api/admin_test.go | 4 +-- authority/admin/api/middleware.go | 11 ------- authority/admin/api/middleware_test.go | 7 +---- authority/admin/api/policy.go | 10 +++---- authority/admin/db/nosql/policy.go | 14 ++++----- authority/policy.go | 25 ++++++++-------- authority/provisioner/aws.go | 5 ++-- authority/provisioner/sign_options.go | 9 +++--- authority/tls.go | 40 -------------------------- policy/engine.go | 26 ----------------- 11 files changed, 37 insertions(+), 122 deletions(-) diff --git a/api/utils.go b/api/utils.go index 67b46aa9..761430ed 100644 --- a/api/utils.go +++ b/api/utils.go @@ -83,13 +83,13 @@ func ReadProtoJSONWithCheck(w http.ResponseWriter, r io.Reader, m proto.Message) Status: http.StatusBadRequest, Message: err.Error(), } - data, err := json.Marshal(wrapper) // TODO(hs): handle err; even though it's very unlikely to fail + errData, err := json.Marshal(wrapper) if err != nil { panic(err) } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusBadRequest) - w.Write(data) + w.Write(errData) return false } if err := protojson.Unmarshal(data, m); err != nil { @@ -99,13 +99,13 @@ func ReadProtoJSONWithCheck(w http.ResponseWriter, r io.Reader, m proto.Message) }{ Message: err.Error(), } - data, err := json.Marshal(wrapper) // TODO(hs): handle err; even though it's very unlikely to fail + errData, err := json.Marshal(wrapper) if err != nil { panic(err) } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusBadRequest) - w.Write(data) + w.Write(errData) return false } diff --git a/authority/admin/api/admin_test.go b/authority/admin/api/admin_test.go index d9592ff2..678cf6a1 100644 --- a/authority/admin/api/admin_test.go +++ b/authority/admin/api/admin_test.go @@ -141,11 +141,11 @@ func (m *mockAdminAuthority) GetAuthorityPolicy(ctx context.Context) (*linkedca. return nil, errors.New("not implemented yet") } -func (m *mockAdminAuthority) CreateAuthorityPolicy(ctx context.Context, admin *linkedca.Admin, policy *linkedca.Policy) (*linkedca.Policy, error) { +func (m *mockAdminAuthority) CreateAuthorityPolicy(ctx context.Context, adm *linkedca.Admin, policy *linkedca.Policy) (*linkedca.Policy, error) { return nil, errors.New("not implemented yet") } -func (m *mockAdminAuthority) UpdateAuthorityPolicy(ctx context.Context, admin *linkedca.Admin, policy *linkedca.Policy) (*linkedca.Policy, error) { +func (m *mockAdminAuthority) UpdateAuthorityPolicy(ctx context.Context, adm *linkedca.Admin, policy *linkedca.Policy) (*linkedca.Policy, error) { return nil, errors.New("not implemented yet") } diff --git a/authority/admin/api/middleware.go b/authority/admin/api/middleware.go index 74bb2234..4ca62bfc 100644 --- a/authority/admin/api/middleware.go +++ b/authority/admin/api/middleware.go @@ -1,7 +1,6 @@ package api import ( - "context" "net/http" "go.step.sm/linkedca" @@ -70,13 +69,3 @@ func (h *Handler) checkAction(next nextHTTP, supportedInStandalone bool) nextHTT next(w, r) } } - -// adminFromContext searches the context for a *linkedca.Admin. -// Returns the admin or an error. -func adminFromContext(ctx context.Context) (*linkedca.Admin, error) { - val, ok := ctx.Value(admin.AdminContextKey).(*linkedca.Admin) - if !ok || val == nil { - return nil, admin.NewError(admin.ErrorBadRequestType, "admin not in context") - } - return val, nil -} diff --git a/authority/admin/api/middleware_test.go b/authority/admin/api/middleware_test.go index ffa319db..158374d0 100644 --- a/authority/admin/api/middleware_test.go +++ b/authority/admin/api/middleware_test.go @@ -169,12 +169,7 @@ func TestHandler_extractAuthorizeTokenAdmin(t *testing.T) { } next := func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - a := ctx.Value(admin.AdminContextKey) // verifying that the context now has a linkedca.Admin - adm, ok := a.(*linkedca.Admin) - if !ok { - t.Errorf("expected *linkedca.Admin; got %T", a) - return - } + adm := linkedca.AdminFromContext(ctx) // verifying that the context now has a linkedca.Admin opts := []cmp.Option{cmpopts.IgnoreUnexported(linkedca.Admin{}, timestamppb.Timestamp{})} if !cmp.Equal(adm, adm, opts...) { t.Errorf("linkedca.Admin diff =\n%s", cmp.Diff(adm, adm, opts...)) diff --git a/authority/admin/api/policy.go b/authority/admin/api/policy.go index 30e05c48..6b59803f 100644 --- a/authority/admin/api/policy.go +++ b/authority/admin/api/policy.go @@ -8,6 +8,7 @@ import ( "go.step.sm/linkedca" "github.com/smallstep/certificates/api" + "github.com/smallstep/certificates/api/read" "github.com/smallstep/certificates/authority/admin" "github.com/smallstep/certificates/authority/provisioner" ) @@ -121,7 +122,7 @@ func (par *PolicyAdminResponder) UpdateAuthorityPolicy(w http.ResponseWriter, r } var newPolicy = new(linkedca.Policy) - if err := api.ReadProtoJSON(r.Body, newPolicy); err != nil { + if err := read.ProtoJSON(r.Body, newPolicy); err != nil { api.WriteError(w, err) return } @@ -220,7 +221,7 @@ func (par *PolicyAdminResponder) CreateProvisionerPolicy(w http.ResponseWriter, } var newPolicy = new(linkedca.Policy) - if err := api.ReadProtoJSON(r.Body, newPolicy); err != nil { + if err := read.ProtoJSON(r.Body, newPolicy); err != nil { api.WriteError(w, err) return } @@ -256,7 +257,7 @@ func (par *PolicyAdminResponder) UpdateProvisionerPolicy(w http.ResponseWriter, } var policy = new(linkedca.Policy) - if err := api.ReadProtoJSON(r.Body, policy); err != nil { + if err := read.ProtoJSON(r.Body, policy); err != nil { api.WriteError(w, err) return } @@ -271,7 +272,7 @@ func (par *PolicyAdminResponder) UpdateProvisionerPolicy(w http.ResponseWriter, api.ProtoJSONStatus(w, policy, http.StatusOK) } -// DeleteProvisionerPolicy ... +// DeleteProvisionerPolicy handles the DELETE /admin/provisioners/{name}/policy request func (par *PolicyAdminResponder) DeleteProvisionerPolicy(w http.ResponseWriter, r *http.Request) { ctx := r.Context() @@ -308,7 +309,6 @@ func (par *PolicyAdminResponder) DeleteProvisionerPolicy(w http.ResponseWriter, api.JSON(w, &DeleteResponse{Status: "ok"}) } -// GetACMEAccountPolicy ... func (par *PolicyAdminResponder) GetACMEAccountPolicy(w http.ResponseWriter, r *http.Request) { api.JSON(w, "ok") } diff --git a/authority/admin/db/nosql/policy.go b/authority/admin/db/nosql/policy.go index 94ff2a0e..8e11ddb0 100644 --- a/authority/admin/db/nosql/policy.go +++ b/authority/admin/db/nosql/policy.go @@ -63,13 +63,13 @@ func (db *DB) getDBAuthorityPolicy(ctx context.Context, authorityID string) (*db return dbap, nil } -func (db *DB) unmarshalAuthorityPolicy(data []byte, authorityID string) (*linkedca.Policy, error) { - dbap, err := db.unmarshalDBAuthorityPolicy(data, authorityID) - if err != nil { - return nil, err - } - return dbap.convert(), nil -} +// func (db *DB) unmarshalAuthorityPolicy(data []byte, authorityID string) (*linkedca.Policy, error) { +// dbap, err := db.unmarshalDBAuthorityPolicy(data, authorityID) +// if err != nil { +// return nil, err +// } +// return dbap.convert(), nil +// } func (db *DB) CreateAuthorityPolicy(ctx context.Context, policy *linkedca.Policy) error { diff --git a/authority/policy.go b/authority/policy.go index ee132f31..88f301e0 100644 --- a/authority/policy.go +++ b/authority/policy.go @@ -17,23 +17,23 @@ func (a *Authority) GetAuthorityPolicy(ctx context.Context) (*linkedca.Policy, e a.adminMutex.Lock() defer a.adminMutex.Unlock() - policy, err := a.adminDB.GetAuthorityPolicy(ctx) + p, err := a.adminDB.GetAuthorityPolicy(ctx) if err != nil { return nil, err } - return policy, nil + return p, nil } -func (a *Authority) CreateAuthorityPolicy(ctx context.Context, adm *linkedca.Admin, policy *linkedca.Policy) (*linkedca.Policy, error) { +func (a *Authority) CreateAuthorityPolicy(ctx context.Context, adm *linkedca.Admin, p *linkedca.Policy) (*linkedca.Policy, error) { a.adminMutex.Lock() defer a.adminMutex.Unlock() - if err := a.checkPolicy(ctx, adm, policy); err != nil { + if err := a.checkPolicy(ctx, adm, p); err != nil { return nil, err } - if err := a.adminDB.CreateAuthorityPolicy(ctx, policy); err != nil { + if err := a.adminDB.CreateAuthorityPolicy(ctx, p); err != nil { return nil, err } @@ -41,18 +41,18 @@ func (a *Authority) CreateAuthorityPolicy(ctx context.Context, adm *linkedca.Adm return nil, admin.WrapErrorISE(err, "error reloading policy engines when creating authority policy") } - return policy, nil // TODO: return the newly stored policy + return p, nil // TODO: return the newly stored policy } -func (a *Authority) UpdateAuthorityPolicy(ctx context.Context, adm *linkedca.Admin, policy *linkedca.Policy) (*linkedca.Policy, error) { +func (a *Authority) UpdateAuthorityPolicy(ctx context.Context, adm *linkedca.Admin, p *linkedca.Policy) (*linkedca.Policy, error) { a.adminMutex.Lock() defer a.adminMutex.Unlock() - if err := a.checkPolicy(ctx, adm, policy); err != nil { + if err := a.checkPolicy(ctx, adm, p); err != nil { return nil, err } - if err := a.adminDB.UpdateAuthorityPolicy(ctx, policy); err != nil { + if err := a.adminDB.UpdateAuthorityPolicy(ctx, p); err != nil { return nil, err } @@ -60,7 +60,7 @@ func (a *Authority) UpdateAuthorityPolicy(ctx context.Context, adm *linkedca.Adm return nil, admin.WrapErrorISE(err, "error reloading policy engines when updating authority policy") } - return policy, nil // TODO: return the updated stored policy + return p, nil // TODO: return the updated stored policy } func (a *Authority) RemoveAuthorityPolicy(ctx context.Context) error { @@ -111,11 +111,10 @@ func isAllowed(engine authPolicy.X509Policy, sans []string) error { ) if allowed, err = engine.AreSANsAllowed(sans); err != nil { var policyErr *policy.NamePolicyError - if errors.As(err, &policyErr); policyErr.Reason == policy.NotAuthorizedForThisName { + if isPolicyErr := errors.As(err, &policyErr); isPolicyErr && policyErr.Reason == policy.NotAuthorizedForThisName { return fmt.Errorf("the provided policy would lock out %s from the CA. Please update your policy to include %s as an allowed name", sans, sans) - } else { - return err } + return err } if !allowed { diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go index 0bbe546b..f8f14671 100644 --- a/authority/provisioner/aws.go +++ b/authority/provisioner/aws.go @@ -266,7 +266,6 @@ type AWS struct { Claims *Claims `json:"claims,omitempty"` Options *Options `json:"options,omitempty"` config *awsConfig - audiences Audiences ctl *Controller x509Policy policy.X509Policy sshHostPolicy policy.HostPolicy @@ -557,7 +556,7 @@ func (p *AWS) readURL(url string) ([]byte, error) { if err != nil { return nil, err } - return nil, fmt.Errorf("Request for metadata returned non-successful status code %d", + return nil, fmt.Errorf("request for metadata returned non-successful status code %d", resp.StatusCode) } @@ -590,7 +589,7 @@ func (p *AWS) readURLv2(url string) (*http.Response, error) { } defer resp.Body.Close() if resp.StatusCode >= 400 { - return nil, fmt.Errorf("Request for API token returned non-successful status code %d", resp.StatusCode) + return nil, fmt.Errorf("request for API token returned non-successful status code %d", resp.StatusCode) } token, err := io.ReadAll(resp.Body) if err != nil { diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go index 2d8a13c3..df2551a3 100644 --- a/authority/provisioner/sign_options.go +++ b/authority/provisioner/sign_options.go @@ -6,7 +6,6 @@ import ( "crypto/rsa" "crypto/x509" "crypto/x509/pkix" - "encoding/asn1" "encoding/json" "net" "net/http" @@ -427,10 +426,10 @@ func (v *x509NamePolicyValidator) Valid(cert *x509.Certificate, _ SignOptions) e return err } -var ( - stepOIDRoot = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 37476, 9000, 64} - stepOIDProvisioner = append(asn1.ObjectIdentifier(nil), append(stepOIDRoot, 1)...) -) +// var ( +// stepOIDRoot = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 37476, 9000, 64} +// stepOIDProvisioner = append(asn1.ObjectIdentifier(nil), append(stepOIDRoot, 1)...) +// ) // type stepProvisionerASN1 struct { // Type int diff --git a/authority/tls.go b/authority/tls.go index 297c796e..df38091c 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -237,14 +237,6 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Sign // the cert to see if the CA is allowed to sign it. func (a *Authority) isAllowedToSign(cert *x509.Certificate) (bool, error) { - // // check if certificate is an admin identity token certificate and the admin subject exists - // b := isAdminIdentityTokenCertificate(cert) - // _ = b - - // if isAdminIdentityTokenCertificate(cert) && a.admins.HasSubject(cert.Subject.CommonName) { - // return true, nil - // } - // if no policy is configured, the cert is implicitly allowed if a.x509Policy == nil { return true, nil @@ -253,38 +245,6 @@ func (a *Authority) isAllowedToSign(cert *x509.Certificate) (bool, error) { return a.x509Policy.AreCertificateNamesAllowed(cert) } -func isAdminIdentityTokenCertificate(cert *x509.Certificate) bool { - - // TODO: remove this check - - if cert.Issuer.CommonName != "" { - return false - } - - subject := cert.Subject.CommonName - if subject == "" { - return false - } - - dnsNames := cert.DNSNames - if len(dnsNames) != 1 { - return false - } - - if dnsNames[0] != subject { - return false - } - - extras := cert.ExtraExtensions - if len(extras) != 1 { - return false - } - - extra := extras[0] - - return extra.Id.Equal(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 37476, 9000, 64, 1}) -} - // Renew creates a new Certificate identical to the old certificate, except // with a validity window that begins 'now'. func (a *Authority) Renew(oldCert *x509.Certificate) ([]*x509.Certificate, error) { diff --git a/policy/engine.go b/policy/engine.go index c37e1f59..63d8452a 100755 --- a/policy/engine.go +++ b/policy/engine.go @@ -4,9 +4,7 @@ import ( "bytes" "crypto/x509" "crypto/x509/pkix" - "errors" "fmt" - "io" "net" "net/url" "reflect" @@ -42,30 +40,6 @@ type NamePolicyError struct { Detail string } -type NameError struct { - error - Reason NamePolicyReason -} - -func a() { - err := io.EOF - var ne *NameError - errors.As(err, ne) - errors.Is(err, ne) -} - -func newPolicyError(reason NamePolicyReason, err error) error { - return &NameError{ - error: err, - Reason: reason, - } -} - -func newPolicyErrorf(reason NamePolicyReason, format string, args ...interface{}) error { - err := fmt.Errorf(format, args...) - return newPolicyError(reason, err) -} - func (e *NamePolicyError) Error() string { switch e.Reason { case NotAuthorizedForThisName: