zmap / zlint

X.509 Certificate Linter focused on Web PKI standards and requirements.
https://zmap.io
Apache License 2.0
361 stars 110 forks source link

Qcstatem psd2 national scheme #861

Open mtgag opened 5 months ago

mtgag commented 5 months ago

Following discussion at https://github.com/zmap/zlint/issues/847.

This one is a PSD2 related lint.

defacto64 commented 5 months ago

@mtgag If I am not mistaken, this lint checks that, in case the organizationIdentifier contains a "locally defined identity type reference" (i.e. it starts with two characters followed by ":"), the QcStatement id-qcs-pkixQCSyntax-v2 is present and contains at least one element of type URI. This is fine, as it is in line with ETSI EN 319 412-1 (§5.1.4), however it's not a specific requirement for PSD2 certificates, but for all qualified certificates.

mtgag commented 5 months ago

@mtgag If I am not mistaken, this lint checks that, in case the organizationIdentifier contains a "locally defined identity type reference" (i.e. it starts with two characters followed by ":"), the QcStatement id-qcs-pkixQCSyntax-v2 is present and contains at least one element of type URI. This is fine, as it is in line with ETSI EN 319 412-1 (§5.1.4), however it's not a specific requirement for PSD2 certificates, but for all qualified certificates.

In check applies it also checks that the certificate has the 0.4.0.19495.2 OID in QcStatements:

    _, isPresent := util.IsQcStatemPresent(c, &util.IdEtsiPsd2Statem)
    if !isPresent {
        return false
    }

in order to be triggered only for such certificates. What would be the proper check? IsAnyEtsiQcStatementPresent (cf. lint_qcstatem_etsi_present_qcs_critical.go)?

defacto64 commented 4 months ago

@mtgag , sorry for the wait.

What I meant to say is that this check should be carried out regardless of whether the certificate is of PSD2 type or not, as it is a requirement that applies to all types of qualified certificates. At least, this is my understanding of ETSI EN 319 412-2, and I may well be wrong. If I am right, this lint should (ideally, IMO) not care if the certificate being linted contains the PSD2 QcStatement or not, even less parse it, but just verify if it is a qualified certificate (of any type). From this perspective - or perhaps in any case - I think the correct normative reference for this particular lint should be ETSI EN 319 412-1 §5.1.4 and not ETSI TS 119 495 §5.2.1.

Still from this perspective, some of the code and test files attached to this PR are perhaps, with all due respect, a little overabundant, as the check in question could be achieved in a more streamlined way.

I would like to clarify that IMO there is nothing wrong in doing this check on a PSD2 certificate, but only that it would be worth doing it in a more generalized way given that it is for a requirement that applies to a much broader set of certificates that not just the PSD2 ones, so why not try to hit many more birds with one stone? (apologies to the birds)

mtgag commented 4 months ago

@mtgag , sorry for the wait.

What I meant to say is that this check should be carried out regardless of whether the certificate is of PSD2 type or not, as it is a requirement that applies to all types of qualified certificates. At least, this is my understanding of ETSI EN 319 412-2, and I may well be wrong. If I am right, this lint should (ideally, IMO) not care if the certificate being linted contains the PSD2 QcStatement or not, even less parse it, but just verify if it is a qualified certificate (of any type). From this perspective - or perhaps in any case - I think the correct normative reference for this particular lint should be ETSI EN 319 412-1 §5.1.4 and not ETSI TS 119 495 §5.2.1.

Still from this perspective, some of the code and test files attached to this PR are perhaps, with all due respect, a little overabundant, as the check in question could be achieved in a more streamlined way.

I would like to clarify that IMO there is nothing wrong in doing this check on a PSD2 certificate, but only that it would be worth doing it in a more generalized way given that it is for a requirement that applies to a much broader set of certificates that not just the PSD2 ones, so why not try to hit many more birds with one stone? (apologies to the birds)

Thank you for the feedback. I will take a deeper look into the comments, implementation, test data, and so on. Currently, I am quite busy, and the holiday season is about to start, so I will probably get back to it in a few weeks.

mtgag commented 2 months ago

Coming back to this issue. Please excuse the delayed answer.

On page 9 of https://www.etsi.org/deliver/etsi_en/319400_319499/31941202/02.02.01_60/en_31941202v020201p.pdf the content of the subject is discussed. As far as organizationIdentifier and serialNumber concerning it states:

"Certificates may include one or more semantics identifiers as specified in ETSI EN 319 412-1 [i.4], clause 5 which defines the semantics for the organizationIdentifier attribute.

.....

Certificates may include one or more semantics identifiers as specified in ETSI EN 319 412-1 [i.4], clause 5 which define the semantics for the serialNumber attribute."

Therefore, the special format applies only when specific semantics identifiers are present and not to all qualified certificates.

I believe, that if it implemented as proposed in the following we would cover all cases where the ETSI specification is explicit and thus avoid any false positives:

IF [ When the natural person semantics identifier is included (5.1.3, https://www.etsi.org/deliver/etsi_en/319400_319499/31941201/01.04.04_60/en_31941201v010404p.pdf) AND two characters according to local ... are present in serialNumber ] OR [ When the legal person semantics identifier is included (5.1.4, https://www.etsi.org/deliver/etsi_en/319400_319499/31941201/01.04.04_60/en_31941201v010404p.pdf) AND two characters according to local ... are present in organizationIdentifier] ] OR [ If a PSD2 Authorization Number was issued by the NCA (5.2.1,GEN-5.2.1-2, https://www.etsi.org/deliver/etsi_ts/119400_119499/119495/01.04.01_60/ts_119495v010401p.pdf) AND two characters according to local ... are present in organizationIdentifier] ] THEN check that the remainder of the string also fulfills the national scheme syntax.

What do you think? If it is OK. I could start rewriting this lint.

mtgag commented 2 weeks ago

There was no feedback on this ticket since several weeks. Is this issue still relevant for review/inclusion? If not, I believe we could close this PR.

christopher-henderson commented 2 weeks ago

@defacto64 do you still have a particular opinion on this one?

defacto64 commented 2 weeks ago

Sorry for the delay, unfortunately I can only dedicate a very small fraction of my time to this project. I will give a feedback by tomorrow at the latest.

defacto64 commented 2 weeks ago

@mtgag: could you repost your pseudo-code with some indentation so that the processing flow is better understood?

mtgag commented 2 weeks ago

@mtgag: could you repost your pseudo-code with some indentation so that the processing flow is better understood?

Sure:

IF
   [ (natural person semantics identifier is included) AND (two characters according to local ... are present in serialNumber) ] 
    OR 
   [ (natural person semantics identifier is included) AND (two characters according to local ... are present in organizationIdentifier) ] 
    OR 
   [ (a PSD2 Authorization Number was issued by the NCA) AND (two characters according to local ... are present in organizationIdentifier) ]
THEN
    check that the remainder of the string also fulfills the national scheme syntax.

I left the references out to focus on the pseudo-code, the references to support this are found here https://github.com/zmap/zlint/pull/861#issuecomment-2323910969

defacto64 commented 2 weeks ago

Your pseudo code seems ok to me, at least the first two conditions.... not sure about the third.

It still does not catch non-PSD2 certificates that include the legal person semantics identifier and which therefore are subject to requirement LEG-5.1.4-05 of ETSI EN 319 412-1. Therefore, if the organizationIdentifier in those certificates starts with "two characters etc.", it would be good to "check that the remainder of the string also fulfills the national scheme syntax" in those certificates as well.

Besides, it seems to me (perhaps I am missing some cross reference) that ETSI TS 119 495, although requiring that the organizationIdentifier be encoded according to the syntax defined in clause 5.1.4 (Legal person semantics identifier) of ETSI EN 319 412-1, does not require the the certificate includes such semantics identifier. If I was right, than your third condition would be wrong..... frankly, I am not sure at this time. However, if indeed PSD2 certificates are required to include the Legal person semantics identifier when their organizationIdentifier is encoded according to a national scheme, then they would still be linted by the pseudo code that I am suggesting below.

Having said that, I would suggest to change the condition for the third case in order to check compliance to requirement LEG-5.1.4-05 of ETSI EN 319 412-1, which certainly applies to both PSD2 and non-PSD2 certificates, like follows:

IF
   [ (natural person semantics identifier is included) AND (two characters according to local ... are present in serialNumber) ] 
    OR 
   [ (natural person semantics identifier is included) AND (two characters according to local ... are present in organizationIdentifier) ] 
    OR 
   [ (legal person semantics identifier is included) AND (two characters according to local ... are present in organizationIdentifier) ]
THEN
    check that the remainder of the string also fulfills the national scheme syntax.

How about that?

Please note that this are just my own personal remarks, that I am expressing here in a personal capacity, and it is not my intent to oppose to this PR.

mtgag commented 1 week ago

There was a transfer error from the first pseudocode here to the second simplified version: It should be:

IF
   [ (natural person semantics identifier is included) AND (two characters according to local ... are present in serialNumber) ] 
    OR 
   [ (*legal* person semantics identifier is included) AND (two characters according to local ... are present in organizationIdentifier) ] 
    OR 
   [ (a PSD2 Authorization Number was issued by the NCA) AND (two characters according to local ... are present in organizationIdentifier) ]
THEN
    check that the remainder of the string also fulfills the national scheme syntax.

The corrected part is exactly your proposal which matches the first pseudo-code. Therefore, I think now everything is on the right track. Let us skip the PSD part and just check the natural and legal person aspects. So the final check is:

IF
   [ (natural person semantics identifier is included) AND (two characters according to local ... are present in serialNumber) ] 
    OR 
   [ (legal person semantics identifier is included) AND (two characters according to local ... are present in organizationIdentifier) ] 
THEN
    check that the remainder of the string also fulfills the national scheme syntax.

Ok?

defacto64 commented 1 week ago

Well, if that's the case, then it's fine with me.

mtgag commented 1 week ago

Well, if that's the case, then it's fine with me.

Great. I will start working on this and come back to you for a review if this is fine.