Missing fingerprint in PMD leads to an error

[tschmidtb51]

Today, I came across a weird report regarding OpenPGP stating:

Requirement 20: Public OpenPGP Key (failed)
    - ERROR: Fingerprint of public OpenPGP key https://example.com/.well-known/csaf/openpgp/4C071FE3584D1E7CD70FE8B2CF9436A769B3A45B.asc does not match remotely loaded.
    - INFO: No OpenPGP keys loaded.

In the same report, it says:

Requirement 19: Signatures
    - INFO: All signatures verified.

That does not make sense to me: Either the OpenPGP key couldn't be loaded (then one can't check signatures) or the OpenPGP key was loaded. Also, it is not quite clear to me what "Fingerprint of public OpenPGP key https://example.com/.well-known/csaf/openpgp/4C071FE3584D1E7CD70FE8B2CF9436A769B3A45B.asc does not match remotely loaded." mean. Where does the fingerprint come from?

(Sorry, I needed to redact some values for privacy concerns.)

[bernhardreiter]

The message comes from our source code, here are the relevant places

https://github.com/csaf-poc/csaf_distribution/blob/8feddc70e1c945e2cf2ec8cab92525aa8e89106d/cmd/csaf_checker/processor.go#L1521-L1524

https://github.com/csaf-poc/csaf_distribution/blob/8feddc70e1c945e2cf2ec8cab92525aa8e89106d/cmd/csaf_downloader/downloader.go#L369-L374

[bernhardreiter]

The code compares that for a PMD section like this:

{
 "public_openpgp_keys": [
    {
      "fingerprint": "804FED63730227FF2FB6D9712EA2477380F3EDCB",
      "url": "https://intevation.de/.well-known/csaf/openpgp/804FED63730227FF2FB6D9712EA2477380F3EDCB.asc"
    }
  ]
}

the fingerprint calculated from the downloaded file from the URL matches the given string for "fingerprint":. The matching rules are "a general form of case-insensitivity" (https://pkg.go.dev/strings#EqualFold). A widely used implementation like GnuPG accepts more fingerprint formats. So this is a possible cause for the missmatch (but would not be allowed by the schema, see below). A further analysis should do the comparison manually.

[bernhardreiter]

The following patch adds more detail to the checker output:

diff --git a/cmd/csaf_checker/processor.go b/cmd/csaf_checker/processor.go
index 451a315..b0929a8 100644
--- a/cmd/csaf_checker/processor.go
+++ b/cmd/csaf_checker/processor.go
@@ -1519,7 +1519,7 @@ func (p *processor) checkPGPKeys(_ string) error {
                }

                if !strings.EqualFold(ckey.GetFingerprint(), string(key.Fingerprint)) {
-                       p.badPGPs.error("Fingerprint of public OpenPGP key %s does not match remotely loaded.", u)
+                       p.badPGPs.error("Given Fingerprint ('%s') of public OpenPGP key %s does not match remotely loaded ('%s').", string(key.Fingerprint), u, ckey.GetFingerprint())
                        continue
                }
                if p.keys == nil {

Edit: %q should be used instead of '%s' as minor improvement.

[bernhardreiter]

Looking at https://docs.oasis-open.org/csaf/csaf/v2.0/provider_json_schema.json

    "public_openpgp_keys": {
      "title": "List of public OpenPGP keys",
      "description": "Contains a list of OpenPGP keys used to sign CSAF documents.",
      "type": "array",
      "items": {
        "title": "PGP keys",
        "description": "Contains all information about an OpenPGP key used to sign CSAF documents.",
        "type": "object",
        "required": [
          "url"
        ],
        "properties": {
          "fingerprint": {
            "title": "Fingerprint of the key",
            "description": "Contains the fingerprint of the OpenPGP key.",
            "type": "string",
            "minLength": 40,
            "pattern": "^[0-9a-fA-F]{40,}$"
          },
          "url": {
            "title": "URL of the key",
            "description": "Contains the URL where the key can be retrieved.",
            "$ref": "#/$defs/url_t"
          }
        }
      }
    }

The schema does not seem to mandate the fingerprint, which is in contrast to Example 124 (see https://docs.oasis-open.org/csaf/csaf/v2.0/os/csaf-v2.0-os.html#717-requirement-7-provider-metadatajson) which is labled Minimal but contains a fingerprint value.

So either the Standard needs a small correction or the schema.

The checker acts more like the standard.

[bernhardreiter]

The checker will check all signatures against the pubkeys provided in the PMD. So the message - INFO: All signatures verified. means that all found documents were verified fine against the pubkeys.

@tschmidtb51 are all questions answered for you?

As followup and part of this issue should we:

• Consider adding a patch like the above one for more verbosity?
• Report the standard contradiction from above to the TC?

[tschmidtb51]

@tschmidtb51 are all questions answered for you?

Yes. Thank you for the fast analysis.

As followup and part of this issue should we:

• Consider adding a patch like the above one for more verbosity?

Yes. Please do so.

• Report the standard contradiction from above to the TC?

Already done in https://github.com/oasis-tcs/csaf/issues/764 (I took the liberty to copy your analysis.).

[bernhardreiter]

As https://github.com/oasis-tcs/csaf/issues/764 says:

It must be an oversight, that the fingerprint is not required per schema.

we will turn a missing fingerprint into a warning.

So we have to change the implementation in two ways: a) change error to warning b) give more details (like patch above)

And checking that it works in all places where this is considered, at least checker and downloader.

[bernhardreiter]

558 is back to draft, as we need a version without API change for 3.x.

And a test is needed if an empty fingerprint is flagged as error correctly by the schema check. If the empty fingerprint is correctly flagged, then we would need no API change.

[bernhardreiter]

@koplas thanks for doing the test in #558. The test shows that an empty fingerprint will already hit the schema test barrier.

Can you do a merge request that does not change the public API, but adds the additional details for a missing or unmatching fingerprint next? That would be a siginificant improvement for the analysis and we can release it with the next 3.x release.

We should then open a new issue with the wish to improve diagnostics when there is an empty fingerprint and discuss technical options there. One option is to see if we can get more details about the failed schema check to users.