Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Various PKI updates #3953

Merged
merged 7 commits into from
Feb 10, 2018
Merged

Various PKI updates #3953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
56b48a4
Various PKI updates:
jefferai Feb 9, 2018
48d2cd9
Vendor deps
jefferai Feb 9, 2018
2a19b7f
Simplify the idna logic a bit
jefferai Feb 9, 2018
d2452aa
Remove hostname regex
jefferai Feb 9, 2018
ffba3a4
Fix build
jefferai Feb 9, 2018
873e321
Change nonhostname test to IDN test
jefferai Feb 9, 2018
c327d24
Re-add hostname regex. IDNA's strict domain name checking appears to
jefferai Feb 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 28 additions & 32 deletions builtin/logical/pki/backend_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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