zlint E qcstatem correct national scheme

This PR replaces the previous PR (PR https://github.com/zmap/zlint/pull/861) based on our discussion there. I will close the original PR so we can continue work here.

This lint implements the following pseudocode:

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.

May 07 '25 12:05 mtgag

Thank you @mtgag! I'll start reviewing this (which will likely take longer than usual because we have historical context to catch up on).

Including @defacto64 because you were party to the conversation last fall in #861

May 11 '25 15:05 christopher-henderson

Hello!

My first observation is that this PR includes quite a bit of code that is not really needed for the specific lint proposed. I am referring to a number of structs and utility functions (mostly in the qc_stmt.go source) related to the etsi-psd2-qcStatement (from ETSI TS 119 495), which would be useful for additional lints that @mtgag will presumably propose later, but not for this one here. Personally I have no objection to this "anticipated" utility code, from a technical point of view. However, I do have some general doubts in the prospect of PSD2-oriented lints, because ETSI TS 119 495 presents some "undeterminacies" that IMO are not a good basis on which to build certificate lints. But we can discuss this later, case by case, if and when some lints based on ETSI TS 119 495 will be proposed.

Apart from the said additions, the rest of the changes made to the qc_stmt.go source are basically an enhancement of the util.ParseQcStatem() function in order to also handle two additional QcStatements, namely qcs-pkixQCSyntax-v2 (needed for this lint) and etsi-psd2-qcStatement (not needed for this lint).

On this specific lint I have a couple of doubts:

it seems to me the two functions CheckApplies() and Execute() do almost the same thing, that is, they check if there is a semanticsIdentifier and if the subject identifier (be it serialNumber or orgId) starts with two characters followed by a ":"; the only difference is that Execute() also checks if after the ":" there are two other characters and a "-". This is not wrong, but it means that you parse the QcStatements twice and in particular the qcs-pkixQCSyntax-v2 element; the whole thing could be simplified, without impacting performance, by differentiating the tasks of the two functions.
this lint fails to verify that, if the subject identifier (serialNumber or orgId) uses a "national scheme", the NameRegistrationAuthority be also present as prescribed by ETSI EN 319 412-1:

When a locally defined identity type reference is provided (two characters followed by ":"), the nameRegistrationAuthorities element of SemanticsInformation (IETF RFC 3739 [1]) shall be present and shall contain at least a uniformResourceIdentifier generalName.

This seems more important to me than verifying that the subject identifier has two characters and a "-" after the ":".

May 13 '25 09:05 defacto64

Hello!

My first observation is that this PR includes quite a bit of code that is not really needed for the specific lint proposed. I am referring to a number of structs and utility functions (mostly in the qc_stmt.go source) related to the etsi-psd2-qcStatement (from ETSI TS 119 495), which would be useful for additional lints that @mtgag will presumably propose later, but not for this one here. Personally I have no objection to this "anticipated" utility code, from a technical point of view. However, I do have some general doubts in the prospect of PSD2-oriented lints, because ETSI TS 119 495 presents some "undeterminacies" that IMO are not a good basis on which to build certificate lints. But we can discuss this later, case by case, if and when some lints based on ETSI TS 119 495 will be proposed.

I am a bit concerned as well and not entirely satisfied with the current code. It might be useful in the future, but it is difficult to estimate how much effort we could invest in the ETSI lints. If you or the project team do not consider this a showstopper, I suppose we can proceed as is. Otherwise, I suggest we close this PR for now until I have the opportunity to refactor the code.

Apart from the said additions, the rest of the changes made to the qc_stmt.go source are basically an enhancement of the util.ParseQcStatem() function in order to also handle two additional QcStatements, namely qcs-pkixQCSyntax-v2 (needed for this lint) and etsi-psd2-qcStatement (not needed for this lint).

On this specific lint I have a couple of doubts:
* it seems to me the two functions `CheckApplies()` and `Execute()` do almost the same thing, that is, they check if there is a semanticsIdentifier and if the subject identifier (be it serialNumber or orgId) starts with two characters followed by a ":"; the only difference is that `Execute()` also checks if after the ":" there are two other characters and a "-". This is not wrong, but it means that you parse the QcStatements twice and in particular the **qcs-pkixQCSyntax-v2** element; the whole thing could be simplified, without impacting performance, by differentiating the tasks of the two functions.

We could simply check whether IdQcsPkixQCSyntaxV2 is present and return true from CheckApplies. Would that resolve the issue?

* this lint fails to verify that, if the subject identifier (serialNumber or orgId) uses a "national scheme", the NameRegistrationAuthority be also present as prescribed by ETSI EN 319 412-1:
When a locally defined identity type reference is provided (two characters followed by ":"), the nameRegistrationAuthorities element of SemanticsInformation (IETF RFC 3739 [1]) shall be present and shall contain at least a uniformResourceIdentifier generalName.

This seems more important to me than verifying that the subject identifier has two characters and a "-" after the ":".

When I reviewed the specification a few days ago, it also seemed more important to me. This should be implemented as a new lint.

May 13 '25 10:05 mtgag

My first doubt on this specific lint ("On this specific lint I have a couple of doubts...") is mainly a matter of taste: I tend to avoid doing the same thing twice if it's not really necessary, even when the performance impact is negligible. If it was me to code this lint, I would somehow "memorize" some of the results of the checks made in CheckApplies() for subsequent use by Execute(). Not all the results, but only the parts that are strictly necessary to Execute(), based on the fact that if Execute() is invoked than the certificate necessarily has the features that were checked for by CheckApplies(). For instance, one could store some info into some private field(s) of the qcStatemNationalScheme struct. After all, Zlint only processes one certificate at a time, so this should not have unwanted side-effects. But again, it's a matter of personal taste and you are not supposed to share my personal inclinations. 😀 In any case, for questions of style it is better to refer to @christopher-henderson's opinion.

By the way, @mtgag: would you point us to some real certificates that would not pass this lint (AS IS) ?

May 14 '25 09:05 defacto64

I've been thinking about this lint a bit more and have one more point to make, in addition to what I said above. Let me be clear that this is just my personal opinion, and @christopher-henderson might see it differently.

This lint performs a check that is a subset of a broader check that I think would be worth performing instead. In fact, this lint only considers the so-called "local scheme", but I see no good reason to limit it to that scheme, especially considering that the "local scheme" entails additional requirements that are better covered by a separate lint. According to ETSI EN 319 412-1, paragraphs 5.1.3 and 5.1.4, if the certificate contains one of the two semantic identifiers defined there (i.e. Natural or Legal), then the serialNumber attribute or the organizationaldentifier attribute (as applicable) SHALL (uppercase mine) be encoded as specified there. And since N different schemes are allowed (including the "local" one), in my opinion it would be worth checking all the allowed possibilities, i.e. without limiting oneselves to the "local scheme", but rather validating the appropriate attribute (i.e. organizationalidentifier or serialNumber) with suitable regexps corresponding to all the allowed schemes (i.e. one for the VAT scheme, one for the NTR scheme, one for the PSD scheme, one for the "local scheme", and so on). What do you think?

May 15 '25 14:05 defacto64

If I am understanding your thinking @defacto64, you would like CheckApplies to mutate the struct so that it can remember the result of that operation in Execute.

Rust is, perhaps, a better tool to visualize this difference since it has explicit, enforced, mutability rules. Essentially, it would be the difference between...

// Immutable
fn check_applies(&self, cert: &Certificate) -> bool {}

// Mutable
fn check_applies(&mut self, cert: &Certificate) -> bool {}

After all, Zlint only processes one certificate at a time

This is certainly true of the CLI usage of ZLint. However, I too frequently forget that CAs use ZLint as a library.

Given that we store these lint structs in a global registry, that means that we have invited the devil into our house - Global Mutable State. When operating in "library mode", it would be possible for one run of a lint to poison consequent runs.

As such, I think that I would prefer it if we simply eat the computational cost and keep these structs as stateless as is reasonable.

With regard to dead code util functions, it's more a matter of reviewability than anything else. Dead code in a compile language is trivial to identify, so if we fast forward several years and find that these functions are still unused then it is easy enough to delete them. Better to have it and not need it....

Although it does make code reviews more difficult, which then reduces due diligence from reviewers (we are all human, after all).

With regard to the interpretation of the requirements, I admit that I am quite far from caught up on these documents. @mtgag it sounds like you are not quite confident in these changes either, and that you would like to put this lint back into the oven?

May 18 '25 16:05 christopher-henderson

If I am understanding your thinking @defacto64, you would like CheckApplies to mutate the struct so that it can remember the result of that operation in Execute.

Rather than wanting it, let's say I had hypothesized it, but what you say convinces me that it would not be a good idea. I also forgot that ZLint is also used as a library.

May 19 '25 08:05 defacto64

I've been thinking about this lint a bit more and have one more point to make, in addition to what I said above. Let me be clear that this is just my personal opinion, and @christopher-henderson might see it differently.

This lint performs a check that is a subset of a broader check that I think would be worth performing instead. In fact, this lint only considers the so-called "local scheme", but I see no good reason to limit it to that scheme, especially considering that the "local scheme" entails additional requirements that are better covered by a separate lint. According to ETSI EN 319 412-1, paragraphs 5.1.3 and 5.1.4, if the certificate contains one of the two semantic identifiers defined there (i.e. Natural or Legal), then the serialNumber attribute or the organizationaldentifier attribute (as applicable) SHALL (uppercase mine) be encoded as specified there. And since N different schemes are allowed (including the "local" one), in my opinion it would be worth checking all the allowed possibilities, i.e. without limiting oneselves to the "local scheme", but rather validating the appropriate attribute (i.e. organizationalidentifier or serialNumber) with suitable regexps corresponding to all the allowed schemes (i.e. one for the VAT scheme, one for the NTR scheme, one for the PSD scheme, one for the "local scheme", and so on). What do you think?

My first doubt on this specific lint ("On this specific lint I have a couple of doubts...") is mainly a matter of taste: I tend to avoid doing the same thing twice if it's not really necessary, even when the performance impact is negligible. If it was me to code this lint, I would somehow "memorize" some of the results of the checks made in CheckApplies() for subsequent use by Execute(). Not all the results, but only the parts that are strictly necessary to Execute(), based on the fact that if Execute() is invoked than the certificate necessarily has the features that were checked for by CheckApplies(). For instance, one could store some info into some private field(s) of the qcStatemNationalScheme struct. After all, Zlint only processes one certificate at a time, so this should not have unwanted side-effects. But again, it's a matter of personal taste and you are not supposed to share my personal inclinations. 😀 In any case, for questions of style it is better to refer to @christopher-henderson's opinion.

By the way, @mtgag: would you point us to some real certificates that would not pass this lint (AS IS) ?

I am afraid I cannot provide any real world certificates. Also the test corpus does not contain one. It would be quite interesting to locate a few ETSI/PSD certificates to run some tests. Trying https://crt.sh/?CAName=%25QSEAL%25 or https://crt.sh/?CAName=%25PSD%25 does locate a few CAs, but as far as I can tell they have not issued many certificates.

May 19 '25 08:05 mtgag

I've been thinking about this lint a bit more and have one more point to make, in addition to what I said above. Let me be clear that this is just my personal opinion, and @christopher-henderson might see it differently.

This lint performs a check that is a subset of a broader check that I think would be worth performing instead. In fact, this lint only considers the so-called "local scheme", but I see no good reason to limit it to that scheme, especially considering that the "local scheme" entails additional requirements that are better covered by a separate lint. According to ETSI EN 319 412-1, paragraphs 5.1.3 and 5.1.4, if the certificate contains one of the two semantic identifiers defined there (i.e. Natural or Legal), then the serialNumber attribute or the organizationaldentifier attribute (as applicable) SHALL (uppercase mine) be encoded as specified there. And since N different schemes are allowed (including the "local" one), in my opinion it would be worth checking all the allowed possibilities, i.e. without limiting oneselves to the "local scheme", but rather validating the appropriate attribute (i.e. organizationalidentifier or serialNumber) with suitable regexps corresponding to all the allowed schemes (i.e. one for the VAT scheme, one for the NTR scheme, one for the PSD scheme, one for the "local scheme", and so on). What do you think?

You may find some interesting lints about ETSI/PSD on this branch https://github.com/mtgag/zlint/tree/all/v3/lints/etsi. This is the list of new lints which are not in the main project:

e_ev_ntr_subject_jurisdiction_serial
e_ev_orgid
e_ev_orgid_encoding
e_ev_orgid_well_formed
e_ev_orgidext_matches_subject
e_ev_orgidext_present_mandatory
e_qcstatem_psd2_national_scheme (has been completely refactored in this PR)
e_qcstatem_psd2_orgid
e_qcstatem_psd2_policy_mandatory
w_qcstatem_psd2_policy_recommended
e_qcstatem_psd2_psd2statem_encoding
w_qcstatem_psd2_psd2statem_ncaid_eulist
e_qcstatem_psd2_psd2statem_ncaid_format
e_qcstatem_psd2_psd_subject
e_qcstatem_psd2_roles
e_qcstatem_psd2_vat_ntr
e_qcstatem_repeated_stmt

You may take a quick view to see if any lint addresses your suggestion. We can then reactivate it and suggest a PR. We could also just try to integrate all of them into the project. If I have a significant amount of ETSI certificates I could test the lints against them to assess their quality.

May 19 '25 08:05 mtgag

With regard to the interpretation of the requirements, I admit that I am quite far from caught up on these documents. @mtgag it sounds like you are not quite confident in these changes either, and that you would like to put this lint back into the oven?

I would prefer to test this and other ETSI lints against real certificates as @defacto64 suggested. Please let us wait, if we can locate a good number of ETSI certificates to test against.

May 19 '25 08:05 mtgag