Proposal #129: Dual TLS certification and crypto agility#130
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
sergio-correia
left a comment
sergio-correia
left a comment
There was a problem hiding this comment.
Good proposal. I pointed out a few design gaps to address, mostly around the tenant's role in forwarding the secondary agent cert and the trust model for that cert relative to the TPM identity chain.
| 1. Extend the agent's TLS context to accept multiple certificate/key pairs. | ||
| Use `set_certificate()`/`set_private_key()` for the first pair and | ||
| `add_certificate()`/`add_private_key()` for subsequent pairs: | ||
| ```rust | ||
| ssl_context_builder.set_certificate(&tls_certs[0])?; | ||
| ssl_context_builder.set_private_key(&keys[0])?; | ||
| for (cert, key) in tls_certs[1..].iter().zip(keys[1..].iter()) { | ||
| ssl_context_builder.add_certificate(cert)?; | ||
| ssl_context_builder.add_private_key(key)?; | ||
| } | ||
| ``` | ||
| If the `openssl` crate does not expose `add_certificate()` on | ||
| `SslAcceptorBuilder`, use lower-level `SslContextBuilder` methods which | ||
| map directly to OpenSSL's `SSL_CTX_use_certificate()` C API. |
There was a problem hiding this comment.
If I'm not mistaken, the Rust openssl crate does not have an add_certificate() or add_private_key() method on SslContextBuilder or SslAcceptorBuilder. The fallback you describe on lines 412-414 is essentially the right approach though, just using set_certificate() / set_private_key() instead:
for (cert, key) in tls_certs.iter().zip(keys.iter()) {
ssl_context_builder.set_certificate(cert)?;
ssl_context_builder.set_private_key(key)?;
}Despite the set_ naming, OpenSSL internally indexes by key type, so each call with a different key type populates a new slot rather than overwriting. Have you had a chance to prototype this?
There was a problem hiding this comment.
Yes, I have a PoC here https://github.com/ansasaki/dual-cert
There was a problem hiding this comment.
I updated the example code snippet
ansasaki
force-pushed
the
dual-certification
branch
from
f7c4e73 to
4abdb22
Compare
This adds the proposal to add dual TLS certification and crypto agility support to all servers (verifier, registrar, and pull agent). Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
ansasaki
force-pushed
the
dual-certification
branch
from
4abdb22 to
064c42b
Compare
3 participants
This adds the proposal to add dual TLS certification and crypto agility support to all servers (verifier, registrar, and pull agent).