Add gpg self-signature verification support #257

lukpueh · 2019-02-28T12:09:09Z

I apologize for the size of this PR, but separating the commits into more (inter-dependent) PRs would only increase the PR handling effort and still leave at least one PR with a large diff. Let me know if there is anything I can do to facilitate code review. A guide to this PR is provided below.

Fixes issue #:

Depends on PR Fix various minor gpg packet parsing issues #255, please review and merge there first (ed1ccd5 - 050b7cb)
Provides infrastructure required by Added checks for gpg key expiration in signature verification #245

Description of the changes being introduced by the pull request:
This PR adds signature verification on gpg public key export for two types of self-signatures:

Self-certificates related to the primary key, which can be used to acquire additional certified information about the the primary key, such as the key expiration date.
Subkey binding signatures, that bind each subkey to the primary key. So far we just assumed that a subkey attached to a primary key belongs to the primary key without verifying the binding signature. This PR discards all subkey that aren't verifiably bound to the primary key.

The lion share of this PR is provided by d3b6c44, which updates gpg public key parsing to verify the corresponding self-signatures.
Suggestions to re-organize the relevant functions, i.e. get_pubkey_bundle , parse_pubkey_bundle, _assign_certified_key_info and _get_verified_subkeys, are appreciated.

So far we only supported gpg binary type signatures (i.e. signatures over in-toto metadata) and restricted signature parsing accordingly. b7ce963 updates signature parsing to support the self-signature types described above.

For binary type signature creation and verification we only support SHA256. 713929b, 9388560 add changes to support SHA1, and SHA512 too (for self-signatures only!), which, based on real-word keyring inspections, seem to be widely-used.

4181469 updates signature parsing to return additional information, which is needed for further signature handling (as part of this PR), and also to get additional information about the primary key, such as the expiration date (required for #245). Note that this PR only provides certificate verification support, while the information extraction should be handled in #245 (see inline # TODO: comment).

Further tangential clean-up/refactor commits include:

26c1fbf to remove an obsolete else clause
ae4f5d0 to enhance some related error messages and fix comments and docstrings
4d825be to slightly modify signature keyid handling

Caveats/TODOs

Lacks proper testing (I'm inclined to do this in a separate PR out of sympathies for the reviewers)
Does not handle contradictory self-certificates (see inline # TODO: comment, should be addressed with Added checks for gpg key expiration in signature verification #245)
Does not handle key revocation certificates (see Handle GPG revocation signatures #263)

Please verify and check that the pull request fulfills the following
requirements:

The code follows the Code Style Guidelines
Tests have been added for the bug fix or new feature
Docs have been added for the bug fix or new feature

Support verification of gpg signatures that use SHA1 and SHA512. Together with SHA256, these seem to be the most widely used hashing algorithms for signatures over keys, such as self-signatures. Use the following command to see the distribution of hash algos on your local gpg keyring (2, 8 and 10 should stick out). gpg --export | gpg --list-packets | grep ":signature packet:" -a2 |\ grep -o "digest algo [0-9]*" | sort | uniq -c See RFC4880 9.4 for the list of all hash algorithm IDs. We continue to use SHA256 exclusively for metadata signing and verification. This addition is only needed to support self-signature verification.

Add optional supported_hash_algorithms argument to parse_signature_packet to support parsing signature packets with other hash algos. So far only SHA256 was supported, which remains default.

Add optional supported_signature_types argument to parse_signature_packet to support parsing signature packets of other types, e.g. different types of self-signatures. So far only SIGNATURE_TYPE_BINARY packets were supported, which remains the default.

Update parse_signature_packet to add an optional opaque info dictionary to the returned signature. Whether it is added or not may be configured via boolean "include_info" parameter. Per default it is not added. Currently the info dictionary contains the signature_type and hash_algorithm and an empty list that may be used to add subpackets such as a key expiration date. The corresponding subpacket parsing code is already there (commented out).

Update parse_signature_packet to only return the optional short_keyid if it was parsed from the signature packet to (better) match in_toto.gpg.formats.SIGNATURE_SCHEMA. Note that the returned signature still might fail the schema check, due to an empty keyid, but it is the responsibility of the caller to adjust accordingly. This commit also updates comments and calling functions.

This commit modifies the gpg public key parsing function to parse additional packets, including User ID and User Attribute packets, as well as signature packets corresponding to any of the other parsed packets. Additionally, the commit adds a helper function to verify self-certificates that may carry information about the primary key, such as key expiration date. And it adds a helper function to verify sub key binding signatures and updates the public key export function to only return subkeys that are verifiably bound to the primary key.

aaaaalbert · 2019-02-28T12:44:32Z

in_toto/gpg/util.py

+    raise ValueError("Hash algorithm '{}' not supported, must be one of '{}' "
+        "(see RFC4880 9.4. Hash Algorithms).".format(hash_algorithm_id,
+        {in_toto.gpg.constants.SHA1, in_toto.gpg.constants.SHA256,
+         in_toto.gpg.constants.SHA512}))


If this is never extended to support other hash types, my comment is moot, but there's a perhaps more concise way that avoids the if cascade:

supported_hashing_algorithms = [in_toto.gpg.constants.SHA1, in_toto.gpg.constants.SHA256, in_toto.gpg.constants.SHA512] corresponding_hashing_classes = [hashing.SHA1, hashing.SHA256, hashing.SHA512] hashing_class = dict(zip(supported_hashing_algorithms, corresponding_hashing_classes)) try: return hashing_class[hash_algorihm_id] except KeyError: raise ValueError("Hash algorithm '{}' not supported, must be one of '{}' " "(see RFC4880 9.4. Hash Algorithms).".format(hash_algorithm_id, supported_hashing_algorithms))

(And had 9388560 not dropped SUPPORTED_HASH_ALGORITHMS from gpg/constants.py, the proposed code could be even DRYer.)

I opted for the if/else variant because it seemed a tad easier to read, and I am not sure if we need to support other algos in the near future. But your approach is definitely more elegant.

I was going to suggest something similar. I'm fine either way though.

aaaaalbert · 2019-02-28T12:50:18Z

in_toto/gpg/common.py

+      elif packet_type == PACKET_TYPE_PRIMARY_KEY and \
+          key_bundle[PACKET_TYPE_PRIMARY_KEY]["key"]:
+        raise Exception("Unexpected primary key.")
+


I always find if cascades with complex causes difficult to read, as I'm never sure if I'm not missing a combination of conditions that's not handled. (I do think however that L193-L251 make sense.)

The elif in L198-199 could be rolled into L205+ as a sub-if clause.

Agreed, that's why I added comprehensive comments.

The elif in L198-199 could be rolled into L205+ as a sub-if clause.

That's how I wrote it initially, but then I thought it would be more readable the way it is now, as it is less arrow-head'ish.

arrow-head'ish

Hmm, like, two levels in doesn't feel that spiky to me :-)

It would be 4 levels already: while, try, elif, if ... 5 if you count the def... :P

...which is technically correct, but I think the def and while and try only count as overarching brackets. So: two ;-)

Agreed, it's only two (...technically). I'll still keep this one.

aaaaalbert · 2019-02-28T12:51:09Z

in_toto/gpg/common.py

+      # - disallow duplicate packets
+      if packet_type != PACKET_TYPE_PRIMARY_KEY and \
+          not key_bundle[PACKET_TYPE_PRIMARY_KEY]["key"]:
+        raise Exception("First packet must be a primary key ('{}'), got '{}'."


Please raise more specific error types than plain Exceptions. (See also L200)

This one (and L200) are caught and re-raised with more information as specific PacketParsingErrors in L254, adding additional information about the packet position.

OK, got it. I wonder if it makes sense to raise the PacketParsingError on that level already, just so as to not catch other potential exceptions and report them in a confusing way.

(Then L254 would only except PacketParsingError and act upon it, no other unrelated exceptions.)

Makes sense!

aaaaalbert · 2019-02-28T12:54:06Z

in_toto/gpg/common.py

+            PACKET_TYPE_USER_ATTR, PACKET_TYPE_SUB_KEY,
+            PACKET_TYPE_SIGNATURE]))
+
+    except Exception as e:


There's a bunch of except Exceptions across this PR. I wonder why the blanket except is desired (instead of a more specific exception type), and why it makes sense in a few places to continue after logging the exception.

Agreed! I do have this concern somewhere in my notes and I meant to add it to the PR description's "caveats/todos" section, as this issue applies to other places in the gpg module too, and I plan on addressing it in a separate PR. Thanks for pointing it out!

Re continue: I considered that question too and decided that in some cases it is okay to continue the overall key export but ignore a self-certificate, if it

belongs to a malformed User ID packet (L307),

cannot be parsed (L315),

is invalid (L337),

and, ignore a subkey, if it

cannot be parsed (L386, L389),

has a corresponding binding signature that cannot be parsed (L400),

has not exactly one binding signature (L420), or

the binding signature is invalid (L430).

But maybe it makes sense too do abort, since these errors are a sign that something is not fully supported or fishy.

Will be addressed in #126 (comment).

aaaaalbert · 2019-02-28T13:12:50Z

in_toto/gpg/util.py

+
+  # If Bit 6 of 1st octet is set we parse a New Format Packet Length, and
+  # an Old Format Packet Lengths otherwise
+  if data[0] & 0x40: # pragma: no cover


Consider using binary representations for this and L132, 0b0100_0000 and 0b0011_1111 and such. (I think this is what they're here for :-)

Great idea, will do.

lukpueh · 2019-02-28T14:43:35Z

Thanks for your comments @aaaaalbert, they are very much appreciated! ❤️❤️❤️

Do you have any advice regarding the organization of get_pubkey_bundle , parse_pubkey_bundle, _assign_certified_key_info and _get_verified_subkeys? I had this as one long spaghetti function initially, but that was too much to bear, so I split them into separate functions, but the boundaries feel a little arbitrary. Any thoughts?

aaaaalbert · 2019-02-28T14:58:01Z

My pleasure! 😄

Re organizing these functions, I think it's very sensible to have them separated even if they are called from a single place only. Each is moderately complicated, each has side- and sub-conditions in its checks, there must be tons of docs in each.... I'm totally fine with the way you did this!

The only thing that comes to mind is the choice of where exactly you collate the return values of the private / sub-functions into what the caller uses. For example, parse_... combines a few values; meanwhile get_... could do the same right here with similar effort, assuming that it got parse_...'s key_bundle instead of the tuple it actually returns.

This isn't super-important, it's just to consider the time spent for someone who reads the code and has to trace function calls.

lukpueh · 2019-02-28T15:43:29Z

@aaaaalbert, funny that you would mention this. Changing the code from the way you are proposing to what it is now was one of the last things I did on my WIP branch.

The (probably only semi-compelling) reason why I'm calling the helper functions from within parse_... is that I didn't want to expose the raw key bundle (i.e. primary key, user id, user attribute, and sub key packets plus their respective signature packets) in order to not have it confused it with the key bundle (i.e. parsed primary key with certified additional info and parsed and bound subkeys) that's returned from get_... eventually.

But parse_... could of course be seen as just another helper function that returns a private data structure to be used by other helper functions.

aaaaalbert · 2019-02-28T16:05:28Z

That's interesting! Any way is fine though, I suggest no change.

Fix unsubscriptable-object error on Python2.7 that occurs due to a dict operation on a variable that gets initialized as None first and then must get re-initialized as dict inside a loop.

SantiagoTorres · 2019-03-04T17:39:07Z

in_toto/gpg/functions.py

+            one of SHA1, SHA256, SHA512 (see in_toto.gpg.constants) used to
+            verify the signature. Default is SHA256.
+            NOTE: The hash algorithms specified in "pubkey_info"'s
+            "hashes" or "method" field are currently not considered.


I'm curious as to why we want to do this instead of updating our signature-type class?

What do you mean by "signature-type class"? The information that's on the pubkey, i.e. hashes and method? If yes, than the reason is that that's the hashes/method we support for binary signatures, i.e. signatures over documents, which is the main purpose of the keys, and which still is only sha256.

I added sha1 and sha512 for self-signatures only, which, IIRC, is something we agreed on in a different conversation. Does this make sense, or did I misunderstand your question?

SantiagoTorres · 2019-03-04T17:42:02Z

in_toto/gpg/util.py

@@ -187,12 +250,12 @@ def parse_subpackets(subpacket_octets):
    length = subpacket_octets[ptr]
    ptr += 1

-    if length > 192 and length < 255 : # pragma: no cover
+    if length >= 192 and length < 255 : # pragma: no cover


Nice catch, thanks for correcting this.

Minimal style issue: No leading space for the : here.

@SantiagoTorres, it only got to my attention because I have a key in my system keyring, where that byte is exactly 192. :)
@aaaaalbert, thanks!

SantiagoTorres · 2019-03-04T17:43:07Z

in_toto/gpg/util.py

+    raise ValueError("Hash algorithm '{}' not supported, must be one of '{}' "
+        "(see RFC4880 9.4. Hash Algorithms).".format(hash_algorithm_id,
+        {in_toto.gpg.constants.SHA1, in_toto.gpg.constants.SHA256,
+         in_toto.gpg.constants.SHA512}))


I was going to suggest something similar. I'm fine either way though.

SantiagoTorres · 2019-03-04T17:43:47Z

in_toto/gpg/constants.py

+PACKET_TYPE_SIGNATURE = 0x02
+PACKET_TYPE_PRIMARY_KEY = 0x06
+PACKET_TYPE_USER_ID = 0x0D
+PACKET_TYPE_USER_ATTR = 0x11


I'm assuming we need these two to concatenate them and verify the key-binding sig? If so, then ignore this. I just wanted to bring this up.

You mean User ID and User Attribute? Yes, well no, not key-binding sigs but rather self-certs (0x10 - 0x13). See _assign_certified_key_info() for more details.

lukpueh · 2019-03-05T11:03:14Z

Thanks for your feedback, @SantiagoTorres and @aaaaalbert! I'll address your comments and add tests soon.

@aaaaalbert

Replace if cascade with dict/zip niceness. Kudos to @aaaaalbert and @SantiagoTorres.

Replace except generic Exception in parsing loop with actually expected exceptions.

Use binary notation for bitmasks for better readability.

Revert adding optional hash algorithm argument in gpg.functions.gpg_verify_signature. This function is meant to be used for binary type signatures, for which we only accept sha265. For other signature types (self sigs), where we accept other hash algos too, we use a lower level gpg_verify_signature function.

Add test to trigger KeyNotFoundError

Also store the header length in raw gpg pubkey bundles for testing purposes.

Add test that asserts the presence of packets expected returned from `parse_pubkey_bundle` for specific test key). See gpg --homedir tests/gpg_keyrings/rsa/ --export 9EA70BD13D883381 | \ gpg --list-packets

Move signed content construction code out of try/except block. If that codes fails something must have gone wrong earlier and we should abort. This commit also adds TODO notes to revise exception taxonomy.

Use real gpg key data and modify it to trigger errors for testing.

Use manually crafted signature data to trigger errors for testing.

This differs across test environments.

lukpueh · 2019-03-21T16:24:18Z

36085ad to 9a6141f are related to testing, including some minor modifications in the actual implementation to make testing easier. Please review!

SantiagoTorres

Whoa, this took me hours to review.

On the surface I could only find very minor stylistic stuff, nice job!

I'm approving this, and probably doing the style changes myself. I want to test backwards-compatibilty with old metadata, as I'm sure our integrators would appreciate.

SantiagoTorres · 2019-03-21T16:50:33Z

in_toto/gpg/rsa.py

@@ -26,7 +25,7 @@
 import in_toto.gpg.util
 import in_toto.gpg.exceptions
 import in_toto.gpg.formats
-
+import in_toto.gpg.constants


I see that this removed one whitespace (we should have 2 here right?)

Are you talking about blank lines after imports?

See bac9726

SantiagoTorres · 2019-03-21T16:50:57Z

in_toto/gpg/util.py

@@ -26,6 +26,7 @@

 import in_toto.gpg.exceptions
 import in_toto.process
+import in_toto.gpg.constants



same whitespace issue.

See bac9726

SantiagoTorres · 2019-03-21T16:56:16Z

in_toto/gpg/functions.py

-  # If we have a full keyid it is safe to remove the short keyid to save space
-  del signature["short_keyid"]
+  # It is okay now to remove the optional short keyid to save space
+  signature.pop("short_keyid", None)


Curious: what's the rationale for this change?

del signature["short_keyid"] might raise a KeyError. Since short_keyid is optional, I'm using pop to be safe.

Ah right. Thanks for clarifying!

SantiagoTorres · 2019-03-21T17:00:12Z

in_toto/gpg/common.py

+      # and it must be ordered because each signature packet belongs to the
+      # most recently parsed packet of a type.
+      elif packet_type in [PACKET_TYPE_USER_ID, PACKET_TYPE_USER_ATTR,
+          PACKET_TYPE_SUB_KEY]:


nit, can we use a set instead?

sure will change

Done a4a7e58

SantiagoTorres · 2019-03-21T17:02:03Z

in_toto/gpg/common.py

+      # The remaining relevant packets are signatures, required to bind subkeys
+      # to the primary key, or to gather additional information about the
+      # primary key, e.g. expiration date.
+      # A signatures corresponds to the most recently parsed packet of a type,


s/signatures/signature/g

Done 77d7fb1

SantiagoTorres · 2019-03-21T17:07:28Z

in_toto/gpg/common.py

+
+    except Exception as e:
+      log.info(e)
+      continue


I wonder if we want to remove this except block and just let it blow up instead.

I am doing this in a more recent commit.

SantiagoTorres · 2019-03-21T17:07:42Z

in_toto/gpg/common.py

+
+      except Exception as e:
+        log.info(e)
+        continue


Here we shouldn't because parse_signature_packet might fail legitimately, because we want to continue if parse_signature_packet fails because we're giving it e.g. a revocation signature, which might happen. In a more recent commit I added a TODO: Revise exception taxonomy

SantiagoTorres · 2019-03-21T17:12:35Z

in_toto/gpg/dsa.py

@@ -183,6 +183,8 @@ def gpg_verify_signature(signature_object, pubkey_info, content,
    hash_algorithm_id:
            one of SHA1, SHA256, SHA512 (see in_toto.gpg.constants) used to
            verify the signature
+            NOTE: Overrides any hash algorithm specification in "pubkey_info"'s
+            "hashes" or "method" fields.


I wonder if this note sounds more dramatic than it really is (as far as I understand hash_algorithm_id is populated with pubkey_info["hashes"] or "method" fields when not used directly?

No. Currently SHA256 is passed explicitly in the interface function gpg.functions.gpg_verify_signature to the corresponding dsa or rsa verification function. While it aligns with the possible values for "hashes" or "method", assigned in gpg.common.parse_pubkey_payload, it is not populated from those fields.

But even if it were populated from those fields (IMO a good idea), we still want to be able to use a public key that we restrict to sha256 for binary signatures (i.e. key_info["hashes"] = "pgp+SHA2"), to verify self-signatures with other hash algos ... hence the support for an override and hence the comment. :)

We can of course re-think the fields in the public key dict and how we assign them. But I wouldn't do it as part of this PR.

SantiagoTorres · 2019-03-21T17:30:52Z

in_toto/gpg/common.py

@@ -329,14 +672,18 @@ def parse_signature_packet(data):
    if subpacket_type == PARTIAL_KEYID_SUBPACKET:
      short_keyid = binascii.hexlify(subpacket_data).decode("ascii")

+    # info["subpackets"].append((
+    #   subpacket_type,
+    #   binascii.hexlify(subpacket_data).decode("ascii")))


This somehow sneaked through. Could you please remove it?

I left it for michael, but I can remove it.

I added a TODO comment 60555c7 and will remove or un-comment it with #245

@SantiagoTorres

Addressing @SantiagoTorres' request: in-toto#257 (comment)

SantiagoTorres · 2019-03-22T15:42:00Z

LGTM.

Thanks @lukpueh !

@aaaaalbert

Change parse_pubkey_bundle to return the parsed raw pubkey bundle and let the caller, i.e. get_pubkey_bundle, call functions for further processing, i.e. _assign_certified_key_info and _get_verified_subkeys, to eventually return the processed pubkey bundle, i.e. master_public_key (enriched with certified key info and optional verified subkeys). This follows suggestions made by @aaaaalbert in in-toto/in-toto#257 (comment) and also makes testing easier.

@SantiagoTorres

Addressing @SantiagoTorres' request: in-toto/in-toto#257 (comment)

@aaaaalbert

Change parse_pubkey_bundle to return the parsed raw pubkey bundle and let the caller, i.e. get_pubkey_bundle, call functions for further processing, i.e. _assign_certified_key_info and _get_verified_subkeys, to eventually return the processed pubkey bundle, i.e. master_public_key (enriched with certified key info and optional verified subkeys). This follows suggestions made by @aaaaalbert in in-toto/in-toto#257 (comment) and also makes testing easier.

@SantiagoTorres

Addressing @SantiagoTorres' request: in-toto/in-toto#257 (comment)

@aaaaalbert

Change parse_pubkey_bundle to return the parsed raw pubkey bundle and let the caller, i.e. get_pubkey_bundle, call functions for further processing, i.e. _assign_certified_key_info and _get_verified_subkeys, to eventually return the processed pubkey bundle, i.e. master_public_key (enriched with certified key info and optional verified subkeys). This follows suggestions made by @aaaaalbert in in-toto/in-toto#257 (comment) and also makes testing easier.

@SantiagoTorres

Addressing @SantiagoTorres' request: in-toto/in-toto#257 (comment)

@aaaaalbert

Change parse_pubkey_bundle to return the parsed raw pubkey bundle and let the caller, i.e. get_pubkey_bundle, call functions for further processing, i.e. _assign_certified_key_info and _get_verified_subkeys, to eventually return the processed pubkey bundle, i.e. master_public_key (enriched with certified key info and optional verified subkeys). This follows suggestions made by @aaaaalbert in in-toto/in-toto#257 (comment) and also makes testing easier.

@SantiagoTorres

Addressing @SantiagoTorres' request: in-toto/in-toto#257 (comment)

@aaaaalbert

Change parse_pubkey_bundle to return the parsed raw pubkey bundle and let the caller, i.e. get_pubkey_bundle, call functions for further processing, i.e. _assign_certified_key_info and _get_verified_subkeys, to eventually return the processed pubkey bundle, i.e. master_public_key (enriched with certified key info and optional verified subkeys). This follows suggestions made by @aaaaalbert in in-toto/in-toto#257 (comment) and also makes testing easier.

@SantiagoTorres

Addressing @SantiagoTorres' request: in-toto/in-toto#257 (comment)

lukpueh added 8 commits February 20, 2019 16:59

Update supported hash algos in signature parsing

9388560

Add optional supported_hash_algorithms argument to parse_signature_packet to support parsing signature packets with other hash algos. So far only SHA256 was supported, which remains default.

Enhance gpg error messages and fix comments/docs

ae4f5d0

Remove obsolete else clause in parse_pubkey_payload

26c1fbf

aaaaalbert reviewed Feb 28, 2019

View reviewed changes

Init primary key as empty dict to fix pylint error

5f18f39

Fix unsubscriptable-object error on Python2.7 that occurs due to a dict operation on a variable that gets initialized as None first and then must get re-initialized as dict inside a loop.

lukpueh changed the title ~~Add gpg self sigs~~ Add gpg self-signature verification support Feb 28, 2019

SantiagoTorres reviewed Mar 4, 2019

View reviewed changes

lukpueh mentioned this pull request Mar 7, 2019

Support gpg signatures (copied from in-toto project) secure-systems-lab/securesystemslib#163

Closed

3 tasks

lukpueh added 4 commits March 15, 2019 14:33

Clean up gpg.util.get_hashing_class

06346dc

Replace if cascade with dict/zip niceness. Kudos to @aaaaalbert and @SantiagoTorres.

Raise/catch specific error in parse_pubkey_bundle

e2142fe

Replace except generic Exception in parsing loop with actually expected exceptions.

Use binary notation for bitmasks in gpg.util

eb7c90f

Use binary notation for bitmasks for better readability.

Remove stray whitespace before colon

8c5794e

This was referenced Mar 15, 2019

Revisit exception taxonomy #126

Open

Handle GPG revocation signatures #263

Closed

lukpueh added 2 commits March 18, 2019 14:06

Remove unused import in gpg.rsa

2513795

lukpueh added 6 commits March 21, 2019 12:24

Add gpg.common.get_pubkey_bundle test

9917e9d

Add test to trigger KeyNotFoundError

Update gpg raw pubkey bundle data format

d41a4a3

Also store the header length in raw gpg pubkey bundles for testing purposes.

Add gpg.common._parse_pubkey_bundle test

a2d6496

Add test that asserts the presence of packets expected returned from `parse_pubkey_bundle` for specific test key). See gpg --homedir tests/gpg_keyrings/rsa/ --export 9EA70BD13D883381 | \ gpg --list-packets

Update error handling in gpg pubkey parse helpers

18ea3a3

Move signed content construction code out of try/except block. If that codes fails something must have gone wrong earlier and we should abort. This commit also adds TODO notes to revise exception taxonomy.

Add gpg.common._assign_certified_key_info tests

046262f

Use real gpg key data and modify it to trigger errors for testing.

Add gpg.common._get_verified_subkeys tests

91ffe04

Use real gpg key data and modify it to trigger errors for testing.

lukpueh force-pushed the add-gpg-self-sigs branch from 8347964 to 91ffe04 Compare March 21, 2019 15:22

lukpueh added 3 commits March 21, 2019 16:53

Add gpg.common.parse_signature_packet tests

d20841a

Use manually crafted signature data to trigger errors for testing.

Fix gpg.common tests for windows

21f22f1

Exclude full gpg keyid related code from coverage

9a6141f

This differs across test environments.

SantiagoTorres approved these changes Mar 21, 2019

View reviewed changes

lukpueh added 4 commits March 22, 2019 09:33

Fix blank lines at module head in gpg subpackage

bac9726

Use set instead of list in in condition

a4a7e58

Addressing @SantiagoTorres' request: in-toto#257 (comment)

Fix typos in gpg subpackage comments

77d7fb1

Add TODO comment to commented out gpg block

60555c7

SantiagoTorres merged commit eadfce5 into in-toto:develop Mar 22, 2019

lukpueh added a commit to lukpueh/securesystemslib that referenced this pull request Aug 8, 2019

Use set instead of list in in condition

17a58e3

Addressing @SantiagoTorres' request: in-toto/in-toto#257 (comment)

lukpueh added a commit to lukpueh/securesystemslib that referenced this pull request Aug 14, 2019

Use set instead of list in in condition

d199e4a

Addressing @SantiagoTorres' request: in-toto/in-toto#257 (comment)

lukpueh added a commit to lukpueh/securesystemslib that referenced this pull request Sep 4, 2019

Use set instead of list in in condition

7d200e2

Addressing @SantiagoTorres' request: in-toto/in-toto#257 (comment)

lukpueh added a commit to lukpueh/securesystemslib that referenced this pull request Sep 4, 2019

Use set instead of list in in condition

cd9396c

Addressing @SantiagoTorres' request: in-toto/in-toto#257 (comment)

This was referenced Oct 29, 2019

Handle GPG revocation signatures secure-systems-lab/securesystemslib#190

Open

Revise exception taxonomy secure-systems-lab/securesystemslib#191

Closed

tanishqjasoria pushed a commit to tanishqjasoria/securesystemslib that referenced this pull request Jan 30, 2020

Use set instead of list in in condition

cced657

Addressing @SantiagoTorres' request: in-toto/in-toto#257 (comment)

Add gpg self-signature verification support #257

Add gpg self-signature verification support #257

Conversation

lukpueh commented Feb 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaaaalbert Feb 28, 2019 • edited Loading

Choose a reason for hiding this comment

lukpueh Feb 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaaaalbert Feb 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukpueh commented Feb 28, 2019

aaaaalbert commented Feb 28, 2019

lukpueh commented Feb 28, 2019

aaaaalbert commented Feb 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukpueh commented Mar 5, 2019

lukpueh commented Mar 21, 2019

SantiagoTorres left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SantiagoTorres commented Mar 22, 2019

lukpueh commented Feb 28, 2019 •

edited

Loading

aaaaalbert Feb 28, 2019 •

edited

Loading

lukpueh Feb 28, 2019 •

edited

Loading

aaaaalbert Feb 28, 2019 •

edited

Loading