Add required validation to new Certificates #23
Conversation
samribeiro
commented
May 6, 2019
samribeiro
commented
- make certificate lowercase
- add wording that allows to optionally validating a newly installed certificate against a CA present in the Target's CA pool.
|
Sam, looks good. Thanks for adding the validation of the certificate being installed. |
|
|
||
| For increased security, the Target may use certificates in its CA pool to | ||
| validate a newly installed certificate. This requires the CA pool to contain a | ||
| CA certificate that can validate the new certificate. |
There was a problem hiding this comment.
Sam, thanks for adding this. In cert.proto you have added some comments on the certificate order (in the ca_certificates bundle) and its potential impact on performance in the target . The same applies for the CA-certificate used to validate the new certificate: not knowing where it is in the bundle also has an impact on performance.
There was a problem hiding this comment.
Indeed, however there are two mechanisms in play there:
- find & identify certificate chains;
- find the certificate that validates this one I got;
The change proposed in the cert.proto, facilitates reducing performance impact because of 1).
Mechanism 2) is applicable to any certificate validation. Like: a) New certificate installation or b) certificates presented by a new session. Seeking to further specify rules for ordering certificates in the bundle to take into consideration a) seems excessive, given that b) is ordinarily expected. If you tell me that a) is orders of magnitude more costly than b), then I would be very curious as to why. =)
There was a problem hiding this comment.
I agree that the performance of 1) has been helped by #22 (assuming the suggested order is followed).
For 2 b) when a client presents a certificate for a new session, we know a priori whether the client certificate has to be validated and if so by which CA-Cert (it has been installed by the bundle in a previous install).
For 2 a) when a new certificate is installed, since we don't know which certificate in the bundle ca_certificates should be used to validate the certificate being installed, then the worse case is that we need to try all the certificates in the bundle.
There was a problem hiding this comment.
for 2b) If the CA bundle has a few Certificates, how do you know which one of them to use for validating the client? If there is a direct bind, I don't know about it.
There was a problem hiding this comment.
Sam, you can figure this out at install time. So if your bundle has Int1-CA-Cert, Int2-CA-Cert, Root-CA-Cert, by validating the signatures you can know which one is the "lowest" CA-cert which should be used to validate the client cert when it is presented.
Did I misunderstand your question?
There was a problem hiding this comment.
Got it. So there is always an extra effort whenever the CA bundle is provided. For situations where the CA bundle is empty the system already has a mapping for the chains in the stored CA list.
There was a problem hiding this comment.
Yes there is extra work when the CA bundle is provided. This is why we'd like some "predictability" for the bundle: expected order of the CA-certs for peer authentication and location of the CA-cert to validate the target cert being installed. Thanks.
There was a problem hiding this comment.
Sam, your comment above about CA bundle empty reminded me of a question which I think I never asked: when the CA bundle is empty does it mean continue using the last CA bundle which was provided or does it mean delete the CA bundle (no more peer authentication). I believe it's the former (please confirm), if so then does that mean that the CA bundle is never deleted?
There was a problem hiding this comment.
Reshad, the documentation states that when not empty, the provided certificates should squash the existing bundle, see:
https://github.com/openconfig/gnoi/blob/master/cert/cert.proto#L277
The only way to delete the CA bundle is to factory reset the device:
https://github.com/openconfig/gnoi/pull/17/files
There was a problem hiding this comment.
Sam, thanks for confirming that empty CA bundle means no change.
samribeiro
changed the title
|
Made this a required validation, instead of optional. |
