bug fix: don't add common name to CSR validation claims in Sign

* added unit test for this case
This commit is contained in:
max furman 2019-02-06 16:26:25 -08:00
parent 47228cd9a0
commit 7e43402575
3 changed files with 34 additions and 10 deletions

2
Gopkg.lock generated
View file

@ -231,7 +231,7 @@
"utils", "utils",
] ]
pruneopts = "UT" pruneopts = "UT"
revision = "b2f4880e0fedde339530d29a2004c5bed4ecbf62" revision = "509daab78ae73d3c6bf37ae3a16abe5c4b0a9d94"
[[projects]] [[projects]]
branch = "master" branch = "master"

View file

@ -114,7 +114,6 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts SignOptions, ext
} }
mods = append(mods, m...) mods = append(mods, m...)
mods = append(mods, []x509util.WithOption{ mods = append(mods, []x509util.WithOption{
x509util.WithHosts(csr.Subject.CommonName),
withDefaultASN1DN(a.config.AuthorityConfig.Template), withDefaultASN1DN(a.config.AuthorityConfig.Template),
}...) }...)
claims = append(claims, c...) claims = append(claims, c...)

View file

@ -7,6 +7,7 @@ import (
"crypto/x509/pkix" "crypto/x509/pkix"
"encoding/asn1" "encoding/asn1"
"fmt" "fmt"
"net"
"net/http" "net/http"
"testing" "testing"
"time" "time"
@ -18,11 +19,14 @@ import (
"github.com/smallstep/cli/crypto/x509util" "github.com/smallstep/cli/crypto/x509util"
) )
func getCSR(t *testing.T, priv interface{}) *x509.CertificateRequest { func getCSR(t *testing.T, priv interface{}, opts ...func(*x509.CertificateRequest)) *x509.CertificateRequest {
_csr := &x509.CertificateRequest{ _csr := &x509.CertificateRequest{
Subject: pkix.Name{CommonName: "test.smallstep.com"}, Subject: pkix.Name{CommonName: "smallstep test"},
DNSNames: []string{"test.smallstep.com"}, DNSNames: []string{"test.smallstep.com"},
} }
for _, opt := range opts {
opt(_csr)
}
csrBytes, err := x509.CreateCertificateRequest(rand.Reader, _csr, priv) csrBytes, err := x509.CreateCertificateRequest(rand.Reader, _csr, priv)
assert.FatalError(t, err) assert.FatalError(t, err)
csr, err := x509.ParseCertificateRequest(csrBytes) csr, err := x509.ParseCertificateRequest(csrBytes)
@ -52,6 +56,12 @@ func TestSign(t *testing.T) {
} }
p := a.config.AuthorityConfig.Provisioners[1] p := a.config.AuthorityConfig.Provisioners[1]
extraOpts := []interface{}{
&commonNameClaim{"smallstep test"},
&dnsNamesClaim{[]string{"test.smallstep.com"}},
&ipAddressesClaim{[]net.IP{}},
p,
}
type signTest struct { type signTest struct {
auth *Authority auth *Authority
@ -67,7 +77,7 @@ func TestSign(t *testing.T) {
return &signTest{ return &signTest{
auth: a, auth: a,
csr: csr, csr: csr,
extraOpts: []interface{}{p, "42"}, extraOpts: append(extraOpts, "42"),
signOpts: signOpts, signOpts: signOpts,
err: &apiError{errors.New("sign: invalid extra option type string"), err: &apiError{errors.New("sign: invalid extra option type string"),
http.StatusInternalServerError, http.StatusInternalServerError,
@ -81,7 +91,7 @@ func TestSign(t *testing.T) {
return &signTest{ return &signTest{
auth: a, auth: a,
csr: csr, csr: csr,
extraOpts: []interface{}{p}, extraOpts: extraOpts,
signOpts: signOpts, signOpts: signOpts,
err: &apiError{errors.New("sign: error converting x509 csr to stepx509 csr"), err: &apiError{errors.New("sign: error converting x509 csr to stepx509 csr"),
http.StatusInternalServerError, http.StatusInternalServerError,
@ -96,7 +106,7 @@ func TestSign(t *testing.T) {
return &signTest{ return &signTest{
auth: _a, auth: _a,
csr: csr, csr: csr,
extraOpts: []interface{}{p}, extraOpts: extraOpts,
signOpts: signOpts, signOpts: signOpts,
err: &apiError{errors.New("sign: default ASN1DN template cannot be nil"), err: &apiError{errors.New("sign: default ASN1DN template cannot be nil"),
http.StatusInternalServerError, http.StatusInternalServerError,
@ -128,7 +138,7 @@ func TestSign(t *testing.T) {
return &signTest{ return &signTest{
auth: a, auth: a,
csr: csr, csr: csr,
extraOpts: []interface{}{p}, extraOpts: extraOpts,
signOpts: _signOpts, signOpts: _signOpts,
err: &apiError{errors.New("sign: requested duration of 25h0m0s is more than the authorized maximum certificate duration of 24h0m0s"), err: &apiError{errors.New("sign: requested duration of 25h0m0s is more than the authorized maximum certificate duration of 24h0m0s"),
http.StatusUnauthorized, http.StatusUnauthorized,
@ -136,12 +146,27 @@ func TestSign(t *testing.T) {
}, },
} }
}, },
"fail validate sans when adding common name not in claims": func(t *testing.T) *signTest {
csr := getCSR(t, priv, func(csr *x509.CertificateRequest) {
csr.DNSNames = append(csr.DNSNames, csr.Subject.CommonName)
})
return &signTest{
auth: a,
csr: csr,
extraOpts: extraOpts,
signOpts: signOpts,
err: &apiError{errors.New("sign: DNS names claim failed - got [test.smallstep.com smallstep test], want [test.smallstep.com]"),
http.StatusUnauthorized,
context{"csr": csr, "signOptions": signOpts},
},
}
},
"ok": func(t *testing.T) *signTest { "ok": func(t *testing.T) *signTest {
csr := getCSR(t, priv) csr := getCSR(t, priv)
return &signTest{ return &signTest{
auth: a, auth: a,
csr: csr, csr: csr,
extraOpts: []interface{}{p}, extraOpts: extraOpts,
signOpts: signOpts, signOpts: signOpts,
} }
}, },
@ -175,7 +200,7 @@ func TestSign(t *testing.T) {
Locality: []string{tmplt.Locality}, Locality: []string{tmplt.Locality},
StreetAddress: []string{tmplt.StreetAddress}, StreetAddress: []string{tmplt.StreetAddress},
Province: []string{tmplt.Province}, Province: []string{tmplt.Province},
CommonName: tmplt.CommonName, CommonName: "smallstep test",
})) }))
assert.Equals(t, leaf.Issuer, intermediate.Subject) assert.Equals(t, leaf.Issuer, intermediate.Subject)