From d417ce32326067ff249f34eeff20ee82ac243d9d Mon Sep 17 00:00:00 2001 From: Raal Goff Date: Thu, 4 Nov 2021 14:05:07 +0800 Subject: [PATCH] implement changes from review --- api/api.go | 3 +- api/crl.go | 44 ++++++++++++++--- authority/authority.go | 70 +++++++++++++++++++++++++++ authority/config/config.go | 7 +++ authority/tls.go | 98 +++++++++++++++++++++++++++----------- cas/apiv1/requests.go | 10 ++++ cas/apiv1/services.go | 2 +- cas/softcas/softcas.go | 8 ++-- db/db.go | 28 +++++++++-- 9 files changed, 224 insertions(+), 46 deletions(-) diff --git a/api/api.go b/api/api.go index ba7d3d17..25b6efa7 100644 --- a/api/api.go +++ b/api/api.go @@ -50,7 +50,8 @@ type Authority interface { GetRoots() ([]*x509.Certificate, error) GetFederation() ([]*x509.Certificate, error) Version() authority.Version - GenerateCertificateRevocationList(force bool) ([]byte, error) + GenerateCertificateRevocationList() error + GetCertificateRevocationList() ([]byte, error) } // TimeDuration is an alias of provisioner.TimeDuration diff --git a/api/crl.go b/api/crl.go index 46758777..4d20cfeb 100644 --- a/api/crl.go +++ b/api/crl.go @@ -2,23 +2,53 @@ package api import ( "encoding/pem" + "fmt" + "github.com/pkg/errors" "net/http" ) -// CRL is an HTTP handler that returns the current CRL in PEM format +// CRL is an HTTP handler that returns the current CRL in DER or PEM format func (h *caHandler) CRL(w http.ResponseWriter, r *http.Request) { - crlBytes, err := h.Authority.GenerateCertificateRevocationList(false) + crlBytes, err := h.Authority.GetCertificateRevocationList() + + _, formatAsPEM := r.URL.Query()["pem"] if err != nil { w.WriteHeader(500) + _, err = fmt.Fprintf(w, "%v\n", err) + if err != nil { + panic(errors.Wrap(err, "error writing http response")) + } return } - pemBytes := pem.EncodeToMemory(&pem.Block{ - Type: "X509 CRL", - Bytes: crlBytes, - }) + if crlBytes == nil { + w.WriteHeader(404) + _, err = fmt.Fprintln(w, "No CRL available") + if err != nil { + panic(errors.Wrap(err, "error writing http response")) + } + return + } + + if formatAsPEM { + pemBytes := pem.EncodeToMemory(&pem.Block{ + Type: "X509 CRL", + Bytes: crlBytes, + }) + w.Header().Add("Content-Type", "application/x-pem-file") + w.Header().Add("Content-Disposition", "attachment; filename=\"crl.pem\"") + _, err = w.Write(pemBytes) + } else { + w.Header().Add("Content-Type", "application/pkix-crl") + w.Header().Add("Content-Disposition", "attachment; filename=\"crl.der\"") + _, err = w.Write(crlBytes) + } w.WriteHeader(200) - _, err = w.Write(pemBytes) + + if err != nil { + panic(errors.Wrap(err, "error writing http response")) + } + } diff --git a/authority/authority.go b/authority/authority.go index b6071060..2454c272 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -65,6 +65,9 @@ type Authority struct { sshCAUserFederatedCerts []ssh.PublicKey sshCAHostFederatedCerts []ssh.PublicKey + // CRL vars + crlChannel chan int + // Do not re-initialize initOnce bool startTime time.Time @@ -553,6 +556,16 @@ func (a *Authority) init() error { // not be repeated. a.initOnce = true + // Start the CRL generator + if a.config.CRL != nil { + if a.config.CRL.Generate && a.config.CRL.CacheDuration.Duration > time.Duration(0) { + err := a.startCRLGenerator() + if err != nil { + return err + } + } + } + return nil } @@ -646,3 +659,60 @@ func (a *Authority) requiresSCEPService() bool { func (a *Authority) GetSCEPService() *scep.Service { return a.scepService } + +func (a *Authority) startCRLGenerator() error { + + if a.config.CRL.CacheDuration.Duration > time.Duration(0) { + // Check that there is a valid CRL in the DB right now. If it doesnt exist + // or is expired, generated one now + crlDB, ok := a.db.(db.CertificateRevocationListDB) + if !ok { + return errors.Errorf("CRL Generation requested, but database does not support CRL generation") + } + + crlInfo, err := crlDB.GetCRL() + if err != nil { + return errors.Wrap(err, "could not retrieve CRL from database") + } + + if crlInfo == nil { + log.Println("No CRL exists in the DB, generating one now") + err = a.GenerateCertificateRevocationList() + if err != nil { + return errors.Wrap(err, "could not generate a CRL") + } + } + + if crlInfo.ExpiresAt.Before(time.Now().UTC()) { + log.Printf("Existing CRL has expired (at %v), generating a new one", crlInfo.ExpiresAt) + err = a.GenerateCertificateRevocationList() + if err != nil { + return errors.Wrap(err, "could not generate a CRL") + } + } + + log.Printf("CRL will be auto-generated every %v", a.config.CRL.CacheDuration) + tickerDuration := a.config.CRL.CacheDuration.Duration - time.Minute // generate the new CRL 1 minute before it expires + if tickerDuration <= 0 { + log.Printf("WARNING: Addition of jitter to CRL generation time %v creates a negative duration (%v). Using 1 minute cacheDuration", a.config.CRL.CacheDuration, tickerDuration) + tickerDuration = time.Minute + } + crlTicker := time.NewTicker(tickerDuration) + + go func() { + for { + select { + case <-crlTicker.C: + log.Println("Regenerating CRL") + err := a.GenerateCertificateRevocationList() + if err != nil { + // TODO: log or panic here? + panic(errors.Wrap(err, "authority.crlGenerator encountered an error")) + } + } + } + }() + } + + return nil +} diff --git a/authority/config/config.go b/authority/config/config.go index 2c437725..4bf51cfe 100644 --- a/authority/config/config.go +++ b/authority/config/config.go @@ -68,6 +68,7 @@ type Config struct { TLS *TLSOptions `json:"tls,omitempty"` Password string `json:"password,omitempty"` Templates *templates.Templates `json:"templates,omitempty"` + CRL *CRLConfig `json:"crl,omitempty"` } // ASN1DN contains ASN1.DN attributes that are used in Subject and Issuer @@ -99,6 +100,12 @@ type AuthConfig struct { EnableAdmin bool `json:"enableAdmin,omitempty"` } +// CRLConfig represents config options for CRL generation +type CRLConfig struct { + Generate bool `json:"generate,omitempty"` + CacheDuration *provisioner.Duration `json:"cacheDuration,omitempty"` +} + // init initializes the required fields in the AuthConfig if they are not // provided. func (c *AuthConfig) init() { diff --git a/authority/tls.go b/authority/tls.go index c6dc7d27..19e06659 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -408,6 +408,16 @@ func (a *Authority) Revoke(ctx context.Context, revokeOpts *RevokeOptions) error p provisioner.Interface err error ) + + // 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 + } + // If not mTLS nor ACME, then get the TokenID of the token. if !(revokeOpts.MTLS || revokeOpts.ACME) { token, err := jose.ParseSigned(revokeOpts.OTT) @@ -474,7 +484,10 @@ func (a *Authority) Revoke(ctx context.Context, revokeOpts *RevokeOptions) error err = a.revoke(revokedCert, rci) // Generate a new CRL so CRL requesters will always get an up-to-date CRL whenever they request it - _, _ = a.GenerateCertificateRevocationList(true) + err = a.GenerateCertificateRevocationList() + if err != nil { + return errs.Wrap(http.StatusInternalServerError, err, "authority.Revoke", opts...) + } } switch err { case nil: @@ -509,36 +522,64 @@ func (a *Authority) revokeSSH(crt *ssh.Certificate, rci *db.RevokedCertificateIn return a.db.Revoke(rci) } -// GenerateCertificateRevocationList returns a DER representation of a signed CRL. -// It will look for a valid generated CRL in the database, check if it has expired, and generate -// a new CRL on demand if it has expired (or a CRL does not already exist). -// -// force set to true will force regeneration of the CRL regardless of whether it has actually expired -func (a *Authority) GenerateCertificateRevocationList(force bool) ([]byte, error) { - - // check for an existing CRL in the database, and return that if its valid - crlInfo, err := a.db.GetCRL() - - if err != nil { - return nil, err +// 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 { + return nil, errs.Wrap(http.StatusInternalServerError, errors.Errorf("Certificate Revocation Lists are not enabled"), "authority.GetCertificateRevocationList") } - if !force && crlInfo != nil && crlInfo.ExpiresAt.After(time.Now().UTC()) { - return crlInfo.DER, nil + crlDB, ok := a.db.(db.CertificateRevocationListDB) + if !ok { + return nil, errs.Wrap(http.StatusInternalServerError, errors.Errorf("Database does not support Certificate Revocation Lists"), "authority.GetCertificateRevocationList") + } + + 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 +} + +// 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 + return nil + } + + crlDB, ok := a.db.(db.CertificateRevocationListDB) + if !ok { + return errors.Errorf("Database does not support CRL generation") } // some CAS may not implement the CRLGenerator interface, so check before we proceed caCRLGenerator, ok := a.x509CAService.(casapi.CertificateAuthorityCRLGenerator) - if !ok { - return nil, errors.Errorf("CRL Generator not implemented") + return errors.Errorf("CA does not support CRL Generation") } - revokedList, err := a.db.GetRevokedCertificates() + crlInfo, err := crlDB.GetCRL() + if err != nil { + return errors.Wrap(err, "could not retrieve CRL from database") + } + + 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 = 0 + var n int64 var bn big.Int if crlInfo != nil { @@ -561,37 +602,36 @@ func (a *Authority) GenerateCertificateRevocationList(force bool) ([]byte, error } // Create a RevocationList representation ready for the CAS to sign - // TODO: use a config value for the NextUpdate time duration // TODO: allow SignatureAlgorithm to be specified? revocationList := x509.RevocationList{ SignatureAlgorithm: 0, RevokedCertificates: revokedCertificates, Number: &bn, ThisUpdate: time.Now().UTC(), - NextUpdate: time.Now().UTC().Add(time.Minute * 10), + NextUpdate: time.Now().UTC().Add(a.config.CRL.CacheDuration.Duration), ExtraExtensions: nil, } - certificateRevocationList, err := caCRLGenerator.CreateCertificateRevocationList(&revocationList) + certificateRevocationList, err := caCRLGenerator.CreateCRL(&casapi.CreateCRLRequest{RevocationList: &revocationList}) if err != nil { - return nil, err + return errors.Wrap(err, "could not create CRL") } // Create a new db.CertificateRevocationListInfo, which stores the new Number we just generated, the - // expiry time, and the DER-encoded CRL - then store it in the DB + // expiry time, and the DER-encoded CRL newCRLInfo := db.CertificateRevocationListInfo{ Number: n, ExpiresAt: revocationList.NextUpdate, - DER: certificateRevocationList, + DER: certificateRevocationList.CRL, } - err = a.db.StoreCRL(&newCRLInfo) + // Store the CRL in the database ready for retrieval by api endpoints + err = crlDB.StoreCRL(&newCRLInfo) if err != nil { - return nil, err + return errors.Wrap(err, "could not store CRL in database") } - // Finally, return our CRL in DER - return certificateRevocationList, nil + return nil } // GetTLSCertificate creates a new leaf certificate to be used by the CA HTTPS server. diff --git a/cas/apiv1/requests.go b/cas/apiv1/requests.go index bf745c17..630cecce 100644 --- a/cas/apiv1/requests.go +++ b/cas/apiv1/requests.go @@ -144,3 +144,13 @@ type CreateCertificateAuthorityResponse struct { PrivateKey crypto.PrivateKey Signer crypto.Signer } + +// CreateCRLRequest is the request to create a Certificate Revocation List. +type CreateCRLRequest struct { + RevocationList *x509.RevocationList +} + +// CreateCRLResponse is the response to a Certificate Revocation List request. +type CreateCRLResponse struct { + CRL []byte //the CRL in DER format +} diff --git a/cas/apiv1/services.go b/cas/apiv1/services.go index 143c1f17..ce7d0364 100644 --- a/cas/apiv1/services.go +++ b/cas/apiv1/services.go @@ -17,7 +17,7 @@ type CertificateAuthorityService interface { // CertificateAuthorityCRLGenerator is an optional interface implemented by CertificateAuthorityService // that has a method to create a CRL type CertificateAuthorityCRLGenerator interface { - CreateCertificateRevocationList(crl *x509.RevocationList) ([]byte, error) + CreateCRL(req *CreateCRLRequest) (*CreateCRLResponse, error) } // CertificateAuthorityGetter is an interface implemented by a diff --git a/cas/softcas/softcas.go b/cas/softcas/softcas.go index 3c400080..bec1d81e 100644 --- a/cas/softcas/softcas.go +++ b/cas/softcas/softcas.go @@ -130,15 +130,15 @@ func (c *SoftCAS) RevokeCertificate(req *apiv1.RevokeCertificateRequest) (*apiv1 }, nil } -// CreateCertificateRevocationList will create a new CRL based on the RevocationList passed to it -func (c *SoftCAS) CreateCertificateRevocationList(crl *x509.RevocationList) ([]byte, error) { +// CreateCRL will create a new CRL based on the RevocationList passed to it +func (c *SoftCAS) CreateCRL(req *apiv1.CreateCRLRequest) (*apiv1.CreateCRLResponse, error) { - revocationList, err := x509.CreateRevocationList(rand.Reader, crl, c.CertificateChain[0], c.Signer) + revocationListBytes, err := x509.CreateRevocationList(rand.Reader, req.RevocationList, c.CertificateChain[0], c.Signer) if err != nil { return nil, err } - return revocationList, nil + return &apiv1.CreateCRLResponse{CRL: revocationListBytes}, nil } // CreateCertificateAuthority creates a root or an intermediate certificate. diff --git a/db/db.go b/db/db.go index 534c4000..bf3b97ec 100644 --- a/db/db.go +++ b/db/db.go @@ -51,9 +51,6 @@ type AuthDB interface { IsSSHRevoked(sn string) (bool, error) Revoke(rci *RevokedCertificateInfo) error RevokeSSH(rci *RevokedCertificateInfo) error - GetRevokedCertificates() (*[]RevokedCertificateInfo, error) - GetCRL() (*CertificateRevocationListInfo, error) - StoreCRL(*CertificateRevocationListInfo) error GetCertificate(serialNumber string) (*x509.Certificate, error) StoreCertificate(crt *x509.Certificate) error UseToken(id, tok string) (bool, error) @@ -63,6 +60,13 @@ type AuthDB interface { Shutdown() error } +// CertificateRevocationListDB is an interface to indicate whether the DB supports CRL generation +type CertificateRevocationListDB interface { + GetRevokedCertificates() (*[]RevokedCertificateInfo, error) + GetCRL() (*CertificateRevocationListInfo, error) + StoreCRL(*CertificateRevocationListInfo) error +} + // DB is a wrapper over the nosql.DB interface. type DB struct { nosql.DB @@ -109,6 +113,7 @@ type RevokedCertificateInfo struct { ReasonCode int Reason string RevokedAt time.Time + ExpiresAt time.Time TokenID string MTLS bool ACME bool @@ -216,7 +221,22 @@ func (db *DB) GetRevokedCertificates() (*[]RevokedCertificateInfo, error) { if err := json.Unmarshal(e.Value, &data); err != nil { return nil, err } - revokedCerts = append(revokedCerts, data) + + if !data.ExpiresAt.IsZero() && data.ExpiresAt.After(time.Now().UTC()) { + revokedCerts = append(revokedCerts, data) + } else if data.ExpiresAt.IsZero() { + cert, err := db.GetCertificate(data.Serial) + if err != nil { + revokedCerts = append(revokedCerts, data) // a revoked certificate may not be in the database, + // so its expiry date is undiscoverable and will need + // to be added to the crl always + continue + } + + if cert.NotAfter.After(time.Now().UTC()) { + revokedCerts = append(revokedCerts, data) + } + } } return &revokedCerts, nil