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

FIX: verifylib: fix subkey keyid resolution #201

Merged

Conversation

SantiagoTorres
Copy link
Member

The load_links_for_layout method in verifylib didn't account for subkey_keyid's
when resolving signatures. Add code to iterate over the subkeys when verifying
signatures made with gpg keys.

The load_links_for_layout method in verifylib didn't account for
subkey_keyid's when resolving signatures. Add code to iterate over the
subkeys when verifying signatures made with gpg keys.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 1a1a1d8 on SantiagoTorres:fix_gpg_keyid_subkeys into b8b61ac on in-toto:develop.

1 similar comment
@coveralls
Copy link

coveralls commented Mar 13, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 1a1a1d8 on SantiagoTorres:fix_gpg_keyid_subkeys into b8b61ac on in-toto:develop.

except IOError:
pass
except IOError:
pass

# Check if the step has been performed by enough number of functionaries
if len(links_per_step) < step.threshold:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the master key, along with its subkeys, considered one unique functionary? Correct me if I'm wrong, but it seems like the code is counting each subkey towards the threshold requirement.

I was expecting (MasterKey, SubKey1, SubKey2) to be treated as one trusted key. If you want three different people to sign for a step, isn't it wrong to accept three different subkeys from just one person?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thinking, @vladimir-v-diaz! This is something that we have to consider, but not in above function, which only performs a preliminary threshold verification, i.e. do we even have enough link files, but rather in the actual signature threshold verification function veriflylib. verify_link_signature_thresholds.

Currently, a functionary could indeed bypass the required threshold by providing threshold link files signed with different subkeys of an authorized master key.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I guess a check is in order in the right place. I'll give this a go this week...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved this discussion to ticket #202. I'll be merging this as-is in order to prepare a new minor release.

@SantiagoTorres SantiagoTorres merged commit 26394c7 into in-toto:develop Mar 30, 2018
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

Successfully merging this pull request may close these issues.

4 participants