diff --git a/Gopkg.lock b/Gopkg.lock index a11d0292..92e72495 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -276,8 +276,8 @@ revision = "de77670473b5492f5d0bce155b5c01534c2d13f7" [[projects]] - branch = "master" - digest = "1:a11fa27b1cebc2aa3650bd2086aeadf1e2aaf1f4b646895893b80260b17a19d2" + branch = "x509util-real-x509" + digest = "1:8b36444f30009b5e124a3ac48b353558024a95c3fccdf3e6bb557a091e67342b" name = "github.com/smallstep/cli" packages = [ "command", @@ -298,7 +298,7 @@ "utils", ] pruneopts = "UT" - revision = "68ac9850f47f4cfe0045f1444f3f23404e2237ba" + revision = "de740bc707a264406a455c405238788d1f9f0666" [[projects]] branch = "master" @@ -570,7 +570,6 @@ "github.com/smallstep/cli/crypto/x509util", "github.com/smallstep/cli/errs", "github.com/smallstep/cli/jose", - "github.com/smallstep/cli/pkg/x509", "github.com/smallstep/cli/token", "github.com/smallstep/cli/token/provision", "github.com/smallstep/cli/usage", diff --git a/Gopkg.toml b/Gopkg.toml index 0e564d1f..a0e088a0 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -45,7 +45,7 @@ required = [ name = "github.com/go-chi/chi" [[override]] - branch = "master" + branch = "x509util-real-x509" name = "github.com/smallstep/cli" [prune] diff --git a/authority/tls.go b/authority/tls.go index c52ac1e8..0b089194 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -14,7 +14,6 @@ import ( "github.com/smallstep/cli/crypto/pemutil" "github.com/smallstep/cli/crypto/tlsutil" "github.com/smallstep/cli/crypto/x509util" - stepx509 "github.com/smallstep/cli/pkg/x509" ) // GetTLSOptions returns the tls options configured. @@ -60,6 +59,7 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Opti errContext = context{"csr": csr, "signOptions": signOpts} mods = []x509util.WithOption{withDefaultASN1DN(a.config.AuthorityConfig.Template)} certValidators = []provisioner.CertificateValidator{} + issIdentity = a.intermediateIdentity ) for _, op := range extraOpts { switch k := op.(type) { @@ -77,19 +77,22 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Opti } } - stepCSR, err := stepx509.ParseCertificateRequest(csr.Raw) - if err != nil { - return nil, nil, &apiError{errors.Wrap(err, "sign: error converting x509 csr to stepx509 csr"), - http.StatusInternalServerError, errContext} + if err := csr.CheckSignature(); err != nil { + return nil, nil, &apiError{errors.Wrap(err, "sign: invalid certificate request"), + http.StatusBadRequest, errContext} } - issIdentity := a.intermediateIdentity - leaf, err := x509util.NewLeafProfileWithCSR(stepCSR, issIdentity.Crt, - issIdentity.Key, mods...) + leaf, err := x509util.NewLeafProfileWithCSR(csr, issIdentity.Crt, issIdentity.Key, mods...) if err != nil { return nil, nil, &apiError{errors.Wrapf(err, "sign"), http.StatusInternalServerError, errContext} } + for _, v := range certValidators { + if err := v.Valid(leaf.Subject()); err != nil { + return nil, nil, &apiError{errors.Wrap(err, "sign"), http.StatusUnauthorized, errContext} + } + } + crtBytes, err := leaf.CreateCertificate() if err != nil { return nil, nil, &apiError{errors.Wrap(err, "sign: error creating new leaf certificate"), @@ -102,13 +105,6 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Opti http.StatusInternalServerError, errContext} } - // FIXME: This should be before creating the certificate. - for _, v := range certValidators { - if err := v.Valid(serverCert); err != nil { - return nil, nil, &apiError{errors.Wrap(err, "sign"), http.StatusUnauthorized, errContext} - } - } - caCert, err := x509.ParseCertificate(issIdentity.Crt.Raw) if err != nil { return nil, nil, &apiError{errors.Wrap(err, "sign: error parsing intermediate certificate"), @@ -120,27 +116,18 @@ 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(ocx *x509.Certificate) (*x509.Certificate, *x509.Certificate, error) { +func (a *Authority) Renew(oldCert *x509.Certificate) (*x509.Certificate, *x509.Certificate, error) { // Check step provisioner extensions - if err := a.authorizeRenewal(ocx); err != nil { + if err := a.authorizeRenewal(oldCert); err != nil { return nil, nil, err } // Issuer issIdentity := a.intermediateIdentity - // Convert a realx509.Certificate to the step x509 Certificate. - oldCert, err := stepx509.ParseCertificate(ocx.Raw) - if err != nil { - return nil, nil, &apiError{ - errors.Wrap(err, "error converting x509.Certificate to stepx509.Certificate"), - http.StatusInternalServerError, context{}, - } - } - now := time.Now().UTC() duration := oldCert.NotAfter.Sub(oldCert.NotBefore) - newCert := &stepx509.Certificate{ + newCert := &x509.Certificate{ PublicKey: oldCert.PublicKey, Issuer: issIdentity.Crt.Subject, Subject: oldCert.Subject, diff --git a/authority/tls_test.go b/authority/tls_test.go index eb1793e2..e64881d9 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -19,7 +19,6 @@ import ( "github.com/smallstep/cli/crypto/tlsutil" "github.com/smallstep/cli/crypto/x509util" "github.com/smallstep/cli/jose" - stepx509 "github.com/smallstep/cli/pkg/x509" ) var ( @@ -110,6 +109,20 @@ func TestSign(t *testing.T) { err *apiError } tests := map[string]func(*testing.T) *signTest{ + "fail invalid signature": func(t *testing.T) *signTest { + csr := getCSR(t, priv) + csr.Signature = []byte("foo") + return &signTest{ + auth: a, + csr: csr, + extraOpts: extraOpts, + signOpts: signOpts, + err: &apiError{errors.New("sign: invalid certificate request"), + http.StatusBadRequest, + context{"csr": csr, "signOptions": signOpts}, + }, + } + }, "fail invalid extra option": func(t *testing.T) *signTest { csr := getCSR(t, priv) csr.Raw = []byte("foo") @@ -124,20 +137,6 @@ func TestSign(t *testing.T) { }, } }, - "fail convert csr to step format": func(t *testing.T) *signTest { - csr := getCSR(t, priv) - csr.Raw = []byte("foo") - return &signTest{ - auth: a, - csr: csr, - extraOpts: extraOpts, - signOpts: signOpts, - err: &apiError{errors.New("sign: error converting x509 csr to stepx509 csr"), - http.StatusInternalServerError, - context{"csr": csr, "signOptions": signOpts}, - }, - } - }, "fail merge default ASN1DN": func(t *testing.T) *signTest { _a := testAuthority(t) _a.config.AuthorityConfig.Template = nil @@ -335,13 +334,6 @@ func TestRenew(t *testing.T) { err *apiError } tests := map[string]func() (*renewTest, error){ - "fail-conversion-stepx509": func() (*renewTest, error) { - return &renewTest{ - crt: &x509.Certificate{Raw: []byte("foo")}, - err: &apiError{errors.New("error converting x509.Certificate to stepx509.Certificate"), - http.StatusInternalServerError, context{}}, - }, nil - }, "fail-create-cert": func() (*renewTest, error) { _a := testAuthority(t) _a.intermediateIdentity.Key = nil @@ -373,7 +365,7 @@ func TestRenew(t *testing.T) { assert.FatalError(t, err) newRootBytes, err := newRootProfile.CreateCertificate() assert.FatalError(t, err) - newRootCrt, err := stepx509.ParseCertificate(newRootBytes) + newRootCrt, err := x509.ParseCertificate(newRootBytes) assert.FatalError(t, err) newIntermediateProfile, err := x509util.NewIntermediateProfile("new-intermediate", @@ -381,7 +373,7 @@ func TestRenew(t *testing.T) { assert.FatalError(t, err) newIntermediateBytes, err := newIntermediateProfile.CreateCertificate() assert.FatalError(t, err) - newIntermediateCrt, err := stepx509.ParseCertificate(newIntermediateBytes) + newIntermediateCrt, err := x509.ParseCertificate(newIntermediateBytes) assert.FatalError(t, err) _a := testAuthority(t)