fix(metrics): check local cert key type in policy compatibility - #6015
fix(metrics): check local cert key type in policy compatibility#6015Harikeshav-R wants to merge 1 commit into
Conversation
The compatibility metrics only looked at what the client advertised in its ClientHello, not the certificate the endpoint actually uses. This made a P-256 cert look CNSA1-compatible, even though CNSA1 rejects P-256 certs and the handshake would break. Add the local (selected) certificate's key types as a dimension of the compatibility check. CNSA1 now only counts as compatible when every local cert key is one CNSA1 allows (P-384 or RSA-3072/4096). Profiles with no cert-key restriction (general, fips) are unaffected. Resolves aws#5982
| .fetch_add(1, Ordering::Relaxed); | ||
| } | ||
| if Cnsa1::supported(&supported_parameter) { | ||
| if Cnsa1::supported(&supported_parameter, &local_cert_keys) { |
There was a problem hiding this comment.
I think that this ends up destroying information ...
E.g. if currently have an secp256r1 cert configuring on my endpoint, I now see that there is 0 CNSA 1.0 support, but I can't distinguish whether that is because
- none of my clients support CNSA 1.0 parameters
- just that I (the server) don't have a CNSA 1.0 cert.
At least with the current state, all of the information is distinguishable, because the cert key types are visible in the handshake metrics.
There was a problem hiding this comment.
How about I leave compatibility.cnsa1 as it is (client-side readiness only) and add a separate counter for the strict signal, which is only true when the client params and the local cert are both CNSA1-compatible. That way "are my clients ready?" and "would a migration definitely be safe?" stay as two distinct numbers, and the difference between them tells you your cert is the blocker.
Also, should I add this extra counter just for CNSA1 (the only policy where the cert check actually changes anything today), or add it across the family (general/fips/cnsa2) for consistency? cnsa2 is a bit awkward since the small cert parser doesn't recognize ML-DSA keys yet, so a cnsa2 cert check wouldn't be meaningful until that's added.
What do you think is the best approach here?
Goal
Make the "which security policy can I safely move to" metrics tell the truth about CNSA1.
Why
The metrics subscriber reports whether a connection could move to a stricter security policy (like CNSA1). But it was only looking at what the client said it supports in its ClientHello. It never looked at the certificate being used.
CNSA1 does not allow P-256 certificates. So a connection using a P-256 cert would break if you switched to CNSA1. Even so, the old code counted that connection as "CNSA1 compatible." That gave a false green light.
This is the problem described in #5982.
How
We now also look at the certificate the endpoint itself presents (its local/selected cert).
For CNSA1, a connection only counts as compatible when every cert in that local chain uses a key CNSA1 actually allows (P-384, or RSA 3072/4096). A P-256 cert now correctly counts as not compatible.
The
generalandfipsmetrics don't restrict certificate keys, so nothing changes for them.Callouts
Testing
Related
Resolves #5982
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.