diff --git a/authority/tls.go b/authority/tls.go index 3900c446c..cd929b421 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -566,6 +566,68 @@ type RevokeOptions struct { OTT string } +// AdministrativeRevokeOptions contains options for RevokeAdministratively. +type AdministrativeRevokeOptions struct { + Certificate *x509.Certificate + Reason string + ReasonCode int + PassiveOnly bool +} + +// RevokeAdministratively revokes an X.509 certificate without performing +// provisioner authorization. Callers must authorize the operation before +// invoking this method. Revocation is propagated to the configured CAS and +// persistence layers, and a CRL is generated when configured. +func (a *Authority) RevokeAdministratively(_ context.Context, revokeOpts *AdministrativeRevokeOptions) error { + if revokeOpts == nil || revokeOpts.Certificate == nil { + return errs.BadRequest("authority.RevokeAdministratively; certificate is required") + } + + crt := revokeOpts.Certificate + if crt.SerialNumber == nil { + return errs.BadRequest("authority.RevokeAdministratively; certificate serial number is required") + } + serial := crt.SerialNumber.String() + opts := []interface{}{ + errs.WithKeyVal("serialNumber", serial), + errs.WithKeyVal("reasonCode", revokeOpts.ReasonCode), + errs.WithKeyVal("reason", revokeOpts.Reason), + errs.WithKeyVal("passiveOnly", revokeOpts.PassiveOnly), + } + rci := &db.RevokedCertificateInfo{ + Serial: serial, + ReasonCode: revokeOpts.ReasonCode, + Reason: revokeOpts.Reason, + RevokedAt: time.Now().UTC(), + ExpiresAt: crt.NotAfter, + } + var provisionerID string + if certificateDataDB, ok := a.db.(interface { + GetCertificateData(string) (*db.CertificateData, error) + }); ok { + if data, err := certificateDataDB.GetCertificateData(serial); err == nil && data != nil && data.Provisioner != nil { + provisionerID = data.Provisioner.ID + } + } + if provisionerID == "" { + if p, err := a.LoadProvisionerByCertificate(crt); err == nil { + provisionerID = p.GetID() + } + } + if provisionerID != "" { + rci.ProvisionerID = provisionerID + opts = append(opts, errs.WithKeyVal("provisionerID", rci.ProvisionerID)) + } + + return a.revokeX509(&RevokeOptions{ + Serial: serial, + Reason: revokeOpts.Reason, + ReasonCode: revokeOpts.ReasonCode, + PassiveOnly: revokeOpts.PassiveOnly, + Crt: crt, + }, rci, "authority.RevokeAdministratively", opts...) +} + // Revoke revokes a certificate. // // NOTE: Only supports passive revocation - prevent existing certificates from @@ -652,66 +714,70 @@ func (a *Authority) Revoke(ctx context.Context, revokeOpts *RevokeOptions) error opts = append(opts, errs.WithKeyVal("provisionerID", rci.ProvisionerID)) } - failRevoke := func(err error) error { - switch { - case errors.Is(err, db.ErrNotImplemented): - return errs.NotImplemented("authority.Revoke; no persistence layer configured", opts...) - case errors.Is(err, db.ErrAlreadyExists): - return errs.ApplyOptions( - errs.BadRequest("certificate with serial number '%s' is already revoked", rci.Serial), - opts..., - ) - default: - return errs.Wrap(http.StatusInternalServerError, err, "authority.Revoke", opts...) - } - } - if provisioner.MethodFromContext(ctx) == provisioner.SSHRevokeMethod { if err := a.revokeSSH(nil, rci); err != nil { - return failRevoke(err) + return revokeError(err, rci.Serial, "authority.Revoke", opts...) } } else { - // Revoke an X.509 certificate using CAS. If the certificate is not - // provided we will try to read it from the db. If the read fails we - // won't throw an error as it will be responsibility of the CAS - // implementation to require a certificate. - var revokedCert *x509.Certificate - if revokeOpts.Crt != nil { - revokedCert = revokeOpts.Crt - } else if rci.Serial != "" { - revokedCert, _ = a.db.GetCertificate(rci.Serial) - } + return a.revokeX509(revokeOpts, rci, "authority.Revoke", opts...) + } - // CAS operation, note that SoftCAS (default) is a noop. - // The revoke happens when this is stored in the db. - _, err := a.x509CAService.RevokeCertificate(&casapi.RevokeCertificateRequest{ - Certificate: revokedCert, - SerialNumber: rci.Serial, - Reason: rci.Reason, - ReasonCode: rci.ReasonCode, - PassiveOnly: revokeOpts.PassiveOnly, - }) - if err != nil { - return errs.Wrap(http.StatusInternalServerError, err, "authority.Revoke", opts...) - } + return nil +} - // Save as revoked in the Db. - if err := a.revoke(revokedCert, rci); err != nil { - return failRevoke(err) - } +func (a *Authority) revokeX509(revokeOpts *RevokeOptions, rci *db.RevokedCertificateInfo, operation string, opts ...interface{}) error { + // Revoke an X.509 certificate using CAS. If the certificate is not provided, + // try to read it from the database. If the lookup fails, the CAS decides + // whether a certificate is required. + var revokedCert *x509.Certificate + if revokeOpts.Crt != nil { + revokedCert = revokeOpts.Crt + } else if rci.Serial != "" { + revokedCert, _ = a.db.GetCertificate(rci.Serial) + } + + // SoftCAS is a no-op; persistence below records the revocation. + _, err := a.x509CAService.RevokeCertificate(&casapi.RevokeCertificateRequest{ + Certificate: revokedCert, + SerialNumber: rci.Serial, + Reason: rci.Reason, + ReasonCode: rci.ReasonCode, + PassiveOnly: revokeOpts.PassiveOnly, + }) + if err != nil { + return errs.Wrap(http.StatusInternalServerError, err, operation, opts...) + } - // Generate a new CRL so CRL requesters will always get an up-to-date - // CRL whenever they request it. - if a.config.CRL.IsEnabled() && a.config.CRL.GenerateOnRevoke { - if err := a.GenerateCertificateRevocationList(); err != nil { - return errs.Wrap(http.StatusInternalServerError, err, "authority.Revoke", opts...) - } + // Persist the revocation. + if err := a.revoke(revokedCert, rci); err != nil { + return revokeError(err, rci.Serial, operation, opts...) + } + + // Generate a new CRL so CRL requesters will always get an up-to-date + // CRL whenever they request it. + if a.config.CRL.IsEnabled() && a.config.CRL.GenerateOnRevoke { + if err := a.GenerateCertificateRevocationList(); err != nil { + return errs.Wrap(http.StatusInternalServerError, err, operation, opts...) } } return nil } +func revokeError(err error, serial, operation string, opts ...interface{}) error { + switch { + case errors.Is(err, db.ErrNotImplemented): + return errs.NotImplemented(operation+"; no persistence layer configured", opts...) + case errors.Is(err, db.ErrAlreadyExists): + return errs.ApplyOptions( + errs.BadRequest("certificate with serial number '%s' is already revoked", serial), + opts..., + ) + default: + return errs.Wrap(http.StatusInternalServerError, err, operation, opts...) + } +} + func (a *Authority) revoke(crt *x509.Certificate, rci *db.RevokedCertificateInfo) error { if lca, ok := a.adminDB.(interface { Revoke(*x509.Certificate, *db.RevokedCertificateInfo) error diff --git a/authority/tls_test.go b/authority/tls_test.go index 1e660dcb5..5d5cf013d 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -26,7 +26,11 @@ import ( "go.step.sm/crypto/pemutil" "go.step.sm/crypto/x509util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/smallstep/certificates/api/render" + authorityadmin "github.com/smallstep/certificates/authority/admin" "github.com/smallstep/certificates/authority/config" "github.com/smallstep/certificates/authority/policy" "github.com/smallstep/certificates/authority/provisioner" @@ -35,8 +39,6 @@ import ( "github.com/smallstep/certificates/db" "github.com/smallstep/certificates/errs" "github.com/smallstep/nosql/database" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) var ( @@ -1501,6 +1503,157 @@ func TestAuthority_GetTLSOptions(t *testing.T) { } } +type recordingRevokeCAS struct { + notImplementedCAS + request *apiv1.RevokeCertificateRequest + err error +} + +type recordingAdminRevokeDB struct { + *authorityadmin.MockDB + certificate *x509.Certificate + info *db.RevokedCertificateInfo +} + +func (d *recordingAdminRevokeDB) Revoke(crt *x509.Certificate, rci *db.RevokedCertificateInfo) error { + d.certificate = crt + d.info = rci + return nil +} + +func (c *recordingRevokeCAS) RevokeCertificate(req *apiv1.RevokeCertificateRequest) (*apiv1.RevokeCertificateResponse, error) { + c.request = req + if c.err != nil { + return nil, c.err + } + return &apiv1.RevokeCertificateResponse{}, nil +} + +func TestAuthority_RevokeAdministratively(t *testing.T) { + crt, err := pemutil.ReadCertificate("./testdata/certs/foo.crt") + require.NoError(t, err) + + t.Run("ok", func(t *testing.T) { + const provisionerID = "provisioner-id" + cas := new(recordingRevokeCAS) + a := testAuthority(t, + WithDatabase(&db.MockAuthDB{ + MGetCertificateData: func(string) (*db.CertificateData, error) { + return &db.CertificateData{Provisioner: &db.ProvisionerData{ID: provisionerID}}, nil + }, + MRevoke: func(rci *db.RevokedCertificateInfo) error { + return errors.New("unexpected direct database revocation") + }, + }), + WithX509CAService(cas), + ) + adminDB := &recordingAdminRevokeDB{MockDB: new(authorityadmin.MockDB)} + a.adminDB = adminDB + + err := a.RevokeAdministratively(context.Background(), &AdministrativeRevokeOptions{ + Certificate: crt, + Reason: "cessation of operation", + ReasonCode: 5, + PassiveOnly: true, + }) + require.NoError(t, err) + require.NotNil(t, cas.request) + assert.Same(t, crt, cas.request.Certificate) + assert.Equal(t, crt.SerialNumber.String(), cas.request.SerialNumber) + assert.Equal(t, "cessation of operation", cas.request.Reason) + assert.Equal(t, 5, cas.request.ReasonCode) + assert.True(t, cas.request.PassiveOnly) + assert.Same(t, crt, adminDB.certificate) + require.NotNil(t, adminDB.info) + assert.Equal(t, crt.SerialNumber.String(), adminDB.info.Serial) + assert.Equal(t, crt.NotAfter, adminDB.info.ExpiresAt) + assert.Equal(t, "cessation of operation", adminDB.info.Reason) + assert.Equal(t, 5, adminDB.info.ReasonCode) + assert.Equal(t, provisionerID, adminDB.info.ProvisionerID) + }) + + t.Run("local database and CRL", func(t *testing.T) { + var got *db.RevokedCertificateInfo + var crlStore db.CertificateRevocationListInfo + var revokedList []db.RevokedCertificateInfo + liveCertificate := *crt + // CRLs omit expired certificates. + liveCertificate.NotAfter = time.Now().Add(time.Hour) + a := testAuthority(t, + WithDatabase(&db.MockAuthDB{ + MRevoke: func(rci *db.RevokedCertificateInfo) error { + got = rci + revokedList = append(revokedList, *rci) + return nil + }, + MGetCRL: func() (*db.CertificateRevocationListInfo, error) { + return nil, database.ErrNotFound + }, + MStoreCRL: func(info *db.CertificateRevocationListInfo) error { + crlStore = *info + return nil + }, + MGetRevokedCertificates: func() (*[]db.RevokedCertificateInfo, error) { + return &revokedList, nil + }, + }), + ) + a.config.CRL = &config.CRLConfig{Enabled: true, GenerateOnRevoke: true} + + err := a.RevokeAdministratively(context.Background(), &AdministrativeRevokeOptions{ + Certificate: &liveCertificate, + Reason: "cessation of operation", + ReasonCode: 5, + }) + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, liveCertificate.SerialNumber.String(), got.Serial) + crl, err := x509.ParseRevocationList(crlStore.DER) + require.NoError(t, err) + require.Len(t, crl.RevokedCertificateEntries, 1) + assert.Equal(t, liveCertificate.SerialNumber, crl.RevokedCertificateEntries[0].SerialNumber) + assert.Equal(t, 5, crl.RevokedCertificateEntries[0].ReasonCode) + }) + + t.Run("certificate required", func(t *testing.T) { + a := testAuthority(t) + err := a.RevokeAdministratively(context.Background(), nil) + require.Error(t, err) + var sc render.StatusCodedError + require.ErrorAs(t, err, &sc) + assert.Equal(t, http.StatusBadRequest, sc.StatusCode()) + }) + + t.Run("serial number required", func(t *testing.T) { + a := testAuthority(t) + err := a.RevokeAdministratively(context.Background(), &AdministrativeRevokeOptions{ + Certificate: new(x509.Certificate), + }) + require.Error(t, err) + var sc render.StatusCodedError + require.ErrorAs(t, err, &sc) + assert.Equal(t, http.StatusBadRequest, sc.StatusCode()) + }) + + t.Run("CAS failure does not persist", func(t *testing.T) { + persisted := false + cas := &recordingRevokeCAS{err: errors.New("CAS unavailable")} + a := testAuthority(t, + WithDatabase(&db.MockAuthDB{ + MRevoke: func(rci *db.RevokedCertificateInfo) error { + persisted = true + return nil + }, + }), + WithX509CAService(cas), + ) + + err := a.RevokeAdministratively(context.Background(), &AdministrativeRevokeOptions{Certificate: crt}) + require.ErrorContains(t, err, "CAS unavailable") + assert.False(t, persisted) + }) +} + func TestAuthority_Revoke(t *testing.T) { reasonCode := 2 reason := "bob was let go"