Add a function to validate an x509 RSA key pair #552
Conversation
mattbostock
force-pushed
the
add_x509_rsa_key_pair
branch
from
96d458b to
d3b5f88
Compare
|
I believe the build failed because of the |
mattbostock
force-pushed
the
add_x509_rsa_key_pair
branch
2 times, most recently
from
0bb61b4 to
ce3b97d
Compare
|
bump |
|
I think it could also be useful to have a type which does this for cases where we create our own certs in the node (like we would for letsencrypt) |
|
@igalic I'm not sure what you mean, can you please elaborate? |
|
a function can only run on a puppet server (or, under puppet apply) a type would run on each node. |
|
@igalic I think that's outside the scope of the stdlib and the use case that this PR is targeted at. |
| end | ||
|
|
||
| unless cert.verify(key) | ||
| raise Puppet::ParseError, "Certificate signature does not match supplied key" |
There was a problem hiding this comment.
Is there any OpenSSL error we can pass through to help debugging this?
There was a problem hiding this comment.
No. cert.verify(key) returns a boolean; either the key matches the certificate or it does not. If the key or cert are malformed or invalid, the earlier checks will catch that and report the error to the user.
|
@igalic has a point that similar functionality would be useful as a type. This does not preclude merging this. I've added a few comments where the code could be improved, then it's good to go! |
|
@DavidS @igalic: I understand now; I agree that adding this as a type would be useful. I'll look into that, but in the meantime I'll address the comments here so that this can be merged. Having a stdlib function remains useful IMHO for validating data that's passed into third-party modules which don't use the type. |
|
Also apologies for the delay in getting back to you, I was away over Christmas 🎄 |
Add a function to validate an x509 RSA certificate and key pair, as
commonly used for TLS certificates.
The rationale behind this is that we store our TLS certificates and
private keys in Hiera YAML files, and poor indentation or formatting in
the YAML file could cause a valid certificate to be considered invalid.
Will cause the Puppet run to fail if:
- an invalid certificate is detected
- an invalid RSA key is detected
- the certificate does not match the key, i.e. the certificate
has not been signed by the supplied key
The test certificates I've used in the spec tests were generated using
the Go standard library:
$ go run $GOROOT/src/crypto/tls/generate_cert.go -host localhost
Example output:
==> cache-1.router: Error: Not a valid RSA key: Neither PUB key nor PRIV key:: nested asn1 error at /var/govuk/puppet/modules/nginx/manifests/config/ssl.pp:30 on node cache-1.router.dev.gov.uk
Test a valid certificate and valid key that have had 48 characters removed from their middle, to simulate a malformed certificate and key. Suggested by @DavidS in puppetlabs#552
Put the tests using a valid certificate fixture together and put tests using a valid key fixture together.
mattbostock
force-pushed
the
add_x509_rsa_key_pair
branch
from
ce3b97d to
41f9319
Compare
|
I've just rebased to address the above comments; please let me know if there's anything else to add. |
Add a function to validate an x509 RSA key pair
|
Excellent work. Love it! Thanks a lot. |
|
@DavidS When is this likely to be released? |
Test a valid certificate and valid key that have had 48 characters removed from their middle, to simulate a malformed certificate and key. Suggested by @DavidS in puppetlabs/puppetlabs-stdlib#552
|
@mattbostock Was this only ever intended for self-signed certs? We had a situation where we wanted to check that a key and cert matched, but as the cert was signed by an external CA, this function didn't work for us. |
|
No, from memory we used this function with certs signed by established CAs.
I'm no longer using Puppet so I'm unlikely to be able to help more am
afraid, except to say that RSA certs are less common these days so perhaps
your certificate is not using RSA?
…On Tue, 11 Jan 2022, 14:15 Alexander Fisher, ***@***.***> wrote:
@mattbostock <https://github.com/mattbostock> Was this only ever intended
for self-signed certs? We had a situation where we wanted to check that a
key and cert matched, but as the cert was signed by an external CA, this
function didn't work for us.
—
Reply to this email directly, view it on GitHub
<#552 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABBFXMFGZ4XKFFLUCWFUOZDUVQ3RFANCNFSM4BVB3GFQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Add a function to validate an x509 RSA certificate and key pair, as
commonly used for TLS certificates.
The rationale behind this is that we store our TLS certificates and
private keys in Hiera YAML files, and poor indentation or formatting in
the YAML file could cause a valid certificate to be considered invalid.
Will cause the Puppet run to fail if:
has not been signed by the supplied key
The test certificates I've used in the spec tests were generated using
the Go standard library:
Example output: