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

Add gpg support and custom (sub)process module #174

Merged
merged 160 commits into from
Sep 6, 2019

Conversation

lukpueh
Copy link
Member

@lukpueh lukpueh commented Aug 13, 2019

Fixes issue #:

Supersedes #163
Closes #55 and in-toto/in-toto#275

Description of the changes being introduced by the pull request:

This PR adds a Python interface for GnuPG providing functions for:

  • signature creation (using private keys in a local gpg keyring)
  • public key export (from a local gpg keyring into a pythonic format),
  • gpg signature verification function that uses the cryptography library and does not require the gpg command line tool

Additionally, this PR adds a:

  • custom (sub)process convenience module, to i.a. hide Python version compatibility details

Both, the gpg sub-package and the process module, were originally developed as part of the in-toto project, and were transferred at in-toto@9c374a9c970f0e8d985cbf06a7e6c7a7df3017c3, preserving parts of the original git history. See commit messages of 62ba239 and de817db for git-technical details about `.

For review only consider commits in the range of lukpueh/securesystemslib@47a9d9a...lukpueh:add-gpg, earlier commits have already been reviewed as part of the in-toto project.

This PR further:

DISCLAIMER: This PR does not integrate the public gpg functions with securesystemslib's public function modules, i.e. keys or interface.
This also means, that the generic keys.verify_signature function can not be used to verify gpg signatures (use gpg.functions.verify_signature instead).

While such an integration seems desirable, I suggest to do this in a separate PR, e.g. in the course of a general API revision (see #35).

  • 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

Coverage Status

Coverage decreased (-0.09%) to 99.908% when pulling 5db2617 on lukpueh:add-gpg into d6227b4 on secure-systems-lab:master.

@coveralls
Copy link

coveralls commented Aug 14, 2019

Coverage Status

Coverage decreased (-0.05%) to 99.954% when pulling 9906acd on lukpueh:add-gpg into ae8517d on secure-systems-lab:master.

@lukpueh
Copy link
Member Author

lukpueh commented Aug 14, 2019

I just rebased onto master, force pushed and lost the merge commits that contained messages about how I used git filter-branch with --subdirectory-filter and --index-filter to preserve the git history of the gpg subpackage (and the process module).

For documentation purposes here is what I did to pull in the gpg subpackage together with its history:

# Grab in-toto repo and create a new branch at tip of development
  git clone git@github.com:in-toto/in-toto.git
  cd in-toto
  git checkout -b in-toto-gpg

  # Make "in_toto/gpg" subdir the root of the repo
  git filter-branch --prune-empty --subdirectory-filter in_toto/gpg HEAD

  # Grab securesystemslib repo and fetch the unrelated in-toto branch that
  # only has the contents of in-toto's gpg subdirectory
  cd ..
  git clone git@github.com:secure-systems-lab/securesystemslib.git
  cd securesystemslib
  git remote add in-toto ../in-toto
  git fetch in-toto
  git checkout in-toto-gpg

  # Move all gpg files back into a corresponding securesystemslib/gpg subdir
  # NOTE: sed does not understand \t so we use a literal tab (Ctrl-v + TAB)
  git filter-branch --index-filter \
    'git ls-files -s | sed "s/$(printf \\t)/&\securesystemslib\/gpg\//" |
      GIT_INDEX_FILE=$GIT_INDEX_FILE.new \
        git update-index --index-info &&
     mv "$GIT_INDEX_FILE.new" "$GIT_INDEX_FILE"' HEAD

  # Merge the unrealted gpg branch into a new working branch based off of
  # securesystemslib's master
  git checkout master
  git checkout -b add-gpg
  git merge in-toto-gpg --allow-unrelated-histories

SantiagoTorres and others added 22 commits September 4, 2019 15:56
Travis is running on ubuntu 14 and using gpg1 by default
The module defines formats for RSA and DSA public key dicts
used for gpg signature verification.
Securesystemslib schema formats don't support dashes in dictionary
keys. To be able to use the existing infrastructure we change the
key's name from "other-headers" to "other_headers"
336b34cf851cd85c7a049d6155c0fdba2ad20698 changed the way
the keyid is parsed from gpg signatures and accidentally removed
the keyid from the signature dict. This commit re-adds it.
The added function uses the gpg2 command to get the version info
and parses the version string using regex.
The function uses get_version to get the version of installed
gpg2 and compares it against a constant specifying the
minimal fully supported version number, needed to parse keyids
from signature packets.
This commit changes parse_signature_packet to warn (instead of fail)
if no keyid can be derived from the signature data.

The signature dict is returned with an empty string as keyid and
the caller must deal with it appropriately.
This commit adds a workaround to gpg_sign_object if the created
signature is returned with an empty keyid.
The workaround consists of exporting the public part of the signing
key to compute the keyid.
Note: this will fail if gpg_sign_object is called without a keyid
(i.e. using the default key), because exporting the public key
requires a keyid.
This commit removes case handling for different versions of gpg
from code coverage (pragma: no cover) to not vary coverage in
different testing environments.
Once travis runs a newer system with an up-to-date version of gpg
when can remove this case handling altogether.
Crafting gpg signature and key data that triggers the excluded
exceptions doesn't seem feasible at the moment.
The gpg interface module `functions` seems better suited for
the publicly available verification function.
Adds usage description and fixes a doc bug in gpg.formats.
The added tests are a verbatim copy of the corresponding test
module in in-toto at 9c374a9c970f0e8d985cbf06a7e6c7a7df3017c3,
with fixed import paths.

The history of the original test module in in-toto is not very
interesting and hence discarded, as opposed to the history of the
process module itself where we made an effort (git filter-branch
...) to preserve it.
The added tests are a verbatim copy of the corresponding test
module in in-toto at 9c374a9c970f0e8d985cbf06a7e6c7a7df3017c3,
with fixed import paths.

The history of the original test module in in-toto is not very
interesting and hence discarded, as opposed to the history of the
gpg sub-package itself where we made an effort (git filter-branch
...) to preserve it.
Set environment variable in test aggregate script that may be
used to skip tests if gpg is not available on the test system.
Copied from in-toto at 9c374a9c970f0e8d985cbf06a7e6c7a7df3017c3.
See in-toto commit history for details.

Use gpg with the homedir option for details about the available
keys, e.g.:

~/securesystemslib $ gpg --homedir tests/gpg_keyrings/rsa --list-keys
[...]
-----------------------------------------------------------------------
pub   rsa2048 2017-11-16 [SC]
      8465A1E2E0FB2B40ADB2478E18FB3F537E0C8A17
uid           [ultimate] Joan Doe <test@donotuseth.is>
sub   rsa2048 2017-11-16 [E]
sub   rsa2048 2018-02-26 [S]
sub   elg2048 2018-02-28 [E]

pub   rsa2048 2017-12-01 [SC]
      7B3ABB26B97B655AB9296BD15B0BD02E1C768C43
uid           [ultimate] Don Joan <test2@donotuseth.is>
sub   rsa2048 2017-12-01 [E]

pub   rsa2048 2017-12-01 [SC]
      8288EF560ED3795F9DF2C0DB56193089B285DA58
uid           [ultimate] Dodo Jojo <test3@donotuseth.is>
sub   rsa2048 2017-12-01 [E]

pub   rsa2048 2019-01-21 [SC]
      40E692C3AE03F6B88DFF95D0D2C9FE930766998D
uid           [ultimate] Arthur Two Keys <test4@donotuseth.is>
sub   rsa2048 2019-01-21 [E]
sub   rsa2048 2019-01-21 [S]
sub   rsa2048 2019-01-21 [S]

pub   rsa2048 2019-03-20 [SC] [revoked: 2019-03-20]
      F557D0FF451DEF45372591429EA70BD13D883381
uid           [ revoked] Test Self-sig <test@self.sig>
uid           [ revoked] [jpeg image of size 4971]

pub   rsa2048 2019-03-25 [SC] [expired: 2019-03-26]
      E8AC80C924116DABB51D4B987CB07D6D2C199C7C
uid           [ expired] Test Expiration II <test@expir.two>
uid           [ expired] Test Expiration I <test@expir.one>
uid           [ expired] Test Expiration III <test@expir.three>

~/securesystemslib $ gpg --homedir tests/gpg_keyrings/dsa --list-keys
[...]
-----------------------------------------------------------------------
pub   dsa2048 2017-11-16 [SC]
      C242A830DAAF1C2BEF604A9EF033A3A3E267B3B1
uid
- Updates gpg tests to cover missing lines in gpg-subpackage.

- One feature, a warning about unhashed information added in
in-toto/in-toto#288, remains uncovered. To not break tox builds,
the required coverage threshold is lowered to 99%

- This commit also enables output buffering on the unittest runner,
i.e. to hide most of the test output until a test fails. See
in-toto/in-toto#240 for details.
Copy schemas in gpg.formats to formats module, adding a "GPG_"
prefix for better distinction.

This is the preferred solution to prevent circular imports between
formats and gpg.formats.

Background: gpg.formats uses some primitives from formats, such as
KEYID_SCHEMA or HEX_SCHEMA, but formats would also need gpg key
and signature schemas to extend its generic key and signature
schemas. To prevent circular imports, one of the modules would
have to duplicate schemas from the other, which is not ideal. Or,
as done here, we just merge the two modules.
Finalizes merge of gpg.formats schemas into
securesystemslib.formats.
Rename some funcs for consistency with existing securesystemslib
function and for simplicity (i.e. remove gpg_ prefix, which is
redundant with subpackage path):

gpg_export_pubkey --> export_pubkey
gpg_verify_signature --> verify_signature
gpg_sign_object --> create_signature
Adopt create and verify signature snippets in documentation to
accept data to be signed as bytes instead of strings, as changed
in secure-systems-lab#162.
@lukpueh
Copy link
Member Author

lukpueh commented Sep 4, 2019

Okay, I just cleaned up the history here. This is ready for reviewing. As mentioned above in the PR description, only lukpueh/securesystemslib@47a9d9a...lukpueh:add-gpg is new code. The rest of the history was transferred from the in-toto repository.

@rennergade, do you want to give it a try?

Copy link

@rennergade rennergade left a comment

Choose a reason for hiding this comment

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

This looks good. This is my first code review so might not have the sharpest eye.

One general comment is that it seems the imports at the beginning of this file break the conventions outlined here. I'm not sure if this is purposely ignored.

for idx, subpacket_tuple in \
enumerate(unhashed_subpacket_info + hashed_subpacket_info):

is_hashed = (idx >= len(unhashed_subpacket_info))

Choose a reason for hiding this comment

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

As an outsider, it's a little unclear to me how is_hashed is verified. I think a comment is warranted on why the index is being compared to the length.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me know if 9906acd works for you.

@lukpueh
Copy link
Member Author

lukpueh commented Sep 5, 2019

Thanks for the quick review, @rennergade!

We've been a little bit lenient regarding import-convention on the in-toto repo (where this code was written originally). There is a corresponding issue over there, but we labeled it low-prio (see in-toto/in-toto#145).

Copy link

@rennergade rennergade left a comment

Choose a reason for hiding this comment

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

Looks great, much clearer.

@lukpueh lukpueh merged commit da1af79 into secure-systems-lab:master Sep 6, 2019
lukpueh added a commit to lukpueh/securesystemslib that referenced this pull request Feb 10, 2023
This module was originally developed in in-toto and transferred to
securesystemslib in secure-systems-lab#174, primarily as Py2/Py3-agnostic wrapper
around stdlib's `subprocess.run`, to to execute `gpg`.
In secure-systems-lab#502 we switched to using `subprocess.run` directly.

Another wrapper around `subprocess.run`, provided by the module,
allows capturing standard streams but still write them to a
terminal. It was developed as specific `in-toto-run` feature and
does not need to be public API in sslib.  in-toto/in-toto#544 moves
the function back to in-toto.

closes secure-systems-lab#345,
transfers secure-systems-lab#337 (to in-toto)

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/securesystemslib that referenced this pull request Feb 10, 2023
This module was originally developed in in-toto and transferred to
securesystemslib in secure-systems-lab#174, primarily as Py2/Py3-agnostic wrapper
around stdlib's `subprocess.run`, to run the `gpg` command. In secure-systems-lab#502
we switched to using `subprocess.run` directly.

Another wrapper around `subprocess.run`, provided by the module,
allows capturing standard streams AND write them to a terminal. It
was developed as specific `in-toto-run` feature and does not need
to be public API in sslib. in-toto/in-toto#544 moves the function
back to in-toto.

closes secure-systems-lab#345,
transfers secure-systems-lab#337 (to in-toto)

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
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.

Support GPG and SSH keys
8 participants