mpeylo / cmpossl

An OpenSSL-based implementation of the Certificate Management Protocol (CMP), defined in IETF RFCs 4210, 4211, and 6712. It is being extended according to the emerging RFCs 'CMP Updates' (CMPv3), 'CMP Algorithms', and 'Lightweight CMP Profile'.
https://github.com/mpeylo/cmpossl/wiki
Other
35 stars 13 forks source link

Output only those certs from extraCerts useful for validating the newly enrolled cert #97

Open tpank opened 6 years ago

tpank commented 6 years ago

Users or applications using the library should not be bothered with certificates needed only for checking the protection of CMP messages received. One could modifiy CMP_CTX_extraCertsIn_get1() not to return such such certifiactes, or add a new function called, e.g., CMP_CTX_get1_newClChain() that returns only the relavant portion of the extraCerts received.

This has been requested originally by Hendrik Brockhaus.

Reported by: DDvO

Original Ticket: cmpforopenssl/feature-requests/43

tpank commented 6 years ago

Generally one should be careful how to implement that. It should likely best also consider potential x-signing and therefore not only include the chain to the currently used trust anchor but also incomplete potentally possible branches.

Original comment by: mpeylo

tpank commented 6 years ago

Diff:


--- old
+++ new
@@ -1,2 +1,5 @@
-This has been requested by Hendrik Brockhaus. 
-It can be implemented essentially by extending certConf_cb().
+Users or applications using the library should not be bothered with certificates needed only for protecting CMP messages.
+
+This be implemented essentially by extending certConf_cb().
+
+This has been requested originally by Hendrik Brockhaus. 

Original comment by: DDvO

tpank commented 6 years ago

Proposal for implementation within the library taking into account Martin's comment:

When returning the extraCerts received, exclude those certificates that have been used for validiating the CMP message protection but are not needed for validating the newly enrolled certificate.

Original comment by: DDvO

tpank commented 6 years ago

Original comment by: DDvO

tpank commented 6 years ago

Diff:


--- old
+++ new
@@ -1,5 +1,3 @@
-Users or applications using the library should not be bothered with certificates needed only for protecting CMP messages.
-
-This be implemented essentially by extending certConf_cb().
+Users or applications using the library should not be bothered with certificates needed only for checking the protection of CMP messages received. One could modifiy `CMP_CTX_extraCertsIn_get1()` not to return such such certifiactes, or add a new function called, e.g., `CMP_CTX_get1_newClChain()` that returns only the relavant portion of the extraCerts received. 

 This has been requested originally by Hendrik Brockhaus. 

Original comment by: DDvO

tpank commented 6 years ago

Original comment by: DDvO

DDvO commented 4 years ago

This issue can be traced back to a design weakness of CMP/RFC 4210: https://tools.ietf.org/html/rfc4210#section-5.1 just very vaguely describes the potential contents of the extraCerts field. Moreover, rather than separating the certificates used for signature-protecting a CMP message from the certificate chain(s) of a newly enrolled cert, CMP lumps them together in the extraCerts. Thus a CMP client typically needs to separate them again on receiving an IP/CP/KUP message. @HBrock, maybe there is a chance to solve this in the current update to the CMP RFC?

BTW, I recently filed a feature request to OpenSSL for a function that can be used conveniently and efficiently to construct the chain of a given certificate from a list of candidate intermediate certs and a trust store: https://github.com/openssl/openssl/issues/10696

HBrock commented 4 years ago

The goal of https://datatracker.ietf.org/doc/draft-brockhaus-lamps-cmp-updates/ is to introduce minimum updates without breaking backward compatibility. We don't want to release a version 3 of CMP. @DDvO If you have concrete suggestions within theses boundaries, please let me know.

Finally, don't you need a function to build a valid chain from a bunch of unstructured certificates anyway? With such function you could handle extraCerts as it is currently specified.

DDvO commented 4 years ago

Sure we should avoid incompatibilities with RFC 4210, i..e, the hitherto version 2 of CMP. I do have a concrete first idea how to achieve this:

Use the extraCerts field for signature-based message protection only, and use the caPubs sub-field of the body (of type CertRepMessage) of ir, cr, and kur messages for transporting the chain(s) of the new cert.

So far in RFC 4210 the use of the caPubs is only very sparsely defined, and the only (and optional) uses mentioned there are for a cert that may be directly trusted as a root CA certificate by the client. Since this cert (if present) is necessarily self-signed and the chains of the new cert better not include self-signed (root) certs it would be very simple to distinguish the old and the new uses of caPubs and even support them simultaneously in the same response message.

Yes, a chain building function is useful/needed anyway, in particular for computing the chain (unless pre-configured) of the client cert when signature-protecting request messages. Moreover, the CMP Client needs to be (backward) compatible with various CMP servers, which might make all kinds of uses of the extraCert field, including mixing the chains of the new cert and of the sender cert when signature-protecting their responses.

HBrock commented 4 years ago

Thank you for the suggestion. It looks like it is backward compatible. But RFC 4210 section 5.1. explicitly mentions extraCerts to provide CA certificates for the newly issued EE certificate. And I think this is also the current best practice of most implementations. This usage of extraCerts is also specified for LTE in 3GPP TS 33-310 section 9.5.4.3. Therefore, I think this change would bring incompatibilities with existing implementations.

DDvO commented 4 years ago

Well, the use of extraCerts for the chain of the new cert is mentioned in section 5.1 as an example but is not required (there is no MUST or the like there), yet it's correct that is has become common practice. On the other hand it also has become common practice to have protection-related certs in the extraCerts, and we cannot require to put those, e,g., into the caPubs field because this exists only for CertRepMessages. So restricting the use of the extraCerts to protection-related certs appears the only choice we have for separating the chains.

Any existing client anyway needs to cope with the situation that the extraCerts may or may not contain the chain of the new cert, except for implementations tailored specifically for 3GPP TS 33-310. That 3GPP spec has some further peculiarities for which CMPforOpenSSL has specific options.

We could at least add an optimization that tries to take the chain from the caPubs if it can be found there, and otherwise resorts to digging through the extraCerts and trying to chain up suitable certs.

DDvO commented 4 years ago

Here is another idea, which does not make use of caPubs and is more in line with the current practice using the extraCerts:

Here are two possible ways of marking the border:

I suppose this is fully backward compatible because the legacy clients will find all the info they expect in the extraCerts and will ignore the marker (whether it is an extra self-signed cert or an extra generalInfo entry that they do not know).

HBrock commented 4 years ago

I do see you wish to more clearly separate the chains of the newly issued certificate and the protection of the CMP message. I do like the extraCerts as it is implemented in ASN.1 today because it offers many flexibility of optimizations for constrained environments.

For compatibility with existing practice, you anyhow need to cope with extraCerts including both the protection certificate+chain and the chain of the newly issued certificate, To ease the parsing we require the CMP protection certificate to be the first certificate in extraCerts. But I am also not sure, if this is really needed, as the senderKID is in the CMP message header to easily identify the protection certificate.

@primetomas, what do you think? David struggles in OpenSSL with deriving the chains for the newly issued certificate and the CMP protection certificate from extraCerts and proposes some more specific stipulations in CMP Updates of the CMP Profile.

primetomas commented 4 years ago

Indeed, quite complex if the CMP message protection PKI and certificate issuing PKI are not the same.

I'm aware of caPubs being used to distribute updated trust anchors, i.e. new self signed CA certificates. I'm aware of 3GPP 9.5.4.3 on including the certificate issuer chain, including the the root shall be in there, albeit it is not protected...

Assuming that the trust anchor(s) have to be settled outside of extraCerts, there can still be two chains there, if we also assume that the RootCA is neither directly used for CMP message protection or for certificate issuance. It's hard to find an easy algorithm except building the path from issuerDN and sender/senderKID. The lightweight CMP profile could for sure profile that signer must be first, followed by certificate issuer, no roots.

We (EJBCA) adds to exteraCerts the signer chain first (including root), followed by additional certificates freely selectable by the administrator in the configuration. We (EJBCA) adds the certificate issuer CA as the first item in caPubs, followed by any CA certificates selected by administrator in the configuration. Duplicates are removed so it may be only one chain if signer and certificate issuer is the same.

I (and customers) was always confused by caPubs and extraCerts as it is specified in RFC4210 (and 3GPP for that matter). It would be better if if signer chain and issuer chain were in different fields (both protected) As now being bound by RFC4210 backwards compatibility...ugh. I could suggest profile extraCerts to only contain signer chain certificates, and caPubs issuer chain...but that's a breaking change...

Just combine caPubs and extraCerts (with roots only from caPubs) into one bucket and iterate figuring out the chains?

mpeylo commented 4 years ago

Just combine caPubs and extraCerts (with roots only from caPubs) into one bucket and iterate figuring out the chains?

I somewhat recall that there should already be a function which does exactly that... I think it uses OpenSSL's inbuilt chain-building function, and it will stop when it finds a valid chain up to a known trust anchor.

I'm not sure about a solution using extraCerts to explicitly communicate information about "the" chain for a certificate. Anyway, there might be several different chains when there's x-signing, the "correct" one depending on the trust anchors of the relying party.

If the client needs help to figure out the correct chain(s) for certificates, what about using the generalInfo field? A new generalInfo Extension could be defined that includes a list of KeyIdentifiers which build up a full chain for newly issued certificates (their issuer/serial should be also included).

Such new generalInfo extension would basically assist identifying all the (known) possible chains if the respective certificate profiles allow x-signing, i.e. in case not the whole chain uses AuthorityKeyIdentifier with authorityCertIssuer+authorityCertSerialNumber but only keyIdentifier. That way, clients could easily make sure they send all possibly needed CA certificate to peers e.g. during TLS/IKE handshakes.

But then, is there actually a real-life problem to solve here? I guess if clients habitually store all the "untrusted" valid CA certificates they eventually learn or, they can use them whenever they need to build up some chain for their certificates, and utilize them whenever they would need them to build up some chain for a certificate of a peer. There might be some storage resource constraints when it comes to storing such certificates?

DDvO commented 4 years ago

Thanks @primetomas and @mpeylo for sharing your thoughts and further solution ideas on this issue and describing the current implementation in EJBCA.

I somewhat recall that there should already be a function which does exactly that... I think it uses OpenSSL's inbuilt chain-building function, and it will stop when it finds a valid chain up to a known trust anchor

As mentioned above I've recently filed an OpenSSL feature request for an official(ly documented) function that can be used conveniently and efficiently to construct the chain of a given certificate from a list of candidate intermediate certs and a trust store: openssl#10696 - the currently used function is rather inefficient for the given purpose (as it also verifies the chain) and uses undocumented features.

I'm not sure about a solution using extraCerts to explicitly communicate information about "the" chain for a certificate. Anyway, there might be several different chains when there's x-signing, the "correct" one depending on the trust anchors of the relying party.

Yes, my solution suggestions have been taking this into account, by speaking of 'chain(s)'.

A new generalInfo Extension could be defined that includes a list of KeyIdentifiers which build up a full chain for newly issued certificates (their issuer/serial should be also included).

This would be (needlessly) complicated.

But then, is there actually a real-life problem to solve here?

Yes. For the client checking a signature-based protection of the response message I agree it does not hurt to make use of potentially all of the extraCerts plus any further untrusted certs the client may know already. The point here is an efficient way of determining the issuer chain(s) to be saved along with the newly issued cert.

From the feedback received so far including the remarks by @HBrock I conclude we should

This brings me back to one of my suggestions, which has not been commented on so far, and which can be refined as follows:

Thus the client can simply use potentially all of the extraCerts when validating a signature-based message protection, and when saving the newly issued cert with its chain(s) the client can simply copy all of the extraCerts in case the response message was not signature-protected and can otherwise copy the trailing portion of the extraCerts, as determined by the protection chain length value, which may be given in the generalInfo field.

primetomas commented 4 years ago

The sender should mark the border between these two types of chains by including in the response the length of the first (i.e., signer/protection) (sub-)chain, e.g., in the generalInfo field of the message header.

This information is redundant though isn't it? By traversing the chain you see when it is ending. Having to parse another field, and presumably check correctness of it you introduce yet another possible error. Is that a bad_message is the number in generalInfo is not correct or missing?

DDvO commented 4 years ago

This information is redundant though isn't it? By traversing the chain you see when it is ending.

No, because part of the protecting chain may be left out to avoid multiple inclusion of the same cert(s) and because the root cert is not included.

Having to parse another field, and presumably check correctness of it you introduce yet another possible error.

I'd say the introduction of a key-value pair in the generalInfo would not be much effort and is not very error-prone.

Is that a bad_message is the number in generalInfo is not correct or missing?

The correctness of the provided number would be the responsibility of the server, like it is already now its responsibility to provide the signer chain (if applicable) and the issuer chain(s). If the number is not given at all then we have the fallback situation that is needed for compatibility anyway: then the client needs to build the issuer chain(s) itself (which would of course incur the inefficiency and potentially incompleteness that we can avoid with the new convention).

mpeylo commented 4 years ago

A new generalInfo Extension could be defined that includes a list of KeyIdentifiers which build up a full chain for newly issued certificates (their issuer/serial should be also included).

This would be (needlessly) complicated.

I wouldn't be so sure if that is so needless if I understand what you want to achieve.

Consider:

I see two realistic possibilities for a somewhat safe solution:

That would btw also permit creation of a new general message to request all currently (known) possible chains for a certificate. Just in case such would be needed.

Standards-complying clients will ignore generalInfo extensions if they don't know them. Those who know it shall then be able to trust it's content - but might get confused if the lists have any inconsistencies.

Is that a bad_message is the number in generalInfo is not correct or missing?

The correctness of the provided number would be the responsibility of the server, [...]

However, the order and length of ExtraCerts is not protected by default, so the server has limited influence to ensure the number it had put is the correct one when the PKIMessage hits the client.