zmap / zlint

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

ISSUE: Time Stamping certificates with false warning results #602

Open sandorszoke opened 3 years ago

sandorszoke commented 3 years ago

Certificates with potential issues

As part of a wider test, we checked several of our Time Stamping certificates with Zlint ver.3.1.0 and we could identify two potential issues with the following certificates:

CERTIFICATE FILE NAME crt.sh link (if included) KEY TYPE QCSTATEMENTS
CodeSigning_e-Szigno_TSA_2020_01.cer xxx RSA2048 1, 2, 3, 5, 6.2 QUALIFIED SEAL
CodeSigning_e-Szigno_TSA_2020_01_2020-07-21.cer xxx RSA2048 1, 2, 3, 5, 6.2 QUALIFIED SEAL
CodeSigning_e-Szigno_TSA_2020_02.cer xxx RSA2048 1, 2, 3, 5, 6.2 QUALIFIED SEAL
e-Szigno_Qualified_TSA_2020_01.cer xxx ECC256 1, 2, 3, 5, 6.2 QUALIFIED SEAL
e-Szigno_Qualified_TSA_2020_01_2020-07-21.cer https://crt.sh/?id=3567896521 ECC256 1, 2, 3, 5, 6.2 QUALIFIED SEAL
e-Szigno_Qualified_TSA_2020_01--ECC.cer xxx ECC256 1, 2, 3, 5, 6.2 QUALIFIED SEAL
e-Szigno_Qualified_TSA_2020_02.cer xxx ECC256 1, 2, 3, 5, 6.2 QUALIFIED SEAL
Qualified_eIDAS_e-Szigno_TSA_2020_01.cer xxx RSA2048 1, 2, 3, 5, 6.2 QUALIFIED SEAL
Qualified_eIDAS_e-Szigno_TSA_2020_01_2020-07-21.cer https://crt.sh/?id=3567896520 RSA2048 1, 2, 3, 5, 6.2 QUALIFIED SEAL
Qualified_eIDAS_e-Szigno_TSA_2020_02.cer https://crt.sh/?id=4133632664 RSA3072 1, 2, 3, 5, 6.2 QUALIFIED SEAL

Only a few of them are available through crt.sh, but the issue is the same with all of them.

The summarized result of the zlint tests

The results of the zlint tests are sligthly different for the different TSA certificates, but they all contain the following problematic information:

LEVEL # OCCURRENCES DETAILS
info 1 w_ct_sct_policy_count_unsatisfied
warn 1 w_qcstatem_qctype_web
error 0 -
fatal 0 -

The use of Qualified certificate statements

All tested Time Stamping certificates contain the QCStatements extension in accordance with the following ETSI standard:

ETSI EN 319 412-5 V2.3.1 (2020-04) 4 Qualified certificate statements

"1.3.6.1.5.5.7.1.3 QCStatements" extension includes among others the following values:

QCStatements value QCStatements name Description
0.4.0.1862.1.1 id-etsi-qcs-QcCompliance QCStatement claiming that the certificate is a EU qualified certificate (no 0.4.0.1862.1.7 value)
0.4.0.1862.1.6 id-etsi-qcs-QcType QCStatement claiming that the certificate is a certificate of a particular type
0.4.0.1862.1.6.2 id-etsi-qct-eseal Certificate for electronic seals as defined in Regulation (EU) No 910/2014 (digital signature created by legal person)

All these certificates are qualified certificates for electronic seals in accordance with Regulation (EU) No 910/2014.

They all contain the "Time Stamping" EKU, so they are special seal certificates dedicated to issuing time stamps.

PROBLEM I., WARNING - "w_qcstatem_qctype_web"

We checked the scope of the lint "w_qcstatem_qctype_web" on GitHub:

The zlint documentation provides the following description regarding this lint:

Name:          "w_qcstatem_qctype_web",
Description:   "Checks that a QC Statement of the type Id-etsi-qcs-QcType features features at least the type IdEtsiQcsQctWeb",
Citation:      "ETSI EN 319 412 - 5 V2.2.1 (2017 - 11) / Section 4.2.3",
Our findings

The citation refers to an outdated version of the ETSI standard, the current version is:

If I understand the source code properly, this lint is dedicated only to verifying the TLS certificates, and the extension "id-etsi-qcs-QcType", if exists, should contain the following value:

    0.4.0.1862.1.6.3  "id-etsi-qct-web"
    if t.Equal(util.IdEtsiQcsQctWeb) {
        found = true
    }

In our Time Stamping certificates this assumption is not correct, so this lint should return with the value "Not Applicable".

Our proposal

If you agree with our interpretation we suggest to run this lint only in case of an SSL/TLS subscriber certificate. In case of any other certificate type ZLint should return with an "NA" output.

PROBLEM II., INFO - "w_ct_sct_policy_count_unsatisfied"

ZLint raised this issue with a lower weight, but the source of the problem can be the same as in case of the previous lint.

We checked the scope of the lint "w_ct_sct_policy_count_unsatisfied" on GitHub:

The ZLint documentation provides the following description regarding this lint:

Name:         "w_ct_sct_policy_count_unsatisfied",
Description:  "Check if certificate has enough embedded SCTs to meet Apple CT Policy",
Citation:     "https://support.apple.com/en-us/HT205280",
Our findings

This requirement exists only for TLS certificates as part of the Certificate Transparency.

In the case of any other type of certificates, it should return with the value "Not Applicable".

Our proposal

If you agree with our interpretation we suggest to run this lint only in case of an SSL/TLS subscriber certificate. In case of any other certificate type ZLint should return with an "NA" output.

General comment/question

Similar problem was raised in https://github.com/zmap/zlint/issues/581

The solution would probaly be to identify the type of the analysed certificate more precisely by making deeper tests of the different attributes.

We do not know the structure and operation of ZLint and what is the best way to do it.

cpu commented 3 years ago

@sandorszoke Thank you for the detailed description! :bow: Minor note: In the future I think we would prefer if you can keep to one problem per issue instead of describing two problems in one. It makes the follow-up discussion easier :-)

Problem 1

w_qcstatem_qctype_web has been flagged in a few other issues at this point (e.g. https://github.com/zmap/zlint/issues/561, https://github.com/zmap/zlint/issues/581). My position remains that we should be removing these lints rather than trying to fix them (I think that position is also held by @zakird and @sleevi based on the discussion in #581). I understand there's interest from DigiCert / @christopher-henderson to step up to keep these lints in the codebase and I think the best path forward is to:

  1. remove the lints - they're buggy as is.
  2. sketch out a path forward w.r.t bugfixes and ongoing maintenance in a planning issue, likely building on the WIP profile support in #595
  3. implement the plan, re-adding lints (probably in an opt-in profile) once they are up to snuff.

Problem 2

I think this is a case where ZLint's use-case has always been certificates in the web PKI and you're seeing a mismatch using it with another domain. I don't think we should add logic to our existing lints for this and instead you should exclude the lints that aren't appropriate to your context instead of using the defaults. If the profile support in #595 lands there might be a future where we can do some of that work as a community with a custom profile but I'm a little bit hesitant to see the scope of the project creep. There's lots of work for a small number of people just sticking to the web PKI.

If you're running zlint from the command line you can exclude this lint with:

zlint -excludeNames="w_ct_sct_policy_count_unsatisfied" ...

If you're using ZLint programmatically you'll want something like:

registry, _ := lint.GlobalRegistry().Filter(lint.FilterOptions{
  ExcludeNames: []string{"w_ct_sct_policy_count_unsatisfied"},
})
zlintResultSet := zlint.LintCertificateEx(parsed, registry)
zakird commented 3 years ago

You could also exclude lints from specific sources (e.g., Apple Root Store Program and Baseline requirements). This is likely easier than trying to exclude specific named lints.

christopher-henderson commented 3 years ago

@cpu with regard to #595 I am also cooking up some code that should allow for lints to accept arbitrary contexts that contain information that is out-of-band of the certificate itself

[e_some_lint]
the_stock_market_was_up_that_year = true
struct eSomeLint {
    theStockMarketWasUpThatYear bool
}

func (e *eSomeLint) Lint(cert *x509.Certificatge) *lint.LintResult {
    if !e.theStockMarketWasUpThatYear {
        return &lint.LintResult{Status: lint.Error, Details: "oh no!"}
    }
    return &lint.LintResult{Status: lint.Pass}
}

Of course there are details to hammer out (not the least of which are global configurables that are easy to maintain without being dynamic and complications arising from runs of ZLint having global state which does not work well for unit tests). But it should help out with issues like this.

sandorszoke commented 3 years ago

@cpu OK, next time I will open separate issues for each infected lint.

@zakird You are right, this is probably the best solution. We can omit the entire source by using -excludeSources instead of dealing with the individual lints separately.