-
Notifications
You must be signed in to change notification settings - Fork 62
Test Suite: clean up interactions with gpg #2367
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
Open
dkg
wants to merge
14
commits into
rnpgp:release/0.x
Choose a base branch
from
dkg:test-suite-gpg-cleanup
base: release/0.x
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When cipher is None, if encrypting fails, throwing an exception itself fails because you can't append None to a string with the + operator. Handle this error case cleanly.
In GnuPG's DETAILS, it says about field 10 (the User ID field of GnuPG colon-delimited output: The value is quoted like a C string to avoid control characters The previous decode_string_escape() function was more complex than it needed to be, and it also didn't cover certain kinds of legitimate C escape sequences. This should be simpler and more robust.
Programmatically parsing the output of gpg --list-keys is explicitly discouraged by gpg upstream. The status file output is designed to be parsed, however (see DETAILS.gz in the GnuPG documentation), and it is automatically produced during --generate-key. This drops one unnecessary invocation of gpg in the test suite.
gpg(1) says: ``` --compress-algo name Use compression algorithm name. "zlib" is RFC-1950 ZLIB compression. "zip" is RFC-1951 ZIP compression which is used by PGP. "bzip2" is a more modern compression scheme that can compress some things better than zip or zlib, but at the cost of more memory used during compression and decompression. "uncompressed" or "none" disables compression. If this option is not used, the default behavior is to examine the recipient key preferences to see which algorithms the recipient supports. If all else fails, ZIP is used for maximum compatibility. ``` The fact that gpg accepts numeric values as an argument for this option is undocumented/accidental. There is no need to supply anything other than the name.
gpg documents symmetric ciphers as aes, aes192, and aes256. While some versions of gpg offer an alias of "aes128" to mean the same thing as "aes", not all of them do. All of them implement "aes" itself.
It is strongly deprecated to operate (sign, encrypt, verify, decrypt) with 1024-bit RSA at this point: https://www.rfc-editor.org/rfc/rfc9580.html#section-12.4-3 This way we expect success from gpg only from more modern RSA.
RSA 1024 is strongly deprecated, and tests in general shouldn't depend on those keys.
The old signatures were made from 1024-bit RSA. It would not be unreasonable for gpg to decline to verify signatures from 1024-bit RSA. Make sure the messages are shaped the same way, but the signatures come from a more modern ECDSA signing key.
GnuPG upstream has explicitly rejected programmatic interpretation of --list-keys output as unstable unless using the machine-readable flag --with-colons. Make the test less brittle by including --with-colons. Some versions and implementations of gpg don't use the exact same escaping as g10code's current development branches. What really matters is whether the unescaped strings match, so test the matches in decoded form, not encoded form. Furthermore, it doesn't matter what order the strings show up in for either implementation: compare the user IDs as sets instead of lists.
Some versions of gpg default to uncompressed messages when producing an inline-signed message. Don't assume that there is a compressed packet.
gpg is expected to reject public key packets that are partial length. But at least some implementations skip over malformed pubkeys in the keyring without causing an error. Confirm that the output of `gpg --list-keys` on a pubring.gpg that contains only a partial-length public key is empty, rather than expecting it to return non-zero. See also: https://gitlab.com/sequoia-pgp/sequoia-chameleon-gnupg/-/issues/148
Some implementations of /usr/bin/gpg don't support El Gamal. If that's the case, run the El Gamal tests against rnp itself.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/0.x #2367 +/- ##
============================================
Coverage 85.46% 85.46%
============================================
Files 126 126
Lines 22710 22712 +2
============================================
+ Hits 19408 19410 +2
Misses 3302 3302 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This series cleans up a series of minor quibbles with the way that the RNP test suite interacts with GnuPG.
A future GnuPG may choose responsible cryptographic primitives as a baseline, and decline signatures from or encrypting to 1024-bit RSA, so this series also moves all the tests that expected 1024-bit RSA interoperability to testing the ECC sample key in keyring 5.
GnuPG can also be built without some weaker/deprecated algorithms (e.g., El Gamal and IDEA), so this cleanup skips over some tests for those algorithms if
gpg --version
doesn't indicate support for them.