Skip to content

feat: load every certificate in the rotating keys engine ca-cert-file - #3528

Open
pjfanning wants to merge 1 commit into
apache:mainfrom
pjfanning:multi-cacert
Open

feat: load every certificate in the rotating keys engine ca-cert-file#3528
pjfanning wants to merge 1 commit into
apache:mainfrom
pjfanning:multi-cacert

Conversation

@pjfanning

Copy link
Copy Markdown
Member

Motivation

RotatingKeysSSLEngineProvider reads ca-cert-file with CertificateFactory.generateCertificate, which stops at the first PEM block. A CA file that bundles more than one certificate — a root plus the intermediates it delegates to, or two roots while a CA is being rotated — was therefore only half trusted, and peers presenting a certificate issued by any of the other CAs in the bundle were rejected.

Modification

  • Add PemManagersProvider.loadCertificates, which uses generateCertificates and closes the stream (the singular loadCertificate never did).
  • buildTrustManagers and buildKeyManagers take the whole Seq[Certificate]; every CA certificate is stored under its own alias, so all of them become trust anchors.
  • The chain attached to the key entry is built by walking issuer → subject from the node certificate upwards, so a CA certificate that did not issue it (an unrelated root kept around for a rotation, say) is trusted without being sent to the peer. When none of them matches the issuer, the previous behaviour of sending them all is kept.
  • readFiles fails with SslTransportException when the CA file yields no certificate, instead of passing a null certificate on.
  • Document the bundle support on ca-cert-file in reference.conf.

Result

ca-cert-file may bundle any number of certificates and all of them are trusted. Single-certificate configurations behave exactly as before.

Tests

  • sbt "remote/testOnly org.apache.pekko.remote.artery.tcp.ssl.PemManagersProviderSpec org.apache.pekko.remote.artery.tcp.ssl.TlsResourcesSpec org.apache.pekko.remote.artery.tcp.ssl.*Rotating*" — passed
  • sbt "remote/mimaReportBinaryIssues" — passed
  • Both were run before this change was merged with the keystore-password change on main; verification of the merged state is left to CI.
  • New cases in PemManagersProviderSpec, backed by a new ssl/exampleca-bundle.crt resource (exampleca.crt + pem/selfsigned-certificate.pem): the trust manager's accepted issuers contain both certificates, and the key manager's chain for the node certificate contains only the CA that issued it.

References

None — follow-up on multi-certificate ca-cert-file support

Motivation:
`RotatingKeysSSLEngineProvider` read `ca-cert-file` with
`CertificateFactory.generateCertificate`, which stops at the first PEM
block. A CA file that bundles more than one certificate - a root plus the
intermediates it delegates to, or two roots while a CA is being rotated -
was therefore only half trusted, and peers presenting a certificate from
any of the other CAs in the bundle were rejected.

Modification:
Add `PemManagersProvider.loadCertificates`, which uses
`generateCertificates` and closes the stream. `buildTrustManagers` and
`buildKeyManagers` now take the whole `Seq[Certificate]`; every CA
certificate is stored under its own alias so all of them become trust
anchors. The chain attached to the key entry is built by walking
issuer -> subject from the node certificate upwards, so CA certificates
that did not issue it are trusted without being sent to the peer; when
none of them matches the issuer the previous behaviour (all of them) is
kept. `readFiles` now fails with `SslTransportException` when the CA file
yields no certificate at all, instead of passing a null certificate on.

Result:
`ca-cert-file` may bundle any number of certificates and all of them are
trusted. Single-certificate configurations behave as before.

Tests:
- sbt "remote/testOnly org.apache.pekko.remote.artery.tcp.ssl.PemManagersProviderSpec org.apache.pekko.remote.artery.tcp.ssl.TlsResourcesSpec org.apache.pekko.remote.artery.tcp.ssl.*Rotating*" - passed
- sbt "remote/mimaReportBinaryIssues" - passed
- both were run before this change was merged with the keystore-password change on main; verification of the merged state is left to CI

References:
None - follow-up on multi-certificate `ca-cert-file` support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant