Icinga
process certificate chains presented by the client
Similar to https://github.com/Icinga/icinga2/pull/8859 this patch works around https://github.com/Icinga/icinga2/issues/7719 by allowing the intermediate certificate presented by the icinga2-agent.
To make this work the icinga2-master only holds to root-ca in its local ca.crt, while the icinga2-agent has the intermediate-cert in its local ca.crt (or the intermediate together with the root in the ca.crt / or the intermediate in the cert.pem - doesn't matter).
Edit (@Al2Klimov)
closes #9026
At first glance, this looks more like the change I was expecting for certificate changes. :+1:
or the intermediate in the cert.pem
So with this PR -in contrast to master- #8859 (comment) (haven’t tested yet) should just work?
That is correct. The intermediate can either go into the cert.pem or into the ca.pem (on the agent-side) as well. OpenSSL internally picks up all the chain-elements and send them over, so no change is actually needed on the agent-side. Only the master needs to pickup the (untrusted) chain from the wire and tries to validate the whole set against its local root-ca-cert.
Hold on a second, do I understand this correctly: You're saying (also taking information from #9798 into account here) that node certificates signed by an intermediate CA work perfectly fine apart from a log message that doesn't limit the functionality ("Received certificate request for CN ... not signed by our CA:")?
So the claim made by #9026 (put that renewal stuff in an if and things work) was indeed correct?
And in contrast, this PR would fix it by making the renewal logic aware of the chain which contains the intermediates required for verification?
Yes. IMAO we don’t even need #9026 on every node, instead we could check the root CA name in code or so. But I prefer PRs like this one.
I didn't want to revive #9026 but my intuition about that PR was "how could this possibly make intermediate certificates work", but if it wasn't even that broken, it makes a bit more sense now.
Just to give an update: I am still in the process of reading through your comments (also in #9798). However because of vacation in the next week it will be a little bit delayed, hope that is ok.
Please rebase.
Rebased to the version we are currently running in production.
This isn't the previously approved implementation anymore. Something went wrong. Please re-create f06672c.
I don't really understand. The API was changed in current upstream (security-changes for older OpenSSL-version), so the old commit cannot work any longer.
However the method on how this changes works is still the same. It still used the full CA-chain, but that CA-chain is no longer supplied instead of the previous only topmost certificate, but rather as an additional parameter (which is nullable if not needed).
VerifyCertificate() still uses X509_STORE_CTX_init() which accepts a chain. VerifyCertificate() should too.