Add URI in allowed_names for cert auth

[nicholasjackson]

This pull request adds the capability for URIs to be part of the allowed_names validation for certificates in the cert auth plugin. This will enable a user to authenticate vault using a SPIFFE SVID which contains a URI SAN for example spiffe://example.com/host.

Currently, I do not feel the tests are in the correct location and have only been implemented to check functionality. At present the test are implemented in the TestBackend_extensions_singleCert and the test fixtures for this test have been regenerated to add an example URI in the certificate extension. I think this should be in a completely separate test for example TestBackend_URI_singleCert and a new leaf cert /key generated containing these.

If you could provide some feedback on this please I would greatly appreciate it, also regarding the test fixtures, currently I am generating the fixtures using openssl not Vault:

# Generate the CSR
$ openssl req -new -config rootcawext.cnf -key rootcawextkey.pem -out rootcawext.csr

# Generate the cert
$ openssl x509 -req -days 365 -in rootcawext.csr -extfile rootcawext.cnf -extensions req_v3 -CA rootcacert.pem -CAkey rootcakey.pem -CAcreateserial -out rootcawextcert.pem

Would also appreciate some advice on the best way to generate the new test fixtures required for this change.

NOTE: This change uses go 1.10 specific features in the crypto/x509 package for the URIs

Kind regards,

Nic

[jefferai]

Hi Nic,

Overloading allowed names for DNS and email is sort of a legacy implementation issue and I think uris should be in a separate parameter. (Eventually I think we should probably split up allowed names into e.g. allowed_dns_hostnames and allowed_email_addresses too.) That removes any ambiguity as to how a name is parsed; for instance if a uri is submitted that has an @ right now we'd assume it's an email address.

As for tests please make specific tests for this rather than inserting new steps; see the ones at the bottom of the test file for guidance.

[nicholasjackson]

Cool thanks, I will add this as a separate parameter and separate the test as requested, happy to do the other stuff too for the split.

[jefferai]

Hi @nicholasjackson -- looks good. I'm happy with this as-is. You mentioned you might take on the work to split the other parameters, are you still thinking of doing that?

[nicholasjackson]

Hey @jefferai, awesome news thank you, Sorry I have been so busy I completely missed your message. Yeah 100% happy to help with doing the split for other parameters. I will ping you on Slack later this week just so I am 100% on the new contract.

[robison]

Hi, is there any way that I might be able to help on this PR? Seems like there's some functionality here that might be useful, especially around signing certs for users (with email addresses as the CN).

[jefferai]

@nicholasjackson Am I right in thinking that #4463 totally supercedes this?

[nicholasjackson]

@jefferai yes, #4463 was based on this PR however it adds the breakout for the api attributes, it might be best to close this one

[jefferai]

Closed by #4463