feat(server/tls): add support for multiple CAs #310
Conversation
b5bf76f to
93cd8c0
Compare
vruello
left a comment
There was a problem hiding this comment.
Hi!
During my tests Windows clients only sent their peer certificate, not including the intermediate CA. Is it a known Windows behavior/quirk?
I'm not aware of this, but that would not be very surprising.
Is using the issuer field a proper way to identify certificate entities? I think it is enough, as the file is controlled by the user and they should be aware of its' contents. Nevertheless there is a warn for duplicates, and a hard error for same issuers, but different thumbprints.
It looks like you used the certificates subject to check for duplication (and not the issuer). I think this approach is okay.
However, I'm unsure about the method you used to retrieve the appropriate thumbprint for the client. As I understand it:
- we retrieve the client peer certificates and only take the first one ()
Line 1002 in fd5cf96
- we try to find a CA subject that matches the issuer field of this certificate, and panic otherwise.
- we retrieve the thumbprint of the matching CA certificate.
If the client properly sends a full certificates chain, the server will panic if it does not trust the first intermediate authority, even if it trusts a parent authority in the chain.
I think we should try to find a matching CA within all the client peer certificates, and raise an error if we cannot find one (rather than panicking). Does this approach seem reasonable to you?
server/src/tls.rs
Outdated
| // Put all certificates from given CA certificate file into certificate store | ||
| // Compute thumbprint for each CA and add to trust store | ||
| for root in ca_certs { | ||
| let issuer = subject_from_cert(root.as_ref())?; |
There was a problem hiding this comment.
Looks like the variable should be named subject?
|
Hey @vruello! Thank you for the quick review, I appreciate it!
Yes, sorry for the confusion, I meant it from the peer's perspective. Yes, the
Absolutely, will change the PR accordingly, thank you! |
This is done in preparation for supporting multiple CAs Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>
This is done in preparation for supporting multiple CAs Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>
Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>
… subject Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>
93cd8c0 to
5337200
Compare
|
Looks good to me, thanks @OverOrion! :) |
This PR allows one to configure multiple CAs for their WEC server.
Given the following scenario:
Root CACA1andCA2, both signed byRoot CA- intermediate certificate authoritiesClient1andClient2- each signed by their respective CA (CA1,CA2)This requires the WEC server to return the proper (CA) thumbprint for each client.
I do have the following questions:
RFC 5246 - 7.4.2 says:
But based on DSP0226) -
C.3.5 http://schemas.dmtf.org/wbem/wsman/1/wsman/secprofile/https/mutualit says the following:This matches with my observed behavior, but wanted to confirm nevertheless.
issuerfield a proper way to identify certificate entities? I think it is enough, as the file is controlled by the user and they should be aware of its' contents. Nevertheless there is a warn for duplicates, and a hard error for same issuers, but different thumbprints.