diff --git a/authority/authority.go b/authority/authority.go index b72fe61e..43216f1e 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -72,8 +72,9 @@ type Authority struct { sshCAHostFederatedCerts []ssh.PublicKey // CRL vars - crlTicker *time.Ticker - crlMutex sync.Mutex + crlTicker *time.Ticker + crlStopper chan struct{} + crlMutex sync.Mutex // Do not re-initialize initOnce bool @@ -662,22 +663,14 @@ func (a *Authority) init() error { // not be repeated. a.initOnce = true - // Start the CRL generator - if a.config.CRL != nil && a.config.CRL.Enabled { - if v := a.config.CRL.CacheDuration; v != nil && v.Duration < 0 { - return errors.New("crl cacheDuration must be >= 0") + // Start the CRL generator, we can assume the configuration is validated. + if a.config.CRL.IsEnabled() { + // Default cache duration to the default one + if v := a.config.CRL.CacheDuration; v == nil || v.Duration <= 0 { + a.config.CRL.CacheDuration = config.DefaultCRLCacheDuration } - - if v := a.config.CRL.CacheDuration; v != nil && v.Duration == 0 { - a.config.CRL.CacheDuration.Duration, _ = time.ParseDuration("24h") - } - - if a.config.CRL.CacheDuration == nil { - a.config.CRL.CacheDuration, _ = provisioner.NewDuration("24h") - } - - err = a.startCRLGenerator() - if err != nil { + // Start CRL generator + if err := a.startCRLGenerator(); err != nil { return err } } @@ -736,6 +729,7 @@ func (a *Authority) IsAdminAPIEnabled() bool { func (a *Authority) Shutdown() error { if a.crlTicker != nil { a.crlTicker.Stop() + close(a.crlStopper) } if err := a.keyManager.Close(); err != nil { @@ -746,9 +740,9 @@ func (a *Authority) Shutdown() error { // CloseForReload closes internal services, to allow a safe reload. func (a *Authority) CloseForReload() { - if a.crlTicker != nil { a.crlTicker.Stop() + close(a.crlStopper) } if err := a.keyManager.Close(); err != nil { @@ -791,27 +785,20 @@ func (a *Authority) requiresSCEPService() bool { return false } -// GetSCEPService returns the configured SCEP Service -// TODO: this function is intended to exist temporarily -// in order to make SCEP work more easily. It can be -// made more correct by using the right interfaces/abstractions -// after it works as expected. +// GetSCEPService returns the configured SCEP Service. +// +// TODO: this function is intended to exist temporarily in order to make SCEP +// work more easily. It can be made more correct by using the right +// interfaces/abstractions after it works as expected. func (a *Authority) GetSCEPService() *scep.Service { return a.scepService } func (a *Authority) startCRLGenerator() error { - - if a.config.CRL.CacheDuration.Duration <= 0 { + if !a.config.CRL.IsEnabled() { return nil } - // Make the renewal ticker run ~2/3 of cacheDuration by default, or use renewPeriod if available - tickerDuration := (a.config.CRL.CacheDuration.Duration / 3) * 2 - if v := a.config.CRL.RenewPeriod; v != nil && v.Duration > 0 { - tickerDuration = v.Duration - } - // Check that there is a valid CRL in the DB right now. If it doesn't exist // or is expired, generate one now _, ok := a.db.(db.CertificateRevocationListDB) @@ -819,37 +806,28 @@ func (a *Authority) startCRLGenerator() error { return errors.Errorf("CRL Generation requested, but database does not support CRL generation") } - // Always create a new CRL on startup in case the CA has been down and the time to next expected CRL - // update is less than the cache duration. - err := a.GenerateCertificateRevocationList() - if err != nil { + // Always create a new CRL on startup in case the CA has been down and the + // time to next expected CRL update is less than the cache duration. + if err := a.GenerateCertificateRevocationList(); err != nil { return errors.Wrap(err, "could not generate a CRL") } - a.crlTicker = time.NewTicker(tickerDuration) + a.crlStopper = make(chan struct{}, 1) + a.crlTicker = time.NewTicker(a.config.CRL.TickerDuration()) go func() { for { - <-a.crlTicker.C - log.Println("Regenerating CRL") - err := a.GenerateCertificateRevocationList() - if err != nil { - log.Printf("ERROR: authority.crlGenerator encountered an error when regenerating the CRL: %v", err) + select { + case <-a.crlTicker.C: + log.Println("Regenerating CRL") + if err := a.GenerateCertificateRevocationList(); err != nil { + log.Printf("error regenerating the CRL: %v", err) + } + case <-a.crlStopper: + return } - } }() return nil } - -func (a *Authority) resetCRLGeneratorTimer() { - if a.crlTicker != nil { - tickerDuration := (a.config.CRL.CacheDuration.Duration / 3) * 2 - if v := a.config.CRL.RenewPeriod; v != nil && v.Duration > 0 { - tickerDuration = v.Duration - } - - a.crlTicker.Reset(tickerDuration) - } -} diff --git a/authority/authority_test.go b/authority/authority_test.go index 48479d5b..9f35f23e 100644 --- a/authority/authority_test.go +++ b/authority/authority_test.go @@ -80,12 +80,6 @@ func testAuthority(t *testing.T, opts ...Option) *Authority { AuthorityConfig: &AuthConfig{ Provisioners: p, }, - CRL: &config.CRLConfig{ - Enabled: true, - CacheDuration: nil, - GenerateOnRevoke: true, - RenewPeriod: nil, - }, } a, err := New(c, opts...) assert.FatalError(t, err) diff --git a/authority/config/config.go b/authority/config/config.go index ad261cdb..79ac3e88 100644 --- a/authority/config/config.go +++ b/authority/config/config.go @@ -37,8 +37,11 @@ var ( DefaultEnableSSHCA = false // DefaultCRLCacheDuration is the default cache duration for the CRL. DefaultCRLCacheDuration = &provisioner.Duration{Duration: 24 * time.Hour} - // GlobalProvisionerClaims default claims for the Authority. Can be overridden - // by provisioner specific claims. + // DefaultCRLExpiredDuration is the default duration in which expired + // certificates will remain in the CRL after expiration. + DefaultCRLExpiredDuration = time.Hour + // GlobalProvisionerClaims is the default duration that expired certificates + // remain in the CRL after expiration. GlobalProvisionerClaims = provisioner.Claims{ MinTLSDur: &provisioner.Duration{Duration: 5 * time.Minute}, // TLS certs MaxTLSDur: &provisioner.Duration{Duration: 24 * time.Hour}, @@ -78,6 +81,55 @@ type Config struct { SkipValidation bool `json:"-"` } +// CRLConfig represents config options for CRL generation +type CRLConfig struct { + Enabled bool `json:"enabled"` + GenerateOnRevoke bool `json:"generateOnRevoke,omitempty"` + CacheDuration *provisioner.Duration `json:"cacheDuration,omitempty"` + RenewPeriod *provisioner.Duration `json:"renewPeriod,omitempty"` +} + +// IsEnabled returns if the CRL is enabled. +func (c *CRLConfig) IsEnabled() bool { + return c != nil && c.Enabled +} + +// Validate validates the CRL configuration. +func (c *CRLConfig) Validate() error { + if c == nil { + return nil + } + + if c.CacheDuration != nil && c.CacheDuration.Duration < 0 { + return errors.New("crl.cacheDuration must be greater than or equal to 0") + } + + if c.RenewPeriod != nil && c.RenewPeriod.Duration < 0 { + return errors.New("crl.renewPeriod must be greater than or equal to 0") + } + + if c.RenewPeriod != nil && c.CacheDuration != nil && + c.RenewPeriod.Duration > c.CacheDuration.Duration { + return errors.New("crl.cacheDuration must be greater than or equal to crl.renewPeriod") + } + + return nil +} + +// TickerDuration the renewal ticker duration. This is set by renewPeriod, of it +// is not set is ~2/3 of cacheDuration. +func (c *CRLConfig) TickerDuration() time.Duration { + if !c.IsEnabled() { + return 0 + } + + if c.RenewPeriod != nil && c.RenewPeriod.Duration > 0 { + return c.RenewPeriod.Duration + } + + return (c.CacheDuration.Duration / 3) * 2 +} + // ASN1DN contains ASN1.DN attributes that are used in Subject and Issuer // x509 Certificate blocks. type ASN1DN struct { @@ -109,14 +161,6 @@ type AuthConfig struct { DisableGetSSHHosts bool `json:"disableGetSSHHosts,omitempty"` } -// CRLConfig represents config options for CRL generation -type CRLConfig struct { - Enabled bool `json:"enabled"` - CacheDuration *provisioner.Duration `json:"cacheDuration,omitempty"` - GenerateOnRevoke bool `json:"generateOnRevoke,omitempty"` - RenewPeriod *provisioner.Duration `json:"renewPeriod,omitempty"` -} - // init initializes the required fields in the AuthConfig if they are not // provided. func (c *AuthConfig) init() { @@ -194,7 +238,7 @@ func (c *Config) Init() { if c.CommonName == "" { c.CommonName = "Step Online CA" } - if c.CRL != nil && c.CRL.CacheDuration == nil { + if c.CRL != nil && c.CRL.Enabled && c.CRL.CacheDuration == nil { c.CRL.CacheDuration = DefaultCRLCacheDuration } c.AuthorityConfig.init() @@ -283,6 +327,11 @@ func (c *Config) Validate() error { return err } + // Validate crl config: nil is ok + if err := c.CRL.Validate(); err != nil { + return err + } + return c.AuthorityConfig.Validate(c.GetAudiences()) } diff --git a/authority/tls.go b/authority/tls.go index 051e09c9..84106002 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -30,6 +30,7 @@ import ( casapi "github.com/smallstep/certificates/cas/apiv1" "github.com/smallstep/certificates/db" "github.com/smallstep/certificates/errs" + "github.com/smallstep/nosql/database" ) // GetTLSOptions returns the tls options configured. @@ -37,8 +38,11 @@ func (a *Authority) GetTLSOptions() *config.TLSOptions { return a.config.TLS } -var oidAuthorityKeyIdentifier = asn1.ObjectIdentifier{2, 5, 29, 35} -var oidSubjectKeyIdentifier = asn1.ObjectIdentifier{2, 5, 29, 14} +var ( + oidAuthorityKeyIdentifier = asn1.ObjectIdentifier{2, 5, 29, 35} + oidSubjectKeyIdentifier = asn1.ObjectIdentifier{2, 5, 29, 14} + oidExtensionIssuingDistributionPoint = asn1.ObjectIdentifier{2, 5, 29, 28} +) func withDefaultASN1DN(def *config.ASN1DN) provisioner.CertificateModifierFunc { return func(crt *x509.Certificate, opts provisioner.SignOptions) error { @@ -488,19 +492,15 @@ func (a *Authority) Revoke(ctx context.Context, revokeOpts *RevokeOptions) error RevokedAt: time.Now().UTC(), } - var ( - p provisioner.Interface - err error - ) - - if revokeOpts.Crt == nil { - // Attempt to get the certificate expiry using the serial number. - cert, err := a.db.GetCertificate(revokeOpts.Serial) - - // Revocation of a certificate not in the database may be requested, so fill in the expiry only - // if we can - if err == nil { - rci.ExpiresAt = cert.NotAfter + // For X509 CRLs attempt to get the expiration date of the certificate. + if provisioner.MethodFromContext(ctx) == provisioner.RevokeMethod { + if revokeOpts.Crt == nil { + cert, err := a.db.GetCertificate(revokeOpts.Serial) + if err == nil { + rci.ExpiresAt = cert.NotAfter + } + } else { + rci.ExpiresAt = revokeOpts.Crt.NotAfter } } @@ -508,8 +508,7 @@ func (a *Authority) Revoke(ctx context.Context, revokeOpts *RevokeOptions) error if !(revokeOpts.MTLS || revokeOpts.ACME) { token, err := jose.ParseSigned(revokeOpts.OTT) if err != nil { - return errs.Wrap(http.StatusUnauthorized, err, - "authority.Revoke; error parsing token", opts...) + return errs.Wrap(http.StatusUnauthorized, err, "authority.Revoke; error parsing token", opts...) } // Get claims w/out verification. @@ -519,28 +518,43 @@ func (a *Authority) Revoke(ctx context.Context, revokeOpts *RevokeOptions) error } // This method will also validate the audiences for JWK provisioners. - p, err = a.LoadProvisionerByToken(token, &claims.Claims) + p, err := a.LoadProvisionerByToken(token, &claims.Claims) if err != nil { return err } rci.ProvisionerID = p.GetID() rci.TokenID, err = p.GetTokenID(revokeOpts.OTT) if err != nil && !errors.Is(err, provisioner.ErrAllowTokenReuse) { - return errs.Wrap(http.StatusInternalServerError, err, - "authority.Revoke; could not get ID for token") + return errs.Wrap(http.StatusInternalServerError, err, "authority.Revoke; could not get ID for token") } opts = append(opts, errs.WithKeyVal("provisionerID", rci.ProvisionerID), errs.WithKeyVal("tokenID", rci.TokenID), ) - } else if p, err = a.LoadProvisionerByCertificate(revokeOpts.Crt); err == nil { + } else if p, err := a.LoadProvisionerByCertificate(revokeOpts.Crt); err == nil { // Load the Certificate provisioner if one exists. rci.ProvisionerID = p.GetID() 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 { - err = a.revokeSSH(nil, rci) + if err := a.revokeSSH(nil, rci); err != nil { + return failRevoke(err) + } } 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 @@ -555,7 +569,7 @@ func (a *Authority) Revoke(ctx context.Context, revokeOpts *RevokeOptions) error // 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{ + _, err := a.x509CAService.RevokeCertificate(&casapi.RevokeCertificateRequest{ Certificate: revokedCert, SerialNumber: rci.Serial, Reason: rci.Reason, @@ -567,37 +581,20 @@ func (a *Authority) Revoke(ctx context.Context, revokeOpts *RevokeOptions) error } // Save as revoked in the Db. - err = a.revoke(revokedCert, rci) - if err != nil { - return errs.Wrap(http.StatusInternalServerError, err, "authority.Revoke", opts...) + if err := a.revoke(revokedCert, rci); err != nil { + return failRevoke(err) } - if a.config.CRL != nil && a.config.CRL.GenerateOnRevoke { - // Generate a new CRL so CRL requesters will always get an up-to-date CRL whenever they request it - err = a.GenerateCertificateRevocationList() - if err != nil { + // 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...) } - - // the timer only gets reset if CRL is enabled - if a.config.CRL.Enabled { - a.resetCRLGeneratorTimer() - } } } - switch { - case err == nil: - return nil - 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...) - } + + return nil } func (a *Authority) revoke(crt *x509.Certificate, rci *db.RevokedCertificateInfo) error { @@ -621,7 +618,7 @@ func (a *Authority) revokeSSH(crt *ssh.Certificate, rci *db.RevokedCertificateIn // GetCertificateRevocationList will return the currently generated CRL from the DB, or a not implemented // error if the underlying AuthDB does not support CRLs func (a *Authority) GetCertificateRevocationList() ([]byte, error) { - if a.config.CRL == nil { + if !a.config.CRL.IsEnabled() { return nil, errs.Wrap(http.StatusNotFound, errors.Errorf("Certificate Revocation Lists are not enabled"), "authority.GetCertificateRevocationList") } @@ -633,11 +630,6 @@ func (a *Authority) GetCertificateRevocationList() ([]byte, error) { crlInfo, err := crlDB.GetCRL() if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.GetCertificateRevocationList") - - } - - if crlInfo == nil { - return nil, nil } return crlInfo.DER, nil @@ -646,9 +638,7 @@ func (a *Authority) GetCertificateRevocationList() ([]byte, error) { // GenerateCertificateRevocationList generates a DER representation of a signed CRL and stores it in the // database. Returns nil if CRL generation has been disabled in the config func (a *Authority) GenerateCertificateRevocationList() error { - - if a.config.CRL == nil { - // CRL is disabled + if !a.config.CRL.IsEnabled() { return nil } @@ -663,34 +653,40 @@ func (a *Authority) GenerateCertificateRevocationList() error { return errors.Errorf("CA does not support CRL Generation") } - a.crlMutex.Lock() // use a mutex to ensure only one CRL is generated at a time to avoid concurrency issues + // use a mutex to ensure only one CRL is generated at a time to avoid + // concurrency issues + a.crlMutex.Lock() defer a.crlMutex.Unlock() crlInfo, err := crlDB.GetCRL() - if err != nil { + if err != nil && !database.IsErrNotFound(err) { return errors.Wrap(err, "could not retrieve CRL from database") } + now := time.Now().UTC() revokedList, err := crlDB.GetRevokedCertificates() if err != nil { return errors.Wrap(err, "could not retrieve revoked certificates list from database") } - // Number is a monotonically increasing integer (essentially the CRL version number) that we need to - // keep track of and increase every time we generate a new CRL - var n int64 + // Number is a monotonically increasing integer (essentially the CRL version + // number) that we need to keep track of and increase every time we generate + // a new CRL var bn big.Int - if crlInfo != nil { - n = crlInfo.Number + 1 + bn.SetInt64(crlInfo.Number + 1) } - bn.SetInt64(n) - // Convert our database db.RevokedCertificateInfo types into the pkix representation ready for the - // CAS to sign it + // Convert our database db.RevokedCertificateInfo types into the pkix + // representation ready for the CAS to sign it var revokedCertificates []pkix.RevokedCertificate - + skipExpiredTime := now.Add(-config.DefaultCRLExpiredDuration.Abs()) for _, revokedCert := range *revokedList { + // skip expired certificates + if !revokedCert.ExpiresAt.IsZero() && revokedCert.ExpiresAt.Before(skipExpiredTime) { + continue + } + var sn big.Int sn.SetString(revokedCert.Serial, 10) revokedCertificates = append(revokedCertificates, pkix.RevokedCertificate{ @@ -713,9 +709,18 @@ func (a *Authority) GenerateCertificateRevocationList() error { SignatureAlgorithm: 0, RevokedCertificates: revokedCertificates, Number: &bn, - ThisUpdate: time.Now().UTC(), - NextUpdate: time.Now().UTC().Add(updateDuration), - ExtraExtensions: nil, + ThisUpdate: now, + NextUpdate: now.Add(updateDuration), + } + + // Add distribution point. + // + // Note that this is currently using the port 443 by default. + fullName := a.config.Audience("/1.0/crl")[0] + if b, err := marshalDistributionPoint(fullName, false); err == nil { + revocationList.ExtraExtensions = []pkix.Extension{ + {Id: oidExtensionIssuingDistributionPoint, Value: b}, + } } certificateRevocationList, err := caCRLGenerator.CreateCRL(&casapi.CreateCRLRequest{RevocationList: &revocationList}) @@ -726,7 +731,7 @@ func (a *Authority) GenerateCertificateRevocationList() error { // Create a new db.CertificateRevocationListInfo, which stores the new Number we just generated, the // expiry time, duration, and the DER-encoded CRL newCRLInfo := db.CertificateRevocationListInfo{ - Number: n, + Number: bn.Int64(), ExpiresAt: revocationList.NextUpdate, DER: certificateRevocationList.CRL, Duration: updateDuration, @@ -834,6 +839,33 @@ func (a *Authority) GetTLSCertificate() (*tls.Certificate, error) { return &tlsCrt, nil } +// RFC 5280, 5.2.5 +type distributionPoint struct { + DistributionPoint distributionPointName `asn1:"optional,tag:0"` + OnlyContainsUserCerts bool `asn1:"optional,tag:1"` + OnlyContainsCACerts bool `asn1:"optional,tag:2"` + OnlySomeReasons asn1.BitString `asn1:"optional,tag:3"` + IndirectCRL bool `asn1:"optional,tag:4"` + OnlyContainsAttributeCerts bool `asn1:"optional,tag:5"` +} + +type distributionPointName struct { + FullName []asn1.RawValue `asn1:"optional,tag:0"` + RelativeName pkix.RDNSequence `asn1:"optional,tag:1"` +} + +func marshalDistributionPoint(fullName string, isCA bool) ([]byte, error) { + return asn1.Marshal(distributionPoint{ + DistributionPoint: distributionPointName{ + FullName: []asn1.RawValue{ + {Class: 2, Tag: 6, Bytes: []byte(fullName)}, + }, + }, + OnlyContainsUserCerts: !isCA, + OnlyContainsCACerts: isCA, + }) +} + // templatingError tries to extract more information about the cause of // an error related to (most probably) malformed template data and adds // this to the error message. diff --git a/authority/tls_test.go b/authority/tls_test.go index 6223bb3d..1d542cb9 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -28,11 +28,13 @@ import ( "github.com/smallstep/assert" "github.com/smallstep/certificates/api/render" + "github.com/smallstep/certificates/authority/config" "github.com/smallstep/certificates/authority/policy" "github.com/smallstep/certificates/authority/provisioner" "github.com/smallstep/certificates/cas/softcas" "github.com/smallstep/certificates/db" "github.com/smallstep/certificates/errs" + "github.com/smallstep/nosql/database" ) var ( @@ -1364,7 +1366,7 @@ func TestAuthority_Revoke(t *testing.T) { return true, nil }, MGetCertificate: func(sn string) (*x509.Certificate, error) { - return nil, nil + return nil, errors.New("not found") }, Err: errors.New("force"), })) @@ -1404,7 +1406,7 @@ func TestAuthority_Revoke(t *testing.T) { return true, nil }, MGetCertificate: func(sn string) (*x509.Certificate, error) { - return nil, nil + return nil, errors.New("not found") }, Err: db.ErrAlreadyExists, })) @@ -1698,11 +1700,10 @@ func TestAuthority_CRL(t *testing.T) { ctx context.Context expected []string err error - code int } tests := map[string]func() test{ - "ok/empty-crl": func() test { - _a := testAuthority(t, WithDatabase(&db.MockAuthDB{ + "fail/empty-crl": func() test { + a := testAuthority(t, WithDatabase(&db.MockAuthDB{ MUseToken: func(id, tok string) (bool, error) { return true, nil }, @@ -1714,7 +1715,7 @@ func TestAuthority_CRL(t *testing.T) { return nil }, MGetCRL: func() (*db.CertificateRevocationListInfo, error) { - return &crlStore, nil + return nil, database.ErrNotFound }, MGetRevokedCertificates: func() (*[]db.RevokedCertificateInfo, error) { return &revokedList, nil @@ -1724,15 +1725,19 @@ func TestAuthority_CRL(t *testing.T) { return nil }, })) + a.config.CRL = &config.CRLConfig{ + Enabled: true, + } return test{ - auth: _a, + auth: a, ctx: crlCtx, expected: nil, + err: database.ErrNotFound, } }, "ok/crl-full": func() test { - _a := testAuthority(t, WithDatabase(&db.MockAuthDB{ + a := testAuthority(t, WithDatabase(&db.MockAuthDB{ MUseToken: func(id, tok string) (bool, error) { return true, nil }, @@ -1754,6 +1759,10 @@ func TestAuthority_CRL(t *testing.T) { return nil }, })) + a.config.CRL = &config.CRLConfig{ + Enabled: true, + GenerateOnRevoke: true, + } var ex []string @@ -1770,7 +1779,7 @@ func TestAuthority_CRL(t *testing.T) { } raw, err := jwt.Signed(sig).Claims(cl).CompactSerialize() assert.FatalError(t, err) - err = _a.Revoke(crlCtx, &RevokeOptions{ + err = a.Revoke(crlCtx, &RevokeOptions{ Serial: sn, ReasonCode: reasonCode, Reason: reason, @@ -1783,7 +1792,7 @@ func TestAuthority_CRL(t *testing.T) { } return test{ - auth: _a, + auth: a, ctx: crlCtx, expected: ex, } @@ -1792,10 +1801,11 @@ func TestAuthority_CRL(t *testing.T) { for name, f := range tests { tc := f() t.Run(name, func(t *testing.T) { - if crlBytes, _err := tc.auth.GetCertificateRevocationList(); _err == nil { + if crlBytes, err := tc.auth.GetCertificateRevocationList(); err == nil { crl, parseErr := x509.ParseCRL(crlBytes) if parseErr != nil { t.Errorf("x509.ParseCertificateRequest() error = %v, wantErr %v", parseErr, nil) + return } var cmpList []string @@ -1804,9 +1814,8 @@ func TestAuthority_CRL(t *testing.T) { } assert.Equals(t, cmpList, tc.expected) - } else { - assert.NotNil(t, tc.err) + assert.NotNil(t, tc.err, err.Error()) } }) } diff --git a/cas/softcas/softcas.go b/cas/softcas/softcas.go index af93420d..6eae9e9e 100644 --- a/cas/softcas/softcas.go +++ b/cas/softcas/softcas.go @@ -135,7 +135,6 @@ func (c *SoftCAS) RevokeCertificate(req *apiv1.RevokeCertificateRequest) (*apiv1 // CreateCRL will create a new CRL based on the RevocationList passed to it func (c *SoftCAS) CreateCRL(req *apiv1.CreateCRLRequest) (*apiv1.CreateCRLResponse, error) { - certChain, signer, err := c.getCertSigner() if err != nil { return nil, err diff --git a/db/db.go b/db/db.go index 4db437fe..bf1b2f48 100644 --- a/db/db.go +++ b/db/db.go @@ -271,14 +271,12 @@ func (db *DB) GetRevokedCertificates() (*[]RevokedCertificateInfo, error) { revokedCerts = append(revokedCerts, data) } } - } return &revokedCerts, nil } // StoreCRL stores a CRL in the DB func (db *DB) StoreCRL(crlInfo *CertificateRevocationListInfo) error { - crlInfoBytes, err := json.Marshal(crlInfo) if err != nil { return errors.Wrap(err, "json Marshal error") @@ -293,11 +291,6 @@ func (db *DB) StoreCRL(crlInfo *CertificateRevocationListInfo) error { // GetCRL gets the existing CRL from the database func (db *DB) GetCRL() (*CertificateRevocationListInfo, error) { crlInfoBytes, err := db.Get(crlTable, crlKey) - - if database.IsErrNotFound(err) { - return nil, nil - } - if err != nil { return nil, errors.Wrap(err, "database Get error") } diff --git a/docs/GETTING_STARTED.md b/docs/GETTING_STARTED.md index 328fba69..0465f0b7 100644 --- a/docs/GETTING_STARTED.md +++ b/docs/GETTING_STARTED.md @@ -119,11 +119,6 @@ starting the CA. * `address`: e.g. `127.0.0.1:8080` - address and port on which the CA will bind and respond to requests. -* `crl`: Certificate Revocation List settings: - - generate: Enable/Disable CRL generation (`true` to generate, `false` to disable) - - - cacheDuration: Time between CRL regeneration task. E.g if set to `5m`, step-ca will regenerate the CRL every 5 minutes. - * `dnsNames`: comma separated list of DNS Name(s) for the CA. * `logger`: the default logging format for the CA is `text`. The other option @@ -131,17 +126,20 @@ is `json`. * `db`: data persistence layer. See [database documentation](./database.md) for more info. + * `type`: `badger`, `bbolt`, `mysql`, etc. + * `dataSource`: string that can be interpreted differently depending on the + type of the database. Usually a path to where the data is stored. See the + [database configuration docs](./database.md#configuration) for more info. + * `database`: name of the database. Used for backends that may have multiple + databases. e.g. MySQL + * `valueDir`: directory to store the value log in (Badger specific). - - type: `badger`, `bbolt`, `mysql`, etc. - - - dataSource: `string` that can be interpreted differently depending on the - type of the database. Usually a path to where the data is stored. See - the [database configuration docs](./database.md#configuration) for more info. - - - database: name of the database. Used for backends that may have - multiple databases. e.g. MySQL - - - valueDir: directory to store the value log in (Badger specific). +* `crl`: Certificate Revocation List settings: + * `enable`: enables CRL generation (`true` to generate, `false` to disable) + * `generateOnRevoke`: a revoke will generate a new CRL if the crl is enabled. + * `cacheDuration`: the duration until next update of the CRL, defaults to 24h. + * `renewPeriod`: the time between CRL regeneration. If not set ~2/3 of the + cacheDuration will be used. * `tls`: settings for negotiating communication with the CA; includes acceptable ciphersuites, min/max TLS version, etc.