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

Schema revision #176

Merged

Conversation

danixeee
Copy link
Contributor

@danixeee danixeee commented Sep 3, 2019

Description of the changes being introduced by the pull request:

This PR modifies formats in order to support GPG keys.
Signature verification function is also modified to allow passing GPG public key format.

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

@coveralls
Copy link

coveralls commented Sep 3, 2019

Coverage Status

Coverage decreased (-0.05%) to 99.9% when pulling 358ca33 on openlawlibrary:danixeee/schema-revision into da1af79 on secure-systems-lab:master.

@danixeee danixeee force-pushed the danixeee/schema-revision branch 4 times, most recently from de9a750 to 001ed1d Compare September 9, 2019 11:13
@danixeee
Copy link
Contributor Author

danixeee commented Sep 9, 2019

@lukpueh I saw you merged gpg work to master branch, so I have rebased this branch.

@lukpueh
Copy link
Member

lukpueh commented Sep 9, 2019

@danixeee, I fear your rebase resurrected some TUF-specific schemas that we removed purposefully with #165.

@danixeee
Copy link
Contributor Author

danixeee commented Sep 9, 2019

@lukpueh, sorry for that, I think it should be fine now.

Copy link
Member

@lukpueh lukpueh left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Please consider addressing a few minor inline comments. Otherwise it looks mostly unproblematic, i.e. he existing SIGNATURES_SCHEMA and SIGNABLE_SCHEMA have become broader, to also allow gpg signatures. And there are a couple of new ANY_* schemas.


# Does 'signature' have the correct format?
securesystemslib.formats.SIGNATURE_SCHEMA.check_match(signature)
securesystemslib.formats.ANY_SIGNATURE_SCHEMA.check_match(signature)
Copy link
Member

Choose a reason for hiding this comment

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

keys.verify_signature does not support gpg keys/signatures (yet). Why allow passing them as arguments? IIUC this should raise a TypeError('Unsupported key type.').

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lukpueh I thought it's fine to get TypeError until it is implemented. I have reverted those changes.

good_sigs = KEYIDS_SCHEMA,
bad_sigs = KEYIDS_SCHEMA,
unknown_sigs = KEYIDS_SCHEMA,
untrusted_sigs = KEYIDS_SCHEMA)
Copy link
Member

Choose a reason for hiding this comment

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

SIGNATURESTATUS_SCHEMA was dropped with #165, please don't add back.

value_schema = ANY_PUBKEY_SCHEMA)

ANY_STRING_SCHEMA = SCHEMA.AnyString()
LIST_OF_ANY_STRING_SCHEMA = SCHEMA.ListOf(ANY_STRING_SCHEMA)
Copy link
Member

Choose a reason for hiding this comment

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

Any reason why ANY_STRING_SCHEMA and LIST_OF_ANY_STRING_SCHEMA were moved down here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was the latest defined schema in the file, I moved it to the top of the file now.

Copy link
Member

@lukpueh lukpueh left a comment

Choose a reason for hiding this comment

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

Thanks for the updates!

@lukpueh lukpueh merged commit 5696ec4 into secure-systems-lab:master Sep 20, 2019
@danixeee danixeee deleted the danixeee/schema-revision branch October 1, 2019 11:04
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.

3 participants