From 3813f57b1a1e1c3364cf14bf7726cb130a2576e4 Mon Sep 17 00:00:00 2001 From: dharanikumar-s Date: Wed, 1 Jul 2020 19:10:13 +0530 Subject: [PATCH 01/12] Add support for rekeying Fixes #292 --- api/api.go | 2 ++ api/rekey.go | 65 +++++++++++++++++++++++++++++++++++++ authority/tls.go | 84 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 151 insertions(+) create mode 100644 api/rekey.go diff --git a/api/api.go b/api/api.go index 37222be8..1ce1e85d 100644 --- a/api/api.go +++ b/api/api.go @@ -35,6 +35,7 @@ type Authority interface { Root(shasum string) (*x509.Certificate, error) Sign(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) ([]*x509.Certificate, error) Renew(peer *x509.Certificate) ([]*x509.Certificate, error) + Rekey(peer *x509.Certificate, csr *x509.CertificateRequest) ([]*x509.Certificate, error) LoadProvisionerByCertificate(*x509.Certificate) (provisioner.Interface, error) LoadProvisionerByID(string) (provisioner.Interface, error) GetProvisioners(cursor string, limit int) (provisioner.List, string, error) @@ -249,6 +250,7 @@ func (h *caHandler) Route(r Router) { r.MethodFunc("GET", "/root/{sha}", h.Root) r.MethodFunc("POST", "/sign", h.Sign) r.MethodFunc("POST", "/renew", h.Renew) + r.MethodFunc("POST", "/rekey", h.Rekey) r.MethodFunc("POST", "/revoke", h.Revoke) r.MethodFunc("GET", "/provisioners", h.Provisioners) r.MethodFunc("GET", "/provisioners/{kid}/encrypted-key", h.ProvisionerKey) diff --git a/api/rekey.go b/api/rekey.go new file mode 100644 index 00000000..c53b9753 --- /dev/null +++ b/api/rekey.go @@ -0,0 +1,65 @@ +package api + +import ( + "net/http" + + "github.com/smallstep/certificates/errs" +) + +// RekeyRequest is the request body for a certificate rekey request. +type RekeyRequest struct { + CsrPEM CertificateRequest `json:"csr"` +} + +// Validate checks the fields of the RekeyRequest and returns nil if they are ok +// or an error if something is wrong. +func (s *RekeyRequest) Validate() error { + if s.CsrPEM.CertificateRequest == nil { + return errs.BadRequest("missing csr") + } + if err := s.CsrPEM.CertificateRequest.CheckSignature(); err != nil { + return errs.Wrap(http.StatusBadRequest, err, "invalid csr") + } + + return nil +} + + +// Rekey is similar to renew except that the certificate will be renewed with new key from csr. +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")) + return + } + + if err := body.Validate(); err != nil { + WriteError(w, err) + return + } + + if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { + WriteError(w, errs.BadRequest("missing peer certificate")) + return + } + + certChain, err := h.Authority.Rekey(r.TLS.PeerCertificates[0],body.CsrPEM.CertificateRequest) + if err != nil { + WriteError(w, errs.Wrap(http.StatusInternalServerError, err, "cahandler.Rekey")) + return + } + certChainPEM := certChainToPEM(certChain) + var caPEM Certificate + if len(certChainPEM) > 1 { + caPEM = certChainPEM[1] + } + + logCertificate(w, certChain[0]) + JSONStatus(w, &SignResponse{ + ServerPEM: certChainPEM[0], + CaPEM: caPEM, + CertChainPEM: certChainPEM, + TLSOptions: h.Authority.GetTLSOptions(), + }, http.StatusCreated) +} diff --git a/authority/tls.go b/authority/tls.go index dbfbf96a..ac8ff578 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -199,6 +199,89 @@ func (a *Authority) Renew(oldCert *x509.Certificate) ([]*x509.Certificate, error "authority.Renew; error renewing certificate from existing server certificate", opts...) } + serverCert, err := x509.ParseCertificate(crtBytes) + if err != nil { + return nil, errs.Wrap(http.StatusInternalServerError, err, + "authority.Renew; error parsing new server certificate", opts...) + } + + if err = a.db.StoreCertificate(serverCert); err != nil { + if err != db.ErrNotImplemented { + return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.Renew; error storing certificate in db", opts...) + } + } + + return []*x509.Certificate{serverCert, a.x509Issuer}, nil +} + +// Rekey is similar to renew except that the certificate will be renewed with new key from csr argument. +func (a *Authority) Rekey(oldCert *x509.Certificate, csr *x509.CertificateRequest) ([]*x509.Certificate, error) { + opts := []interface{}{errs.WithKeyVal("serialNumber", oldCert.SerialNumber.String())} + + // Check step provisioner extensions + if err := a.authorizeRenew(oldCert); err != nil { + return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.Renew", opts...) + } + + + // Durations + backdate := a.config.AuthorityConfig.Backdate.Duration + duration := oldCert.NotAfter.Sub(oldCert.NotBefore) + now := time.Now().UTC() + + + newCert := &x509.Certificate{ + PublicKey: csr.PublicKey, + Issuer: a.x509Issuer.Subject, + Subject: oldCert.Subject, + NotBefore: now.Add(-1 * backdate), + NotAfter: now.Add(duration - backdate), + KeyUsage: oldCert.KeyUsage, + UnhandledCriticalExtensions: oldCert.UnhandledCriticalExtensions, + ExtKeyUsage: oldCert.ExtKeyUsage, + UnknownExtKeyUsage: oldCert.UnknownExtKeyUsage, + BasicConstraintsValid: oldCert.BasicConstraintsValid, + IsCA: oldCert.IsCA, + MaxPathLen: oldCert.MaxPathLen, + MaxPathLenZero: oldCert.MaxPathLenZero, + OCSPServer: oldCert.OCSPServer, + IssuingCertificateURL: oldCert.IssuingCertificateURL, + PermittedDNSDomainsCritical: oldCert.PermittedDNSDomainsCritical, + PermittedEmailAddresses: oldCert.PermittedEmailAddresses, + DNSNames: oldCert.DNSNames, + EmailAddresses: oldCert.EmailAddresses, + IPAddresses: oldCert.IPAddresses, + URIs: oldCert.URIs, + PermittedDNSDomains: oldCert.PermittedDNSDomains, + ExcludedDNSDomains: oldCert.ExcludedDNSDomains, + PermittedIPRanges: oldCert.PermittedIPRanges, + ExcludedIPRanges: oldCert.ExcludedIPRanges, + ExcludedEmailAddresses: oldCert.ExcludedEmailAddresses, + PermittedURIDomains: oldCert.PermittedURIDomains, + ExcludedURIDomains: oldCert.ExcludedURIDomains, + CRLDistributionPoints: oldCert.CRLDistributionPoints, + PolicyIdentifiers: oldCert.PolicyIdentifiers, + } + + // Copy all extensions except for Authority Key Identifier. This one might + // be different if we rotate the intermediate certificate and it will cause + // a TLS bad certificate error. + for _, ext := range oldCert.Extensions { + if !ext.Id.Equal(oidAuthorityKeyIdentifier) { + newCert.ExtraExtensions = append(newCert.ExtraExtensions, ext) + } + } + + leaf, err := x509util.NewLeafProfileWithTemplate(newCert, a.x509Issuer, a.x509Signer) + if err != nil { + return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.Renew", opts...) + } + crtBytes, err := leaf.CreateCertificate() + if err != nil { + return nil, errs.Wrap(http.StatusInternalServerError, err, + "authority.Renew; error renewing certificate from existing server certificate", opts...) + } + serverCert, err := x509.ParseCertificate(crtBytes) if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, @@ -214,6 +297,7 @@ func (a *Authority) Renew(oldCert *x509.Certificate) ([]*x509.Certificate, error return []*x509.Certificate{serverCert, a.x509Issuer}, nil } + // RevokeOptions are the options for the Revoke API. type RevokeOptions struct { Serial string From 8f504483ceaec2bfa86b049009cea8d07c28e3c2 Mon Sep 17 00:00:00 2001 From: dharanikumar-s Date: Fri, 3 Jul 2020 15:58:15 +0530 Subject: [PATCH 02/12] Added RenewOrRekey function based on @maraino suggestion. RenewOrReky is called from Renew. --- api/api.go | 3 +- api/rekey.go | 12 +++--- authority/tls.go | 96 ++++++------------------------------------------ 3 files changed, 19 insertions(+), 92 deletions(-) diff --git a/api/api.go b/api/api.go index 1ce1e85d..57c81262 100644 --- a/api/api.go +++ b/api/api.go @@ -2,6 +2,7 @@ package api import ( "context" + "crypto" "crypto/dsa" "crypto/ecdsa" "crypto/rsa" @@ -35,7 +36,7 @@ type Authority interface { Root(shasum string) (*x509.Certificate, error) Sign(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) ([]*x509.Certificate, error) Renew(peer *x509.Certificate) ([]*x509.Certificate, error) - Rekey(peer *x509.Certificate, csr *x509.CertificateRequest) ([]*x509.Certificate, error) + RenewOrRekey(peer *x509.Certificate, pk crypto.PublicKey) ([]*x509.Certificate, error) LoadProvisionerByCertificate(*x509.Certificate) (provisioner.Interface, error) LoadProvisionerByID(string) (provisioner.Interface, error) GetProvisioners(cursor string, limit int) (provisioner.List, string, error) diff --git a/api/rekey.go b/api/rekey.go index c53b9753..58878fd9 100644 --- a/api/rekey.go +++ b/api/rekey.go @@ -34,17 +34,17 @@ func (h *caHandler) Rekey(w http.ResponseWriter, r *http.Request) { return } - if err := body.Validate(); err != nil { - WriteError(w, err) - return - } - if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { WriteError(w, errs.BadRequest("missing peer certificate")) return } - certChain, err := h.Authority.Rekey(r.TLS.PeerCertificates[0],body.CsrPEM.CertificateRequest) + if err := body.Validate(); err != nil { + WriteError(w, err) + return + } + + certChain, err := h.Authority.RenewOrRekey(r.TLS.PeerCertificates[0],body.CsrPEM.CertificateRequest.PublicKey) if err != nil { WriteError(w, errs.Wrap(http.StatusInternalServerError, err, "cahandler.Rekey")) return diff --git a/authority/tls.go b/authority/tls.go index ac8ff578..3ab15838 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -2,6 +2,7 @@ package authority import ( "context" + "crypto" "crypto/tls" "crypto/x509" "encoding/asn1" @@ -135,92 +136,17 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Opti // 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) { - opts := []interface{}{errs.WithKeyVal("serialNumber", oldCert.SerialNumber.String())} - - // Check step provisioner extensions - if err := a.authorizeRenew(oldCert); err != nil { - return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.Renew", opts...) - } - - // Durations - backdate := a.config.AuthorityConfig.Backdate.Duration - duration := oldCert.NotAfter.Sub(oldCert.NotBefore) - now := time.Now().UTC() - - newCert := &x509.Certificate{ - PublicKey: oldCert.PublicKey, - Issuer: a.x509Issuer.Subject, - Subject: oldCert.Subject, - NotBefore: now.Add(-1 * backdate), - NotAfter: now.Add(duration - backdate), - KeyUsage: oldCert.KeyUsage, - UnhandledCriticalExtensions: oldCert.UnhandledCriticalExtensions, - ExtKeyUsage: oldCert.ExtKeyUsage, - UnknownExtKeyUsage: oldCert.UnknownExtKeyUsage, - BasicConstraintsValid: oldCert.BasicConstraintsValid, - IsCA: oldCert.IsCA, - MaxPathLen: oldCert.MaxPathLen, - MaxPathLenZero: oldCert.MaxPathLenZero, - OCSPServer: oldCert.OCSPServer, - IssuingCertificateURL: oldCert.IssuingCertificateURL, - PermittedDNSDomainsCritical: oldCert.PermittedDNSDomainsCritical, - PermittedEmailAddresses: oldCert.PermittedEmailAddresses, - DNSNames: oldCert.DNSNames, - EmailAddresses: oldCert.EmailAddresses, - IPAddresses: oldCert.IPAddresses, - URIs: oldCert.URIs, - PermittedDNSDomains: oldCert.PermittedDNSDomains, - ExcludedDNSDomains: oldCert.ExcludedDNSDomains, - PermittedIPRanges: oldCert.PermittedIPRanges, - ExcludedIPRanges: oldCert.ExcludedIPRanges, - ExcludedEmailAddresses: oldCert.ExcludedEmailAddresses, - PermittedURIDomains: oldCert.PermittedURIDomains, - ExcludedURIDomains: oldCert.ExcludedURIDomains, - CRLDistributionPoints: oldCert.CRLDistributionPoints, - PolicyIdentifiers: oldCert.PolicyIdentifiers, - } - - // Copy all extensions except for Authority Key Identifier. This one might - // be different if we rotate the intermediate certificate and it will cause - // a TLS bad certificate error. - for _, ext := range oldCert.Extensions { - if !ext.Id.Equal(oidAuthorityKeyIdentifier) { - newCert.ExtraExtensions = append(newCert.ExtraExtensions, ext) - } - } - - leaf, err := x509util.NewLeafProfileWithTemplate(newCert, a.x509Issuer, a.x509Signer) - if err != nil { - return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.Renew", opts...) - } - crtBytes, err := leaf.CreateCertificate() - if err != nil { - return nil, errs.Wrap(http.StatusInternalServerError, err, - "authority.Renew; error renewing certificate from existing server certificate", opts...) - } - - serverCert, err := x509.ParseCertificate(crtBytes) - if err != nil { - return nil, errs.Wrap(http.StatusInternalServerError, err, - "authority.Renew; error parsing new server certificate", opts...) - } - - if err = a.db.StoreCertificate(serverCert); err != nil { - if err != db.ErrNotImplemented { - return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.Renew; error storing certificate in db", opts...) - } - } - - return []*x509.Certificate{serverCert, a.x509Issuer}, nil + return a.RenewOrRekey(oldCert, oldCert.PublicKey) } -// Rekey is similar to renew except that the certificate will be renewed with new key from csr argument. -func (a *Authority) Rekey(oldCert *x509.Certificate, csr *x509.CertificateRequest) ([]*x509.Certificate, error) { +// Rekey is similar to renew except that the certificate will be renewed with new key. +// Function does rekeying if a new public key is passed as pk else if same key is passed, certificate will be just renewed. +func (a *Authority) RenewOrRekey(oldCert *x509.Certificate, pk crypto.PublicKey) ([]*x509.Certificate, error) { opts := []interface{}{errs.WithKeyVal("serialNumber", oldCert.SerialNumber.String())} // Check step provisioner extensions if err := a.authorizeRenew(oldCert); err != nil { - return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.Renew", opts...) + return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.RenewOrRekey", opts...) } @@ -231,7 +157,7 @@ func (a *Authority) Rekey(oldCert *x509.Certificate, csr *x509.CertificateReques newCert := &x509.Certificate{ - PublicKey: csr.PublicKey, + PublicKey: pk, Issuer: a.x509Issuer.Subject, Subject: oldCert.Subject, NotBefore: now.Add(-1 * backdate), @@ -274,23 +200,23 @@ func (a *Authority) Rekey(oldCert *x509.Certificate, csr *x509.CertificateReques leaf, err := x509util.NewLeafProfileWithTemplate(newCert, a.x509Issuer, a.x509Signer) if err != nil { - return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.Renew", opts...) + return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.RenewOrRekey", opts...) } crtBytes, err := leaf.CreateCertificate() if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, - "authority.Renew; error renewing certificate from existing server certificate", opts...) + "authority.RenewOrRenew; error renewing certificate from existing server certificate", opts...) } serverCert, err := x509.ParseCertificate(crtBytes) if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, - "authority.Renew; error parsing new server certificate", opts...) + "authority.RenewOrRekey; error parsing new server certificate", opts...) } if err = a.db.StoreCertificate(serverCert); err != nil { if err != db.ErrNotImplemented { - return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.Renew; error storing certificate in db", opts...) + return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.RenewOrRekey; error storing certificate in db", opts...) } } From 01a6469d25391618a926220151ac0d4019a37364 Mon Sep 17 00:00:00 2001 From: dharanikumar-s Date: Fri, 3 Jul 2020 16:00:22 +0530 Subject: [PATCH 03/12] Moved peer certificate check to the first line --- api/rekey.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/api/rekey.go b/api/rekey.go index 58878fd9..1638b56e 100644 --- a/api/rekey.go +++ b/api/rekey.go @@ -28,16 +28,17 @@ func (s *RekeyRequest) Validate() error { // Rekey is similar to renew except that the certificate will be renewed with new key from csr. func (h *caHandler) Rekey(w http.ResponseWriter, r *http.Request) { + if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { + WriteError(w, errs.BadRequest("missing peer certificate")) + return + } + var body RekeyRequest if err := ReadJSON(r.Body, &body); err != nil { WriteError(w, errs.Wrap(http.StatusBadRequest, err, "error reading request body")) return } - if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { - WriteError(w, errs.BadRequest("missing peer certificate")) - return - } if err := body.Validate(); err != nil { WriteError(w, err) From 954fda657b7852806c16c47587196e1aac9435a2 Mon Sep 17 00:00:00 2001 From: dharanikumar-s Date: Sun, 5 Jul 2020 22:05:00 +0530 Subject: [PATCH 04/12] Added renewOrRekey to mockAuthority. Added Test_caHandler_Rekey --- api/api_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/api/api_test.go b/api/api_test.go index edefbd47..8350a41d 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -3,6 +3,7 @@ package api import ( "bytes" "context" + "crypto" "crypto/dsa" "crypto/ecdsa" "crypto/elliptic" @@ -550,7 +551,8 @@ type mockAuthority struct { getTLSOptions func() *tlsutil.TLSOptions root func(shasum string) (*x509.Certificate, error) sign func(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) ([]*x509.Certificate, error) - renew func(cert *x509.Certificate) ([]*x509.Certificate, error) + renew func(cert *x509.Certificate) ([]*x509.Certificate, error) + renewOrRekey func(oldCert *x509.Certificate, pk crypto.PublicKey) ([]*x509.Certificate, error) loadProvisionerByCertificate func(cert *x509.Certificate) (provisioner.Interface, error) loadProvisionerByID func(provID string) (provisioner.Interface, error) getProvisioners func(nextCursor string, limit int) (provisioner.List, string, error) @@ -611,6 +613,13 @@ func (m *mockAuthority) Renew(cert *x509.Certificate) ([]*x509.Certificate, erro return []*x509.Certificate{m.ret1.(*x509.Certificate), m.ret2.(*x509.Certificate)}, m.err } +func (m *mockAuthority) RenewOrRekey(oldcert *x509.Certificate, pk crypto.PublicKey) ([]*x509.Certificate, error) { + if m.renewOrRekey != nil { + return m.renewOrRekey(oldcert, pk) + } + return []*x509.Certificate{m.ret1.(*x509.Certificate), m.ret2.(*x509.Certificate)}, m.err +} + func (m *mockAuthority) GetProvisioners(nextCursor string, limit int) (provisioner.List, string, error) { if m.getProvisioners != nil { return m.getProvisioners(nextCursor, limit) @@ -952,6 +961,67 @@ func Test_caHandler_Renew(t *testing.T) { } } +func Test_caHandler_Rekey(t *testing.T) { + cs := &tls.ConnectionState{ + PeerCertificates: []*x509.Certificate{parseCertificate(certPEM)}, + } + csr := parseCertificateRequest(csrPEM) + valid, err := json.Marshal(RekeyRequest{ + CsrPEM: CertificateRequest{csr}, + }) + if err != nil { + t.Fatal(err) + } + tests := []struct { + name string + input string + tls *tls.ConnectionState + cert *x509.Certificate + root *x509.Certificate + err error + statusCode int + }{ + {"ok", string(valid), cs, parseCertificate(certPEM), parseCertificate(rootPEM), nil, http.StatusCreated}, + {"no tls", string(valid), nil, nil, nil, nil, http.StatusBadRequest}, + {"no peer certificates", string(valid), &tls.ConnectionState{}, nil, nil, nil, http.StatusBadRequest}, + {"rekey error", string(valid), cs, nil, nil, errs.Forbidden("an error"), http.StatusForbidden}, + {"json read error", "{", cs, nil, nil, nil, http.StatusBadRequest}, + } + + expected := []byte(`{"crt":"` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n","certChain":["` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"]}`) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := New(&mockAuthority{ + ret1: tt.cert, ret2: tt.root, err: tt.err, + getTLSOptions: func() *tlsutil.TLSOptions { + return nil + }, + }).(*caHandler) + req := httptest.NewRequest("POST", "http://example.com/rekey", strings.NewReader(tt.input)) + req.TLS = tt.tls + w := httptest.NewRecorder() + h.Rekey(logging.NewResponseLogger(w), req) + res := w.Result() + + if res.StatusCode != tt.statusCode { + t.Errorf("caHandler.Rekey StatusCode = %d, wants %d", res.StatusCode, tt.statusCode) + } + + body, err := ioutil.ReadAll(res.Body) + res.Body.Close() + if err != nil { + t.Errorf("caHandler.Rekey unexpected error = %v", err) + } + if tt.statusCode < http.StatusBadRequest { + if !bytes.Equal(bytes.TrimSpace(body), expected) { + t.Errorf("caHandler.Rekey Body = %s, wants %s", body, expected) + } + } + }) + } +} + func Test_caHandler_Provisioners(t *testing.T) { type fields struct { Authority Authority From c8c3581e2f8dcc9cc75021e541c64d7771ab2cdb Mon Sep 17 00:00:00 2001 From: dharanikumar-s Date: Sun, 5 Jul 2020 22:15:01 +0530 Subject: [PATCH 05/12] SubjectKeyIdentifier extention is calculated from public key passed to this function instead of copying from old certificate --- authority/tls.go | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/authority/tls.go b/authority/tls.go index 3ab15838..f208c0b5 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -4,7 +4,9 @@ import ( "context" "crypto" "crypto/tls" + "crypto/sha1" "crypto/x509" + "crypto/x509/pkix" "encoding/asn1" "encoding/base64" "encoding/pem" @@ -28,6 +30,7 @@ func (a *Authority) GetTLSOptions() *tlsutil.TLSOptions { } var oidAuthorityKeyIdentifier = asn1.ObjectIdentifier{2, 5, 29, 35} +var oidSubjectKeyIdentifier = asn1.ObjectIdentifier{2, 5, 29, 14} func withDefaultASN1DN(def *x509util.ASN1DN) x509util.WithOption { return func(p x509util.Profile) error { @@ -139,8 +142,8 @@ func (a *Authority) Renew(oldCert *x509.Certificate) ([]*x509.Certificate, error return a.RenewOrRekey(oldCert, oldCert.PublicKey) } -// Rekey is similar to renew except that the certificate will be renewed with new key. -// Function does rekeying if a new public key is passed as pk else if same key is passed, certificate will be just renewed. + +// Func is used for renewing or rekeying based on the public key passed. func (a *Authority) RenewOrRekey(oldCert *x509.Certificate, pk crypto.PublicKey) ([]*x509.Certificate, error) { opts := []interface{}{errs.WithKeyVal("serialNumber", oldCert.SerialNumber.String())} @@ -189,15 +192,26 @@ func (a *Authority) RenewOrRekey(oldCert *x509.Certificate, pk crypto.PublicKey) PolicyIdentifiers: oldCert.PolicyIdentifiers, } - // Copy all extensions except for Authority Key Identifier. This one might - // be different if we rotate the intermediate certificate and it will cause - // a TLS bad certificate error. + // Copy all extensions except: + // 1. Authority Key Identifier - This one might be different if we rotate the intermediate certificate + // and it will cause a TLS bad certificate error. + // 2. Subject Key Identifier - This should be calculated for the public key passed to this function. for _, ext := range oldCert.Extensions { - if !ext.Id.Equal(oidAuthorityKeyIdentifier) { + if ((!ext.Id.Equal(oidAuthorityKeyIdentifier)) && (!ext.Id.Equal(oidSubjectKeyIdentifier))) { newCert.ExtraExtensions = append(newCert.ExtraExtensions, ext) } + if ext.Id.Equal(oidSubjectKeyIdentifier) { + pubBytes, _ := x509.MarshalPKIXPublicKey(pk) + hash := sha1.Sum(pubBytes) + skiExtension := pkix.Extension{ + Id: oidSubjectKeyIdentifier, + Value: append([]byte{4,20}, hash[:]...), + } + newCert.ExtraExtensions = append(newCert.ExtraExtensions, skiExtension) + } } - + + leaf, err := x509util.NewLeafProfileWithTemplate(newCert, a.x509Issuer, a.x509Signer) if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.RenewOrRekey", opts...) @@ -205,7 +219,7 @@ func (a *Authority) RenewOrRekey(oldCert *x509.Certificate, pk crypto.PublicKey) crtBytes, err := leaf.CreateCertificate() if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, - "authority.RenewOrRenew; error renewing certificate from existing server certificate", opts...) + "authority.RenewOrRekey; error renewing certificate from existing server certificate", opts...) } serverCert, err := x509.ParseCertificate(crtBytes) From b368a5314941c1a853261e1096f99d91fab15050 Mon Sep 17 00:00:00 2001 From: dharanikumar-s Date: Sun, 5 Jul 2020 22:17:57 +0530 Subject: [PATCH 06/12] Modified TestAuthority_Renew to TestAuthority_RenewOrRekey --- authority/tls_test.go | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/authority/tls_test.go b/authority/tls_test.go index 183c3083..b94a6a74 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -370,8 +370,9 @@ ZYtQ9Ot36qc= } } -func TestAuthority_Renew(t *testing.T) { +func TestAuthority_RenewOrRekey(t *testing.T) { pub, _, err := keys.GenerateDefaultKeyPair() + pub1, _, err := keys.GenerateDefaultKeyPair() assert.FatalError(t, err) a := testAuthority(t) @@ -428,14 +429,14 @@ func TestAuthority_Renew(t *testing.T) { return &renewTest{ auth: _a, cert: cert, - err: errors.New("authority.Renew; error renewing certificate from existing server certificate"), + err: errors.New("authority.RenewOrRekey; error renewing certificate from existing server certificate"), code: http.StatusInternalServerError, }, nil }, "fail-unauthorized": func() (*renewTest, error) { return &renewTest{ cert: certNoRenew, - err: errors.New("authority.Renew: authority.authorizeRenew: jwk.AuthorizeRenew; renew is disabled for jwk provisioner dev:IMi94WBNI6gP5cNHXlZYNUzvMjGdHyBRmFoo-lCEaqk"), + err: errors.New("authority.RenewOrRekey: authority.authorizeRenew: jwk.AuthorizeRenew; renew is disabled for jwk provisioner dev:IMi94WBNI6gP5cNHXlZYNUzvMjGdHyBRmFoo-lCEaqk"), code: http.StatusUnauthorized, }, nil }, @@ -478,9 +479,9 @@ func TestAuthority_Renew(t *testing.T) { var certChain []*x509.Certificate if tc.auth != nil { - certChain, err = tc.auth.Renew(tc.cert) + certChain, err = tc.auth.RenewOrRekey(tc.cert,pub1) } else { - certChain, err = a.Renew(tc.cert) + certChain, err = a.RenewOrRekey(tc.cert,pub1) } if err != nil { if assert.NotNil(t, tc.err, fmt.Sprintf("unexpected error: %s", err)) { @@ -524,8 +525,9 @@ func TestAuthority_Renew(t *testing.T) { assert.Equals(t, leaf.ExtKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}) assert.Equals(t, leaf.DNSNames, []string{"test.smallstep.com", "test"}) + assert.Equals(t, leaf.PublicKey, pub1) - pubBytes, err := x509.MarshalPKIXPublicKey(pub) + pubBytes, err := x509.MarshalPKIXPublicKey(pub1) assert.FatalError(t, err) hash := sha1.Sum(pubBytes) assert.Equals(t, leaf.SubjectKeyId, hash[:]) @@ -535,6 +537,10 @@ func TestAuthority_Renew(t *testing.T) { assert.Equals(t, leaf.AuthorityKeyId, a.x509Issuer.SubjectKeyId) // Compare extensions: they can be in a different order for _, ext1 := range tc.cert.Extensions { + //skip SubjectKeyIdentifier + if ext1.Id.Equal(oidSubjectKeyIdentifier) { + continue + } found := false for _, ext2 := range leaf.Extensions { if reflect.DeepEqual(ext1, ext2) { @@ -551,6 +557,10 @@ func TestAuthority_Renew(t *testing.T) { assert.Equals(t, leaf.AuthorityKeyId, tc.auth.x509Issuer.SubjectKeyId) // Compare extensions: they can be in a different order for _, ext1 := range tc.cert.Extensions { + //skip SubjectKeyIdentifier + if ext1.Id.Equal(oidSubjectKeyIdentifier) { + continue + } // The authority key id extension should be different b/c the intermediates are different. if ext1.Id.Equal(oidAuthorityKeyIdentifier) { for _, ext2 := range leaf.Extensions { From 2479371c0604947102dffd7e910c7cf00901233d Mon Sep 17 00:00:00 2001 From: dharanikumar-s Date: Sun, 5 Jul 2020 22:37:29 +0530 Subject: [PATCH 07/12] Added error check while marshalling public key --- authority/tls.go | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/authority/tls.go b/authority/tls.go index f208c0b5..c06dc374 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -3,8 +3,8 @@ package authority import ( "context" "crypto" - "crypto/tls" "crypto/sha1" + "crypto/tls" "crypto/x509" "crypto/x509/pkix" "encoding/asn1" @@ -142,8 +142,7 @@ func (a *Authority) Renew(oldCert *x509.Certificate) ([]*x509.Certificate, error return a.RenewOrRekey(oldCert, oldCert.PublicKey) } - -// Func is used for renewing or rekeying based on the public key passed. +// Func is used for renewing or rekeying based on the public key passed. func (a *Authority) RenewOrRekey(oldCert *x509.Certificate, pk crypto.PublicKey) ([]*x509.Certificate, error) { opts := []interface{}{errs.WithKeyVal("serialNumber", oldCert.SerialNumber.String())} @@ -152,13 +151,11 @@ func (a *Authority) RenewOrRekey(oldCert *x509.Certificate, pk crypto.PublicKey) return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.RenewOrRekey", opts...) } - // Durations backdate := a.config.AuthorityConfig.Backdate.Duration duration := oldCert.NotAfter.Sub(oldCert.NotBefore) now := time.Now().UTC() - newCert := &x509.Certificate{ PublicKey: pk, Issuer: a.x509Issuer.Subject, @@ -193,25 +190,28 @@ func (a *Authority) RenewOrRekey(oldCert *x509.Certificate, pk crypto.PublicKey) } // Copy all extensions except: - // 1. Authority Key Identifier - This one might be different if we rotate the intermediate certificate + // 1. Authority Key Identifier - This one might be different if we rotate the intermediate certificate // and it will cause a TLS bad certificate error. // 2. Subject Key Identifier - This should be calculated for the public key passed to this function. for _, ext := range oldCert.Extensions { - if ((!ext.Id.Equal(oidAuthorityKeyIdentifier)) && (!ext.Id.Equal(oidSubjectKeyIdentifier))) { + if (!ext.Id.Equal(oidAuthorityKeyIdentifier)) && (!ext.Id.Equal(oidSubjectKeyIdentifier)) { newCert.ExtraExtensions = append(newCert.ExtraExtensions, ext) } if ext.Id.Equal(oidSubjectKeyIdentifier) { - pubBytes, _ := x509.MarshalPKIXPublicKey(pk) + pubBytes, err := x509.MarshalPKIXPublicKey(pk) + if err != nil { + return nil, errs.Wrap(http.StatusInternalServerError, err, + "authority.RenewOrRekey; error marshalling public key", opts...) + } hash := sha1.Sum(pubBytes) skiExtension := pkix.Extension{ - Id: oidSubjectKeyIdentifier, - Value: append([]byte{4,20}, hash[:]...), + Id: oidSubjectKeyIdentifier, + Value: append([]byte{4, 20}, hash[:]...), } newCert.ExtraExtensions = append(newCert.ExtraExtensions, skiExtension) } } - - + leaf, err := x509util.NewLeafProfileWithTemplate(newCert, a.x509Issuer, a.x509Signer) if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.RenewOrRekey", opts...) @@ -237,7 +237,6 @@ func (a *Authority) RenewOrRekey(oldCert *x509.Certificate, pk crypto.PublicKey) return []*x509.Certificate{serverCert, a.x509Issuer}, nil } - // RevokeOptions are the options for the Revoke API. type RevokeOptions struct { Serial string From 0c21f0ae9e2a44ec6a740255981ecd795429b8b7 Mon Sep 17 00:00:00 2001 From: dharanikumar-s Date: Sun, 5 Jul 2020 22:38:45 +0530 Subject: [PATCH 08/12] Added error check after GenerateDefaultKeyPair --- authority/tls_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/authority/tls_test.go b/authority/tls_test.go index b94a6a74..e12ff266 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -372,6 +372,7 @@ ZYtQ9Ot36qc= func TestAuthority_RenewOrRekey(t *testing.T) { pub, _, err := keys.GenerateDefaultKeyPair() + assert.FatalError(t, err) pub1, _, err := keys.GenerateDefaultKeyPair() assert.FatalError(t, err) @@ -479,9 +480,9 @@ func TestAuthority_RenewOrRekey(t *testing.T) { var certChain []*x509.Certificate if tc.auth != nil { - certChain, err = tc.auth.RenewOrRekey(tc.cert,pub1) + certChain, err = tc.auth.RenewOrRekey(tc.cert, pub1) } else { - certChain, err = a.RenewOrRekey(tc.cert,pub1) + certChain, err = a.RenewOrRekey(tc.cert, pub1) } if err != nil { if assert.NotNil(t, tc.err, fmt.Sprintf("unexpected error: %s", err)) { @@ -525,7 +526,7 @@ func TestAuthority_RenewOrRekey(t *testing.T) { assert.Equals(t, leaf.ExtKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}) assert.Equals(t, leaf.DNSNames, []string{"test.smallstep.com", "test"}) - assert.Equals(t, leaf.PublicKey, pub1) + assert.Equals(t, leaf.PublicKey, pub1) pubBytes, err := x509.MarshalPKIXPublicKey(pub1) assert.FatalError(t, err) From a3b5211e0f59fc7d8e7bc102c6285138b8af600f Mon Sep 17 00:00:00 2001 From: dharanikumar-s Date: Sun, 5 Jul 2020 22:40:36 +0530 Subject: [PATCH 09/12] gofmted the code --- api/api_test.go | 2 +- api/rekey.go | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/api/api_test.go b/api/api_test.go index 8350a41d..3e79971e 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -551,7 +551,7 @@ type mockAuthority struct { getTLSOptions func() *tlsutil.TLSOptions root func(shasum string) (*x509.Certificate, error) sign func(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) ([]*x509.Certificate, error) - renew func(cert *x509.Certificate) ([]*x509.Certificate, error) + renew func(cert *x509.Certificate) ([]*x509.Certificate, error) renewOrRekey func(oldCert *x509.Certificate, pk crypto.PublicKey) ([]*x509.Certificate, error) loadProvisionerByCertificate func(cert *x509.Certificate) (provisioner.Interface, error) loadProvisionerByID func(provID string) (provisioner.Interface, error) diff --git a/api/rekey.go b/api/rekey.go index 1638b56e..b175ff9b 100644 --- a/api/rekey.go +++ b/api/rekey.go @@ -8,7 +8,7 @@ import ( // RekeyRequest is the request body for a certificate rekey request. type RekeyRequest struct { - CsrPEM CertificateRequest `json:"csr"` + CsrPEM CertificateRequest `json:"csr"` } // Validate checks the fields of the RekeyRequest and returns nil if they are ok @@ -24,7 +24,6 @@ func (s *RekeyRequest) Validate() error { return nil } - // Rekey is similar to renew except that the certificate will be renewed with new key from csr. func (h *caHandler) Rekey(w http.ResponseWriter, r *http.Request) { @@ -39,13 +38,12 @@ func (h *caHandler) Rekey(w http.ResponseWriter, r *http.Request) { return } - if err := body.Validate(); err != nil { WriteError(w, err) return } - certChain, err := h.Authority.RenewOrRekey(r.TLS.PeerCertificates[0],body.CsrPEM.CertificateRequest.PublicKey) + certChain, err := h.Authority.RenewOrRekey(r.TLS.PeerCertificates[0], body.CsrPEM.CertificateRequest.PublicKey) if err != nil { WriteError(w, errs.Wrap(http.StatusInternalServerError, err, "cahandler.Rekey")) return From fe73154a20f5655f55ce9f15e04d37986bb4cbaa Mon Sep 17 00:00:00 2001 From: dharanikumar-s Date: Sun, 5 Jul 2020 22:50:02 +0530 Subject: [PATCH 10/12] Corrected misspelling --- authority/tls.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/authority/tls.go b/authority/tls.go index c06dc374..aa26ee01 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -201,7 +201,7 @@ func (a *Authority) RenewOrRekey(oldCert *x509.Certificate, pk crypto.PublicKey) pubBytes, err := x509.MarshalPKIXPublicKey(pk) if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, - "authority.RenewOrRekey; error marshalling public key", opts...) + "authority.RenewOrRekey; error marshaling public key", opts...) } hash := sha1.Sum(pubBytes) skiExtension := pkix.Extension{ From dfda4979290512cbeb33699967c194b8c69b11fb Mon Sep 17 00:00:00 2001 From: dharanikumar-s Date: Wed, 8 Jul 2020 11:47:59 +0530 Subject: [PATCH 11/12] Renamed RenewOrRekey to Rekey --- api/api.go | 2 +- api/api_test.go | 2 +- api/rekey.go | 2 +- authority/tls.go | 16 ++++++++-------- authority/tls_test.go | 10 +++++----- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/api/api.go b/api/api.go index 57c81262..f83a2354 100644 --- a/api/api.go +++ b/api/api.go @@ -36,7 +36,7 @@ type Authority interface { Root(shasum string) (*x509.Certificate, error) Sign(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) ([]*x509.Certificate, error) Renew(peer *x509.Certificate) ([]*x509.Certificate, error) - RenewOrRekey(peer *x509.Certificate, pk crypto.PublicKey) ([]*x509.Certificate, error) + Rekey(peer *x509.Certificate, pk crypto.PublicKey) ([]*x509.Certificate, error) LoadProvisionerByCertificate(*x509.Certificate) (provisioner.Interface, error) LoadProvisionerByID(string) (provisioner.Interface, error) GetProvisioners(cursor string, limit int) (provisioner.List, string, error) diff --git a/api/api_test.go b/api/api_test.go index 3e79971e..27aec1b1 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -613,7 +613,7 @@ func (m *mockAuthority) Renew(cert *x509.Certificate) ([]*x509.Certificate, erro return []*x509.Certificate{m.ret1.(*x509.Certificate), m.ret2.(*x509.Certificate)}, m.err } -func (m *mockAuthority) RenewOrRekey(oldcert *x509.Certificate, pk crypto.PublicKey) ([]*x509.Certificate, error) { +func (m *mockAuthority) Rekey(oldcert *x509.Certificate, pk crypto.PublicKey) ([]*x509.Certificate, error) { if m.renewOrRekey != nil { return m.renewOrRekey(oldcert, pk) } diff --git a/api/rekey.go b/api/rekey.go index b175ff9b..2d24dbb8 100644 --- a/api/rekey.go +++ b/api/rekey.go @@ -43,7 +43,7 @@ func (h *caHandler) Rekey(w http.ResponseWriter, r *http.Request) { return } - certChain, err := h.Authority.RenewOrRekey(r.TLS.PeerCertificates[0], body.CsrPEM.CertificateRequest.PublicKey) + certChain, err := h.Authority.Rekey(r.TLS.PeerCertificates[0], body.CsrPEM.CertificateRequest.PublicKey) if err != nil { WriteError(w, errs.Wrap(http.StatusInternalServerError, err, "cahandler.Rekey")) return diff --git a/authority/tls.go b/authority/tls.go index aa26ee01..0b38ecb7 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -139,16 +139,16 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Opti // 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) { - return a.RenewOrRekey(oldCert, oldCert.PublicKey) + return a.Rekey(oldCert, oldCert.PublicKey) } // Func is used for renewing or rekeying based on the public key passed. -func (a *Authority) RenewOrRekey(oldCert *x509.Certificate, pk crypto.PublicKey) ([]*x509.Certificate, error) { +func (a *Authority) Rekey(oldCert *x509.Certificate, pk crypto.PublicKey) ([]*x509.Certificate, error) { opts := []interface{}{errs.WithKeyVal("serialNumber", oldCert.SerialNumber.String())} // Check step provisioner extensions if err := a.authorizeRenew(oldCert); err != nil { - return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.RenewOrRekey", opts...) + return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.Rekey", opts...) } // Durations @@ -201,7 +201,7 @@ func (a *Authority) RenewOrRekey(oldCert *x509.Certificate, pk crypto.PublicKey) pubBytes, err := x509.MarshalPKIXPublicKey(pk) if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, - "authority.RenewOrRekey; error marshaling public key", opts...) + "authority.Rekey; error marshaling public key", opts...) } hash := sha1.Sum(pubBytes) skiExtension := pkix.Extension{ @@ -214,23 +214,23 @@ func (a *Authority) RenewOrRekey(oldCert *x509.Certificate, pk crypto.PublicKey) leaf, err := x509util.NewLeafProfileWithTemplate(newCert, a.x509Issuer, a.x509Signer) if err != nil { - return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.RenewOrRekey", opts...) + return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.Rekey", opts...) } crtBytes, err := leaf.CreateCertificate() if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, - "authority.RenewOrRekey; error renewing certificate from existing server certificate", opts...) + "authority.Rekey; error renewing certificate from existing server certificate", opts...) } serverCert, err := x509.ParseCertificate(crtBytes) if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, - "authority.RenewOrRekey; error parsing new server certificate", opts...) + "authority.Rekey; error parsing new server certificate", opts...) } if err = a.db.StoreCertificate(serverCert); err != nil { if err != db.ErrNotImplemented { - return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.RenewOrRekey; error storing certificate in db", opts...) + return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.Rekey; error storing certificate in db", opts...) } } diff --git a/authority/tls_test.go b/authority/tls_test.go index e12ff266..7c9caedc 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -370,7 +370,7 @@ ZYtQ9Ot36qc= } } -func TestAuthority_RenewOrRekey(t *testing.T) { +func TestAuthority_Rekey(t *testing.T) { pub, _, err := keys.GenerateDefaultKeyPair() assert.FatalError(t, err) pub1, _, err := keys.GenerateDefaultKeyPair() @@ -430,14 +430,14 @@ func TestAuthority_RenewOrRekey(t *testing.T) { return &renewTest{ auth: _a, cert: cert, - err: errors.New("authority.RenewOrRekey; error renewing certificate from existing server certificate"), + err: errors.New("authority.Rekey; error renewing certificate from existing server certificate"), code: http.StatusInternalServerError, }, nil }, "fail-unauthorized": func() (*renewTest, error) { return &renewTest{ cert: certNoRenew, - err: errors.New("authority.RenewOrRekey: authority.authorizeRenew: jwk.AuthorizeRenew; renew is disabled for jwk provisioner dev:IMi94WBNI6gP5cNHXlZYNUzvMjGdHyBRmFoo-lCEaqk"), + err: errors.New("authority.Rekey: authority.authorizeRenew: jwk.AuthorizeRenew; renew is disabled for jwk provisioner dev:IMi94WBNI6gP5cNHXlZYNUzvMjGdHyBRmFoo-lCEaqk"), code: http.StatusUnauthorized, }, nil }, @@ -480,9 +480,9 @@ func TestAuthority_RenewOrRekey(t *testing.T) { var certChain []*x509.Certificate if tc.auth != nil { - certChain, err = tc.auth.RenewOrRekey(tc.cert, pub1) + certChain, err = tc.auth.Rekey(tc.cert, pub1) } else { - certChain, err = a.RenewOrRekey(tc.cert, pub1) + certChain, err = a.Rekey(tc.cert, pub1) } if err != nil { if assert.NotNil(t, tc.err, fmt.Sprintf("unexpected error: %s", err)) { From 57fb0c80cfa800d6278c58ac912e9b54a8a1900d Mon Sep 17 00:00:00 2001 From: dharanikumar-s Date: Wed, 8 Jul 2020 12:52:53 +0530 Subject: [PATCH 12/12] Removed calculating SubjectKeyIdentifier on Rekey --- authority/tls.go | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/authority/tls.go b/authority/tls.go index 0b38ecb7..f752a45c 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -3,10 +3,8 @@ package authority import ( "context" "crypto" - "crypto/sha1" "crypto/tls" "crypto/x509" - "crypto/x509/pkix" "encoding/asn1" "encoding/base64" "encoding/pem" @@ -139,7 +137,7 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Opti // 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) { - return a.Rekey(oldCert, oldCert.PublicKey) + return a.Rekey(oldCert, nil) } // Func is used for renewing or rekeying based on the public key passed. @@ -157,7 +155,6 @@ func (a *Authority) Rekey(oldCert *x509.Certificate, pk crypto.PublicKey) ([]*x5 now := time.Now().UTC() newCert := &x509.Certificate{ - PublicKey: pk, Issuer: a.x509Issuer.Subject, Subject: oldCert.Subject, NotBefore: now.Add(-1 * backdate), @@ -189,27 +186,26 @@ func (a *Authority) Rekey(oldCert *x509.Certificate, pk crypto.PublicKey) ([]*x5 PolicyIdentifiers: oldCert.PolicyIdentifiers, } + if pk == nil { + newCert.PublicKey = oldCert.PublicKey + } else { + newCert.PublicKey = pk + } + // Copy all extensions except: // 1. Authority Key Identifier - This one might be different if we rotate the intermediate certificate // and it will cause a TLS bad certificate error. - // 2. Subject Key Identifier - This should be calculated for the public key passed to this function. + // 2. Subject Key Identifier, if rekey - For rekey, SubjectKeyIdentifier extension will be calculated + // for the new public key by NewLeafProfilewithTemplate() for _, ext := range oldCert.Extensions { - if (!ext.Id.Equal(oidAuthorityKeyIdentifier)) && (!ext.Id.Equal(oidSubjectKeyIdentifier)) { - newCert.ExtraExtensions = append(newCert.ExtraExtensions, ext) + if ext.Id.Equal(oidAuthorityKeyIdentifier) { + continue } - if ext.Id.Equal(oidSubjectKeyIdentifier) { - pubBytes, err := x509.MarshalPKIXPublicKey(pk) - if err != nil { - return nil, errs.Wrap(http.StatusInternalServerError, err, - "authority.Rekey; error marshaling public key", opts...) - } - hash := sha1.Sum(pubBytes) - skiExtension := pkix.Extension{ - Id: oidSubjectKeyIdentifier, - Value: append([]byte{4, 20}, hash[:]...), - } - newCert.ExtraExtensions = append(newCert.ExtraExtensions, skiExtension) + if ext.Id.Equal(oidSubjectKeyIdentifier) && (pk != nil) { + newCert.SubjectKeyId = nil + continue } + newCert.ExtraExtensions = append(newCert.ExtraExtensions, ext) } leaf, err := x509util.NewLeafProfileWithTemplate(newCert, a.x509Issuer, a.x509Signer)