Skip to content

Commit

Permalink
Various PKI updates (#3953)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jefferai
jefferai authored Feb 10, 2018
1 parent e045cb1 commit 07f8ebb
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 42 deletions.
60 changes: 28 additions & 32 deletions builtin/logical/pki/backend_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
logicaltest "github.com/hashicorp/vault/logical/testing"
"github.com/hashicorp/vault/vault"
"github.com/mitchellh/mapstructure"
"golang.org/x/net/idna"
)

var (
Expand Down Expand Up @@ -153,8 +154,6 @@ func TestBackend_RSAKey(t *testing.T) {
Steps: []logicaltest.TestStep{},
}

stepCount = len(testCase.Steps)

intdata := map[string]interface{}{}
reqdata := map[string]interface{}{}
testCase.Steps = append(testCase.Steps, generateCATestingSteps(t, rsaCACert, rsaCAKey, ecCACert, intdata, reqdata)...)
Expand Down Expand Up @@ -183,8 +182,6 @@ func TestBackend_ECKey(t *testing.T) {
Steps: []logicaltest.TestStep{},
}

stepCount = len(testCase.Steps)

intdata := map[string]interface{}{}
reqdata := map[string]interface{}{}
testCase.Steps = append(testCase.Steps, generateCATestingSteps(t, ecCACert, ecCAKey, rsaCACert, intdata, reqdata)...)
Expand All @@ -211,8 +208,6 @@ func TestBackend_CSRValues(t *testing.T) {
Steps: []logicaltest.TestStep{},
}

stepCount = len(testCase.Steps)

intdata := map[string]interface{}{}
reqdata := map[string]interface{}{}
testCase.Steps = append(testCase.Steps, generateCSRSteps(t, ecCACert, ecCAKey, intdata, reqdata)...)
Expand All @@ -239,8 +234,6 @@ func TestBackend_URLsCRUD(t *testing.T) {
Steps: []logicaltest.TestStep{},
}

stepCount = len(testCase.Steps)

intdata := map[string]interface{}{}
reqdata := map[string]interface{}{}
testCase.Steps = append(testCase.Steps, generateURLSteps(t, ecCACert, ecCAKey, intdata, reqdata)...)
Expand Down Expand Up @@ -278,12 +271,10 @@ func TestBackend_RSARoles(t *testing.T) {
},
}

stepCount = len(testCase.Steps)

testCase.Steps = append(testCase.Steps, generateRoleSteps(t, false)...)
if len(os.Getenv("VAULT_VERBOSE_PKITESTS")) > 0 {
for i, v := range testCase.Steps {
fmt.Printf("Step %d:\n%+v\n\n", i+stepCount, v)
fmt.Printf("Step %d:\n%+v\n\n", i+1, v)
}
}

Expand Down Expand Up @@ -320,12 +311,10 @@ func TestBackend_RSARoles_CSR(t *testing.T) {
},
}

stepCount = len(testCase.Steps)

testCase.Steps = append(testCase.Steps, generateRoleSteps(t, true)...)
if len(os.Getenv("VAULT_VERBOSE_PKITESTS")) > 0 {
for i, v := range testCase.Steps {
fmt.Printf("Step %d:\n%+v\n\n", i+stepCount, v)
fmt.Printf("Step %d:\n%+v\n\n", i+1, v)
}
}

Expand Down Expand Up @@ -362,12 +351,10 @@ func TestBackend_ECRoles(t *testing.T) {
},
}

stepCount = len(testCase.Steps)

testCase.Steps = append(testCase.Steps, generateRoleSteps(t, false)...)
if len(os.Getenv("VAULT_VERBOSE_PKITESTS")) > 0 {
for i, v := range testCase.Steps {
fmt.Printf("Step %d:\n%+v\n\n", i+stepCount, v)
fmt.Printf("Step %d:\n%+v\n\n", i+1, v)
}
}

Expand Down Expand Up @@ -404,12 +391,10 @@ func TestBackend_ECRoles_CSR(t *testing.T) {
},
}

stepCount = len(testCase.Steps)

testCase.Steps = append(testCase.Steps, generateRoleSteps(t, true)...)
if len(os.Getenv("VAULT_VERBOSE_PKITESTS")) > 0 {
for i, v := range testCase.Steps {
fmt.Printf("Step %d:\n%+v\n\n", i+stepCount, v)
fmt.Printf("Step %d:\n%+v\n\n", i+1, v)
}
}

Expand Down Expand Up @@ -588,7 +573,7 @@ func generateURLSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s
Operation: logical.UpdateOperation,
Path: "root/sign-intermediate",
Data: map[string]interface{}{
"common_name": "Intermediate Cert",
"common_name": "intermediate.cert.com",
"csr": string(csrPem1024),
"format": "der",
},
Expand All @@ -609,7 +594,7 @@ func generateURLSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s
Operation: logical.UpdateOperation,
Path: "root/sign-intermediate",
Data: map[string]interface{}{
"common_name": "Intermediate Cert",
"common_name": "intermediate.cert.com",
"csr": string(csrPem2048),
"format": "der",
},
Expand Down Expand Up @@ -638,8 +623,8 @@ func generateURLSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s
return fmt.Errorf("expected\n%#v\ngot\n%#v\n", expected.CRLDistributionPoints, cert.CRLDistributionPoints)
case !reflect.DeepEqual(expected.OCSPServers, cert.OCSPServer):
return fmt.Errorf("expected\n%#v\ngot\n%#v\n", expected.OCSPServers, cert.OCSPServer)
case !reflect.DeepEqual([]string{"Intermediate Cert"}, cert.DNSNames):
return fmt.Errorf("expected\n%#v\ngot\n%#v\n", []string{"Intermediate Cert"}, cert.DNSNames)
case !reflect.DeepEqual([]string{"intermediate.cert.com"}, cert.DNSNames):
return fmt.Errorf("expected\n%#v\ngot\n%#v\n", []string{"intermediate.cert.com"}, cert.DNSNames)
}

return nil
Expand All @@ -651,7 +636,7 @@ func generateURLSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s
Operation: logical.UpdateOperation,
Path: "root/sign-intermediate",
Data: map[string]interface{}{
"common_name": "Intermediate Cert",
"common_name": "intermediate.cert.com",
"csr": string(csrPem2048),
"format": "der",
"exclude_cn_from_sans": true,
Expand Down Expand Up @@ -991,7 +976,7 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
Operation: logical.UpdateOperation,
Path: "intermediate/generate/exported",
Data: map[string]interface{}{
"common_name": "Intermediate Cert",
"common_name": "intermediate.cert.com",
},
Check: func(resp *logical.Response) error {
intdata["intermediatecsr"] = resp.Data["csr"].(string)
Expand All @@ -1009,7 +994,7 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
delete(reqdata, "pem_bundle")
delete(reqdata, "ttl")
reqdata["csr"] = intdata["intermediatecsr"].(string)
reqdata["common_name"] = "Intermediate Cert"
reqdata["common_name"] = "intermediate.cert.com"
reqdata["ttl"] = "10s"
return nil
},
Expand Down Expand Up @@ -1144,7 +1129,7 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
"format": "der",
"key_type": "ec",
"key_bits": 384,
"common_name": "Intermediate Cert",
"common_name": "intermediate.cert.com",
},
Check: func(resp *logical.Response) error {
csrBytes, _ := base64.StdEncoding.DecodeString(resp.Data["csr"].(string))
Expand All @@ -1171,7 +1156,7 @@ func generateCATestingSteps(t *testing.T, caCert, caKey, otherCaCert string, int
delete(reqdata, "pem_bundle")
delete(reqdata, "ttl")
reqdata["csr"] = intdata["intermediatecsr"].(string)
reqdata["common_name"] = "Intermediate Cert"
reqdata["common_name"] = "intermediate.cert.com"
reqdata["ttl"] = "10s"
return nil
},
Expand Down Expand Up @@ -1646,7 +1631,18 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
retName = cert.EmailAddresses[0]
}
if retName != name {
return fmt.Errorf("Error: returned certificate has a DNS SAN of %s but %s was requested", retName, name)
// Check IDNA
p := idna.New(
idna.StrictDomainName(true),
idna.VerifyDNSLength(true),
)
converted, err := p.ToUnicode(retName)
if err != nil {
t.Fatal(err)
}
if converted != name {
return fmt.Errorf("Error: returned certificate has a DNS SAN of %s (from idna: %s) but %s was requested", retName, converted, name)
}
}
return nil
}
Expand All @@ -1662,7 +1658,7 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
SubSubdomain bool `structs:"foo.bar.example.com"`
SubSubdomainWildcard bool `structs:"*.bar.example.com"`
GlobDomain bool `structs:"fooexample.com"`
NonHostname bool `structs:"daɪˈɛrɨsɨs"`
IDN bool `structs:"daɪˈɛrɨsɨs"`
AnyHost bool `structs:"porkslap.beer"`
}

Expand Down Expand Up @@ -1876,10 +1872,10 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
roleVals.AllowAnyName = true
roleVals.EnforceHostnames = true
commonNames.AnyHost = true
commonNames.IDN = true
addCnTests()

roleVals.EnforceHostnames = false
commonNames.NonHostname = true
addCnTests()

// Ensure that we end up with acceptable key sizes since they won't be
Expand Down
47 changes: 39 additions & 8 deletions builtin/logical/pki/cert_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/hashicorp/vault/logical"
"github.com/hashicorp/vault/logical/framework"
"github.com/ryanuber/go-glob"
"golang.org/x/net/idna"
)

type certExtKeyUsage int
Expand Down Expand Up @@ -91,7 +92,11 @@ func (b *caInfoBundle) GetCAChain() []*certutil.CertBlock {
}

var (
hostnameRegex = regexp.MustCompile(`^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$`)
// A note on hostnameRegex: although we set the StrictDomainName option
// when doing the idna conversion, this appears to only affect output, not
// input, so it will allow e.g. host^123.example.com straight through. So
// we still need to use this to check the output.
hostnameRegex = regexp.MustCompile(`^(\*\.)?(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$`)
oidExtensionBasicConstraints = []int{2, 5, 29, 19}
)

Expand Down Expand Up @@ -297,7 +302,15 @@ func validateNames(req *logical.Request, names []string, role *roleEntry) string
// ensure that we are not either checking a full email address or a
// wildcard prefix.
if role.EnforceHostnames {
if !hostnameRegex.MatchString(sanitizedName) {
p := idna.New(
idna.StrictDomainName(true),
idna.VerifyDNSLength(true),
)
converted, err := p.ToASCII(sanitizedName)
if err != nil {
return name
}
if !hostnameRegex.MatchString(converted) {
return name
}
}
Expand Down Expand Up @@ -413,7 +426,6 @@ func validateNames(req *logical.Request, names []string, role *roleEntry) string
}
}

//panic(fmt.Sprintf("\nName is %s\nRole is\n%#v\n", name, role))
return name
}

Expand Down Expand Up @@ -642,9 +654,18 @@ func generateCreationBundle(b *backend,
// used for the purpose for which they are presented
emailAddresses = append(emailAddresses, cn)
} else {
// Only add to dnsNames if it's actually a DNS name
if hostnameRegex.MatchString(cn) {
dnsNames = append(dnsNames, cn)
// Only add to dnsNames if it's actually a DNS name but convert
// idn first
p := idna.New(
idna.StrictDomainName(true),
idna.VerifyDNSLength(true),
)
converted, err := p.ToASCII(cn)
if err != nil {
return nil, errutil.UserError{Err: err.Error()}
}
if hostnameRegex.MatchString(converted) {
dnsNames = append(dnsNames, converted)
}
}
}
Expand All @@ -657,8 +678,18 @@ func generateCreationBundle(b *backend,
if strings.Contains(v, "@") {
emailAddresses = append(emailAddresses, v)
} else {
if hostnameRegex.MatchString(v) {
dnsNames = append(dnsNames, v)
// Only add to dnsNames if it's actually a DNS name but
// convert idn first
p := idna.New(
idna.StrictDomainName(true),
idna.VerifyDNSLength(true),
)
converted, err := p.ToASCII(v)
if err != nil {
return nil, errutil.UserError{Err: err.Error()}
}
if hostnameRegex.MatchString(converted) {
dnsNames = append(dnsNames, converted)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions vendor/vendor.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1647,8 +1647,8 @@
{
"checksumSHA1": "RcrB7tgYS/GMW4QrwVdMOTNqIU8=",
"path": "golang.org/x/net/idna",
"revision": "0ed95abb35c445290478a5348a7b38bb154135fd",
"revisionTime": "2018-01-24T06:08:02Z"
"revision": "f5dfe339be1d06f81b22525fe34671ee7d2c8904",
"revisionTime": "2018-02-04T03:50:36Z"
},
{
"checksumSHA1": "5JWn/wMC+EWNDKI/AYE4JifQF54=",
Expand Down

0 comments on commit 07f8ebb

Please sign in to comment.