improve API for signatures and subkeys #451

dkg · 2023-06-15T21:38:35Z

On top of #450, this change makes the following API adjustments (from the diff in the changelog):

The following properties of PGPSignature now return None if the
corresponding subpacket is not present (they used to return an empty
string in that case):

* keyserver
* policy_uri
* signer
* signer_fingerprint

And the following properties of PGPSignature now return an
enum.IntFlag object instead of a set of custom FlagEnum objects.  When
the corresponding subpacket is not present at all, they return None:

* key_flags
* keyserverprefs
* features

PGPKey.subkeys now returns an OrderedDict indexed by Fingerprint
instead of KeyID.  When accessing this property via subscript (i.e.,
key.subkeys[x]), you can *also* index it by KeyID, but using a full
Fingerprint is recommended.

These underlying changes have a few advantages:

it is easier to reason about types,
it's possible to tell the difference between "no preference" and "an explicitly stated preference for nothing"
it is more likely that keys will be indexed and discovered without worrying about collisions on the (shorter) key ID.

These tests are pulled from the OpenPGP Interoperability Test Suite, in particular from those tests tagged "forward-compat": https://tests.sequoia-pgp.org/?q=forward-compat These failures make it difficult to use PGPy in an OpenPGP ecosystem that can evolve, because PGPy will complain about OpenPGP objects that contain something it doesn't understand, even though the rest of the message is otherwise comprehensible and usable. See Justus Winter's message to openpgp@ietf.org describing his concerns: https://mailarchive.ietf.org/arch/msg/openpgp/QUiEKx3PQeJOXnkcvvnuHpv739M I've selected specific examples that PGPy is known to currently fail with.

pgpy.types.Exportable was renamed to Armorable before version 0.3.0 was released (in d00010e, back in 2014), which indicates that this test program has not been run in over 8 years. Given that the asyncio stuff has also changed in python since then, it is simpler to just drop this file than try to fix it.

This reduces the number of SKIPPED and XFAIL messages from the test suite. Given where the cleanup is happening, also make the remaining XFAIL messages a little clearer.

…iased)

This is not a signature subpacket. Rather, it is a signature type.

dkg · 2023-06-15T23:21:19Z

In addition to the API tune-up, with this PR applied, mypy pgpy returns no errors.

(mypy --strict pgpy still returns 1575 errors indicating that we don't have complete type coverage, but i'll work on fixing that in a subsequent PR)

dkg · 2023-06-16T19:10:47Z

Given that #450 is still a WIP, this is also a WIP. I'll remove "WIP" when it is a bit more stable, but i wanted to make clear the specific API changes i'm considering making.

It works in the basic mode, but we still need to handle the args for encrypt/decrypt.

- add enums for the flags passed into the sop interface - make member functions of StatelessOpenPGP well-typed - adjust docstrings so that help(sop) provides useful guidance - handle sessionkey and timestamp parsing in sop.py - handle all indirect access directly in sop.py - complete strict typing ("mypy --strict sop.py" passes)

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>

By making all arguments to the functions keyword arguments, we can use **kwargs to receive any extended options. Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>

This reflects the changes to the subcommand names and additional arguments from draft-dkg-openpgp-stateless-cli-01

…ithm I don't expect new image encoding types to be defined. Indeed, the version itself could just go away and we could probably retcon UserAttribute 1 as JPEG (see https://gitlab.com/openpgp-wg/rfc4880bis/-/issues/162) with a weird 16-octet header. But it's better to have the code normalized here.

…tion, etc

See the rationale from the upcoming standard: https://www.ietf.org/archive/id/draft-ietf-openpgp-crypto-refresh-07.html#name-optional-checksum

The implementation will select a reasonable option for the indicated pubkey algorithm if you don't supply it.

This could be done only during the --profile draft-ietf-openpgp-crypto-refresh-10, but there's nothing in the earlier specifications that would indicate a problem or incompatibility to include such a signature.

…o base class

Move encrypt into fields.PubKey, and decrypt into fields.PrivKey. This is a simplifying change, and also prepares the codebase for the crypto-refresh, which has different subtle changes in the encoded pkesk, for different versions and different algorithms. The underlying cryptographic code gets pushed into fields.py (out of sight from higher-level functions) and lets us make more straightforward and explicit type definitions.

- if something is not implemented, just raise NotImplementedError directly, rather than trying to sometimes return a NotImplemented object. Returning a NotImplemented object complicates the type signature without providing much of an advantage in terms of workflow. - PGPSignature.target_signature is a property that was never implemented or used. We can just drop this property entirely, implementing it only when it can actually be implemented.

use search_pref_sigs() instead of walking each user ID

This leaves only one place to list the requirements

This will be important if a non-v4 signature gets embedded. It might end up being an opaque signature, if PGPy can't read the version, but it shouldn't try to force a SignatureV4 if it is not actually version 4. This change also isolates the parsing of the embedded signature so that it can only consume bytes within the enclosing subpacket.

This hasn't been necessary since fingerprints knew their own version, it was just a no-op.

This is what caught the problem of passing _version to addnew

dkg added 6 commits June 13, 2023 13:30

tests: Avoid passing naive datetime objects

d5155d2

Avoid looking for encrypted messages with "Plaintext" encoding

17cb140

This reduces the number of SKIPPED and XFAIL messages from the test suite. Given where the cleanup is happening, also make the remaining XFAIL messages a little clearer.

rename setter for EmbeddedSignature._sig (otherwise definition was al…

8e4f7ca

…iased)

Drop erroneous SubkeyBindingSignature

0f8c67d

This is not a signature subpacket. Rather, it is a signature type.

dkg force-pushed the dkg/api-tuneup branch 4 times, most recently from 2dd79a7 to e2b7b8c Compare June 15, 2023 23:19

dkg force-pushed the dkg/api-tuneup branch 2 times, most recently from b1f6827 to e028c80 Compare June 15, 2023 23:48

dkg and others added 16 commits June 16, 2023 17:33

handle Armored text with UTF-8 comments

edd7e4f

initial PGPy-backed implementation of sop

26e6c37

It works in the basic mode, but we still need to handle the args for encrypt/decrypt.

sop.py: allow instances to augment the interface; provide autocomplete

8dabb3a

treat keyflags as a set, as expected

1d5e06e

use specialized exception for missing options

814a65e

sopgpy: add armor and dearmor subcommands

adbd3bd

indicate each unsupported feature separately for cleaner diffs

820a9fc

sopgpy: added --sessionkey to encrypt subcommand

19c7f7e

clean up signature result printing

dad83bc

sopgpy: do not try to encrypt with IDEA

48f2cc1

sop decrypt: handle session key and verification output cleanly

0a64e50

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>

Enable easier extension to the sop interface

95b635a

By making all arguments to the functions keyword arguments, we can use **kwargs to receive any extended options. Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>

sopgpy: Improve shebang so that we can use virtualenv.

d5aaa90

sopgpy: move to 0.2.0 of python-sop

6a5f4cd

This reflects the changes to the subcommand names and additional arguments from draft-dkg-openpgp-stateless-cli-01

PGPy removed __version__ in 0.5.4. This fix relies on python ≥ 3.8

b2b861c

dkg added 21 commits August 11, 2023 14:26

More type annotations

dcfe210

PGPKey: Improve type signatures for unprotect, related keyblob decryp…

98b62a7

…tion, etc

ASCII armoring should not require the CRC line

e467d8e

See the rationale from the upcoming standard: https://www.ietf.org/archive/id/draft-ietf-openpgp-crypto-refresh-07.html#name-optional-checksum

add test vectors from draft-ietf-openpgp-crypto-refresh-10

cc34a73

sopgpy: use default algorithms when protecting secret key material

2d53b9a

PGPKey.new() no longer requires a key_size option.

ee92d8d

The implementation will select a reasonable option for the indicated pubkey algorithm if you don't supply it.

sopgpy: add a Direct Key Signature to the default key creation

f799d4b

This could be done only during the --profile draft-ietf-openpgp-crypto-refresh-10, but there's nothing in the earlier specifications that would indicate a problem or incompatibility to include such a signature.

sopgpy: generate-key: improve type signatures

6fb08f2

Armoring: prepare to not emit CRC in some cases.

11d641c

Look for primary key usage flags with same search path as preferences

08dd35c

SKESK: put common functionality (symmetric cipher, S2K specifier) int…

35b00e8

…o base class

PGPKey.new: add version parameter (only supports v4 at the moment)

18a557c

compare EllipticCurveOID values with "is" -- they are enums

4ab37e4

PubKey.verify should raise an exception, not return one.

49c6083

correct comment about MTI symmetric key algorithm

7dee6b0

PGPKey: add features property

2e883d7

sopgpy: search for cipher preferences more cleanly

a57714c

use search_pref_sigs() instead of walking each user ID

PGPKey.encrypt: add max_featureset option to limit version of SEIPD

51cefc7

dkg force-pushed the dkg/api-tuneup branch from 4ae48f4 to 51cefc7 Compare August 11, 2023 18:35

Have tox take the baseline requirements from requirements.txt

d713701

This leaves only one place to list the requirements

dkg force-pushed the dkg/api-tuneup branch 2 times, most recently from 8558542 to ce54273 Compare August 18, 2023 18:43

note dependency on sop as optional

f6cf609

dkg force-pushed the dkg/api-tuneup branch from ce54273 to f6cf609 Compare August 19, 2023 05:10

dkg added 3 commits August 23, 2023 18:17

Don't pass fingerprint version to FingerprintSubpacket

e810446

This hasn't been necessary since fingerprints knew their own version, it was just a no-op.

SubPackets.addnew: warn when trying to set a non-existent attribute

46d9995

This is what caught the problem of passing _version to addnew

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve API for signatures and subkeys #451

improve API for signatures and subkeys #451

dkg commented Jun 15, 2023

dkg commented Jun 15, 2023

dkg commented Jun 16, 2023

improve API for signatures and subkeys #451

Are you sure you want to change the base?

improve API for signatures and subkeys #451

Conversation

dkg commented Jun 15, 2023

dkg commented Jun 15, 2023

dkg commented Jun 16, 2023