Add testing/certutil/ package for TLS test support #189
Conversation
`testing/certutil/certutil` provides functionality to generate root CA and child certificates for use in tests It can also be used as a CLI
AndersonQ
requested review from
ycombinator,
leehinman,
michalpristas and
blakerouse
and removed request for
a team,
ycombinator and
leehinman
|
I am strongly in favor of generating the CA & certs as needed for testing. But do we need to write our own utility? https://github.com/cloudflare/cfssl Already does all of this, is go so we can embed in our tests, and has been around a while so most of the subtle X509 considerations have been addressed. Downside is that we don't build up as much X509 knowledge in our team. Thoughts? |
I was having a look at it. It feels overkill and I'd bet getting used to use it would not be much different than using what this PR adds. Also as you pointed, by building it ourselves we get some more knowledge in the team about how all the moving pieces get together. Lastly, it's done already, the beats use a similar code that can, and should, be replaced by this new package. Just the work to replace this PR and instead use |
testing/certutil/certutil.go
Outdated
| NotBefore: notBefore, | ||
| NotAfter: notAfter, | ||
| KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, | ||
| ExtKeyUsage: []x509.ExtKeyUsage{ |
There was a problem hiding this comment.
If this is really a root CA it should not have ClientAuth & ServerAuth as extended key usages. CAs are only supposed to sign certificates.
| certTemplate := &x509.Certificate{ | ||
| DNSNames: []string{name}, | ||
| IPAddresses: ips, | ||
| SerialNumber: big.NewInt(1658), |
There was a problem hiding this comment.
Every child cert shouldn't have the same serial number. Ideally the CA is keeping track of this and providing unique serial numbers. If you don't want to track that for testing, just using a random int over a "large" space is good enough.
| // - the certificate and private key as a tls.Certificate | ||
| // | ||
| // If any error occurs during the generation process, a non-nil error is returned. | ||
| func GenerateChildCert(name string, ips []net.IP, caPrivKey crypto.PrivateKey, caCert *x509.Certificate) (*tls.Certificate, Pair, error) { |
There was a problem hiding this comment.
don't have to change this, but as an FYI. This is "unusual" from a CA perspective, usually a client generates a "To Be Signed Certificate Request" and sends that to the CA. In go that would be https://pkg.go.dev/crypto/x509@go1.22.1#CertificateRequest. If you look inside that struct you see it just contains the Public Key of the requester. The CA never "sees" the private key.
|
as discussed directly with @leehinman, the child certificate generation will be improved on another PR and use the |
💚 Build Succeeded
History
cc @AndersonQ |
What does this PR do?
Adds the
testing/certutil/package providing on-the-fly generation of TLS certificate and root CA and a CLI to generate them as well.Why is it important?
The Elastic Agent and Bats need to configure TLS for testing. This provide an easy way to generate the needed certificates without duplicating code in the Elastic Agent and Beats repo
Checklist
[ ] I have added tests that prove my fix is effective or that my feature worksRelated issues
fleet.ssl.certificate_authoritiesfrom agent policy elastic-agent#2247