-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
Support other names in SANs #3889
Changes from 14 commits
02aaa96
0c6f6b1
452cb3c
8ab69b0
ddf6c54
ffb5798
33476a2
a5cc3d3
6c773cd
6d963d3
0e55e12
ef513c6
5219323
ff0a3e0
f5a57df
11156d7
4e4fba9
ec8e9e0
a8d62af
de1fe7a
3201614
aa6a7a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2801,6 +2801,253 @@ func TestBackend_SignSelfIssued(t *testing.T) { | |
} | ||
} | ||
|
||
// This is a really tricky test because the Go stdlib asn1 package is incapable | ||
// of doing the right thing with custom OID SANs (see comments in the package, | ||
// it's readily admitted that it's too magic) but that means that any | ||
// validation logic written for this test isn't being independently verified, | ||
// as in, if cryptobytes is used to decode it to make the test work, that | ||
// doesn't mean we're encoding and decoding correctly, only that we made the | ||
// test pass. Instead, when run verbosely it will first perform a bunch of | ||
// checks to verify that the OID SAN logic doesn't screw up other SANs, then | ||
// will spit out the PEM. This can be validated independently. | ||
// | ||
// You want the hex dump of the octet string corresponding tot he X509v3 | ||
// Subject Alternative Name. Unfortunately, openssl asn1parse doesn't seem to | ||
// be particularly good at then dumping this in a meaningful way, and neither | ||
// does dumpasn1. However, there's a nice online utility at | ||
// https://lapo.it/asn1js You can view the structure of an openssl-generated | ||
// other SAN at | ||
// https://lapo.it/asn1js/#3022A020060A2B060104018237140203A0120C106465766F7073406C6F63616C686F7374 | ||
// | ||
// The structure output from here should match that precisely (even if the OID | ||
// itself doesn't) in the second test. | ||
// | ||
// The test that encodes two should have them be in separate elements in the | ||
// top-level sequence; see | ||
// https://lapo.it/asn1js/#3046A020060A2B060104018237140203A0120C106465766F7073406C6F63616C686F7374A022060A2B060104018237140204A0140C12322D6465766F7073406C6F63616C686F7374 for an openssl-generated example. | ||
// | ||
// The good news is that it's valid to simply copy and paste the ouput from | ||
// here into the form at that site as it will do the right thing so it's pretty | ||
// easy to validate. | ||
func TestBackend_OID_SANs(t *testing.T) { | ||
coreConfig := &vault.CoreConfig{ | ||
LogicalBackends: map[string]logical.Factory{ | ||
"pki": Factory, | ||
}, | ||
} | ||
cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ | ||
HandlerFunc: vaulthttp.Handler, | ||
}) | ||
cluster.Start() | ||
defer cluster.Cleanup() | ||
|
||
client := cluster.Cores[0].Client | ||
var err error | ||
err = client.Sys().Mount("root", &api.MountInput{ | ||
Type: "pki", | ||
Config: api.MountConfigInput{ | ||
DefaultLeaseTTL: "16h", | ||
MaxLeaseTTL: "60h", | ||
}, | ||
}) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
var resp *api.Secret | ||
var certStr string | ||
var block *pem.Block | ||
var cert *x509.Certificate | ||
|
||
_, err = client.Logical().Write("root/root/generate/internal", map[string]interface{}{ | ||
"ttl": "40h", | ||
"common_name": "myvault.com", | ||
}) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
_, err = client.Logical().Write("root/roles/test", map[string]interface{}{ | ||
"allowed_domains": []string{"foobar.com", "zipzap.com"}, | ||
"allow_bare_domains": true, | ||
"allow_subdomains": true, | ||
"allow_ip_sans": true, | ||
"allowed_other_sans": "1.3.6.1.4.1.311.20.2.3;UTF8:devops@*,1.3.6.1.4.1.311.20.2.4;utf8:d*[email protected]", | ||
}) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
// Get a baseline before adding OID SANs. In the next sections we'll verify | ||
// that the SANs are all added even as the OID SAN inclusion forces other | ||
// adding logic (custom rather than built-in Golang logic) | ||
resp, err = client.Logical().Write("root/issue/test", map[string]interface{}{ | ||
"common_name": "foobar.com", | ||
"ip_sans": "1.2.3.4", | ||
"alt_names": "foo.foobar.com,bar.foobar.com", | ||
"ttl": "1h", | ||
}) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
certStr = resp.Data["certificate"].(string) | ||
block, _ = pem.Decode([]byte(certStr)) | ||
cert, err = x509.ParseCertificate(block.Bytes) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
if cert.IPAddresses[0].String() != "1.2.3.4" { | ||
t.Fatalf("unexpected IP SAN %q", cert.IPAddresses[0].String()) | ||
} | ||
if cert.DNSNames[0] != "foobar.com" || | ||
cert.DNSNames[1] != "bar.foobar.com" || | ||
cert.DNSNames[2] != "foo.foobar.com" { | ||
t.Fatalf("unexpected DNS SANs %v", cert.DNSNames) | ||
} | ||
|
||
// First test some bad stuff that shouldn't work | ||
resp, err = client.Logical().Write("root/issue/test", map[string]interface{}{ | ||
"common_name": "foobar.com", | ||
"ip_sans": "1.2.3.4", | ||
"alt_names": "foo.foobar.com,bar.foobar.com", | ||
"ttl": "1h", | ||
// Not a valid value for the first possibility | ||
"other_sans": "1.3.6.1.4.1.311.20.2.3;UTF8:[email protected]", | ||
}) | ||
if err == nil { | ||
t.Fatal("expected error") | ||
} | ||
|
||
resp, err = client.Logical().Write("root/issue/test", map[string]interface{}{ | ||
"common_name": "foobar.com", | ||
"ip_sans": "1.2.3.4", | ||
"alt_names": "foo.foobar.com,bar.foobar.com", | ||
"ttl": "1h", | ||
// Not a valid OID for the first possibility | ||
"other_sans": "1.3.6.1.4.1.311.20.2.5;UTF8:[email protected]", | ||
}) | ||
if err == nil { | ||
t.Fatal("expected error") | ||
} | ||
|
||
resp, err = client.Logical().Write("root/issue/test", map[string]interface{}{ | ||
"common_name": "foobar.com", | ||
"ip_sans": "1.2.3.4", | ||
"alt_names": "foo.foobar.com,bar.foobar.com", | ||
"ttl": "1h", | ||
// Not a valid name for the second possibility | ||
"other_sans": "1.3.6.1.4.1.311.20.2.4;UTF8:[email protected]", | ||
}) | ||
if err == nil { | ||
t.Fatal("expected error") | ||
} | ||
|
||
resp, err = client.Logical().Write("root/issue/test", map[string]interface{}{ | ||
"common_name": "foobar.com", | ||
"ip_sans": "1.2.3.4", | ||
"alt_names": "foo.foobar.com,bar.foobar.com", | ||
"ttl": "1h", | ||
// Not a valid OID for the second possibility | ||
"other_sans": "1.3.6.1.4.1.311.20.2.5;UTF8:[email protected]", | ||
}) | ||
if err == nil { | ||
t.Fatal("expected error") | ||
} | ||
|
||
resp, err = client.Logical().Write("root/issue/test", map[string]interface{}{ | ||
"common_name": "foobar.com", | ||
"ip_sans": "1.2.3.4", | ||
"alt_names": "foo.foobar.com,bar.foobar.com", | ||
"ttl": "1h", | ||
// Not a valid type | ||
"other_sans": "1.3.6.1.4.1.311.20.2.5;UTF2:[email protected]", | ||
}) | ||
if err == nil { | ||
t.Fatal("expected error") | ||
} | ||
|
||
// Valid for first possibility | ||
resp, err = client.Logical().Write("root/issue/test", map[string]interface{}{ | ||
"common_name": "foobar.com", | ||
"ip_sans": "1.2.3.4", | ||
"alt_names": "foo.foobar.com,bar.foobar.com", | ||
"ttl": "1h", | ||
"other_sans": "1.3.6.1.4.1.311.20.2.3;utf8:[email protected]", | ||
}) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
certStr = resp.Data["certificate"].(string) | ||
block, _ = pem.Decode([]byte(certStr)) | ||
cert, err = x509.ParseCertificate(block.Bytes) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
if cert.IPAddresses[0].String() != "1.2.3.4" { | ||
t.Fatalf("unexpected IP SAN %q", cert.IPAddresses[0].String()) | ||
} | ||
if cert.DNSNames[0] != "foobar.com" || | ||
cert.DNSNames[1] != "bar.foobar.com" || | ||
cert.DNSNames[2] != "foo.foobar.com" { | ||
t.Fatalf("unexpected DNS SANs %v", cert.DNSNames) | ||
} | ||
t.Logf("certificate 1 to check:\n%s", certStr) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reminder to remove. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You didn't read the comment carefully enough :-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did now :-) 👍 |
||
|
||
// Valid for second possibility | ||
resp, err = client.Logical().Write("root/issue/test", map[string]interface{}{ | ||
"common_name": "foobar.com", | ||
"ip_sans": "1.2.3.4", | ||
"alt_names": "foo.foobar.com,bar.foobar.com", | ||
"ttl": "1h", | ||
"other_sans": "1.3.6.1.4.1.311.20.2.4;UTF8:[email protected]", | ||
}) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
certStr = resp.Data["certificate"].(string) | ||
block, _ = pem.Decode([]byte(certStr)) | ||
cert, err = x509.ParseCertificate(block.Bytes) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
if cert.IPAddresses[0].String() != "1.2.3.4" { | ||
t.Fatalf("unexpected IP SAN %q", cert.IPAddresses[0].String()) | ||
} | ||
if cert.DNSNames[0] != "foobar.com" || | ||
cert.DNSNames[1] != "bar.foobar.com" || | ||
cert.DNSNames[2] != "foo.foobar.com" { | ||
t.Fatalf("unexpected DNS SANs %v", cert.DNSNames) | ||
} | ||
t.Logf("certificate 2 to check:\n%s", certStr) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reminder to remove. |
||
|
||
// Valid for both | ||
resp, err = client.Logical().Write("root/issue/test", map[string]interface{}{ | ||
"common_name": "foobar.com", | ||
"ip_sans": "1.2.3.4", | ||
"alt_names": "foo.foobar.com,bar.foobar.com", | ||
"ttl": "1h", | ||
"other_sans": "1.3.6.1.4.1.311.20.2.3;utf8:[email protected],1.3.6.1.4.1.311.20.2.4;utf8:[email protected]", | ||
}) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
certStr = resp.Data["certificate"].(string) | ||
block, _ = pem.Decode([]byte(certStr)) | ||
cert, err = x509.ParseCertificate(block.Bytes) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
if cert.IPAddresses[0].String() != "1.2.3.4" { | ||
t.Fatalf("unexpected IP SAN %q", cert.IPAddresses[0].String()) | ||
} | ||
if cert.DNSNames[0] != "foobar.com" || | ||
cert.DNSNames[1] != "bar.foobar.com" || | ||
cert.DNSNames[2] != "foo.foobar.com" { | ||
t.Fatalf("unexpected DNS SANs %v", cert.DNSNames) | ||
} | ||
t.Logf("certificate 3 to check:\n%s", certStr) | ||
} | ||
|
||
const ( | ||
rsaCAKey string = `-----BEGIN RSA PRIVATE KEY----- | ||
MIIEogIBAAKCAQEAmPQlK7xD5p+E8iLQ8XlVmll5uU2NKMxKY3UF5tbh+0vkc+Fy | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/tot he/to the
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.