Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Singed message is not enforcing signature #56

Open
5 of 7 tasks
bwbroersma opened this issue Jul 17, 2023 · 6 comments
Open
5 of 7 tasks

Singed message is not enforcing signature #56

bwbroersma opened this issue Jul 17, 2023 · 6 comments

Comments

@bwbroersma
Copy link
Contributor

bwbroersma commented Jul 17, 2023

Singed message is not enforcing signature, e.g.:

-----BEGIN PGP SIGNED MESSAGE-----
Expires: 2024-01-01T00:00:00.000Z
Contact: https://www.example.org

This currently parses as valid with zero errors.

The issues for signed messages I can see:

  • hash-header not enforced (1*, so should be one or more)
  • no CRLF enforced (after hash-header)
  • armor-header (-----BEGIN PGP SIGNATURE-----) not enforced
  • no CRLF enforced (after either armor-keys or after the armor-header if no keys)
  • no data in signature enforces, no validation on valid base64
  • armor-tail (-----END PGP SIGNATURE-----) not explicitly enforced (only needed when armor-header is present, when armor-tail is missing, the error will be no_line_separators, because the last line is {'type': 'pgp_envelope'}, even when empty)
  • PGP Dash-Escaped Text is still parsed for pgp #54

The current code quite literally is this xkcd PGP 🙃:
PGP

@DigitalTrustCenter
Copy link
Owner

With the new release a pgp formatter checker is added using the PGPy module which checks the OpenPGP message specification in accordance with RFC 4880.
Any issue with the PGP message or signature would result in a pgp_data_error. If the message is not a valid pgp message it would result in a pgp_error.

@baknu
Copy link
Collaborator

baknu commented Oct 30, 2023

It is not clear to us (@mxsasha, @bwbroersma and me) what the exact difference is between pgp_data_error and pgp_error. Moreover, it is not clear to us how these new error messages relate to signed_format_issue. Could you elaborate?

@baknu baknu reopened this Oct 30, 2023
@DigitalTrustCenter
Copy link
Owner

We made 2 pgp error distinctions. These were made because the PGPy library also seems to make these distinction.
The first and most common will be the pgp_data_error. This means that there is an issue with the message formatting. So, for instance, missing information like the end message or other required fields or unexpected fields within the message.
The second error, pgp_error, occurs when there was an issue with the encoded data within the pgp message. So if there was an issue with decoding the data within the pgp armored block.

@baknu
Copy link
Collaborator

baknu commented Apr 9, 2024

Thanks for the explanation! Still some additional questions:

@DigitalTrustCenter
Copy link
Owner

If it is regarding a pgp signed message it will enforce the addition of a valid pgp signature. If it is not present it will throw a pgp_data_error. This includes a pgp signature that is not correctly formatted, so for instance no CRLF (or too many) after the armor header or no armor-tail will also throw a pgp_data_error. It will also validate the base64 data. If this is not valid it will give a pgp_error because it could not decode the pgp message.
There is currently no enforcment of the hash-header or the CRLF after the hash header. This is not listed as required in the RFC 4880 which describes the OpenPGP Message Format, but since seems to be a requirement listed in chapter 4 of rfc9116 we will add a check for this with a newer version.

@DigitalTrustCenter
Copy link
Owner

the signed_format_issue error only checks if the header is at the start of the security.txt. If this is not the case it will not throw a pgp_data_error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants