[feat] Extend Key expiration and sign public key #171

ayoyoness · 2016-12-01T09:59:05Z

Key expiry date extension:

use --edit-key but confining it to key expiry through an interface
only allow for 1 subkey extension

Public key signing:

using --sign-key and passing passphrase and confirmation through stdin ( file descriptor 0)
does not cater for key signing from non-default secret keys

ayoyoness · 2016-12-01T10:01:22Z

This combines #167
and #168.
They are closed now.

- use --edit-key but confining it to key expiry through an interface - only allow for 1 subkey extension - extension period is counted from the creation date

…when only signing verification fails

zariye · 2017-01-10T11:50:09Z

@kalikaneko you labeled this pr for the next-release do you know when that will be, or what we could do about to move things forward? ;) As far as I know it has been reviewed a few days after you have been started with the current release by @bwagnerr, something else? I know we spoke about last month, but I just wanted to make sure things do not get forgotten from both sides. =)

meskio

It looks nice, and works fine in my tests. I think is a great contribution.

I left a bunch comments, mostly about naming (why is always naming so hard?).

Also, the commits a1b4897 and a9f71c9 could be merged in the previous commits so we have a cleaner git history.

meskio · 2017-02-09T11:54:13Z

gnupg/gnupg.py

@@ -558,6 +558,39 @@ def _parse_keys(self, result):
            if keyword in valid_keywords:
                getattr(result, keyword)(L)

+    def extend_key(self, keyid, validity='1y', passphrase=None, extend_subkey=True):


I find a big weird the name extend_key when the validity parameter is taken from it's creation date instead, it's too easy to acutally shorten the expiration date by mistake. What about rename it to expire as it's called in gnupg?

meskio · 2017-02-09T12:14:21Z

gnupg/gnupg.py

+                               to extend the key, from its creation date.
+        :param str passphrase: passphrase used when creating the key, leave None otherwise
+        :param bool extend_subkey: to indicate whether the first subkey will also extended
+                by the same period --default is True


I think extend_subkey should act on all subkeys, not just on the first one. Or let you choose what subkey to act on. It's a bit estrange to hardcode it to modify only the first subkey.

You're right! We should definitely act on all subkeys, but we're not sure how to determine how many keys there are. Any ideas how to do this?

What about self.list_sigs(keyid)[0]['subkeys']? It somehow feels hacky to use list_sigs for that, but list_keys will return the whole keyring which might be slow for big keyrings.

Maybe we should have a method to get keys from the fingerprint/uid/..., maybe an extra param in list_keys?

Anyway I'm happy with the hacky solution for now.

Good call, @meskio.
We tried to make a unit test to check if all subkeys were getting the expiration time changed, but we couldn't find a way to fetch the individual expiration time of each subkey.

meskio · 2017-02-09T12:27:08Z

gnupg/gnupg.py

+
+        :param str keyid: key shortID, longID, email_address or fingerprint
+        :param str validity: 0 or number of days (d), or weeks (*w) , or months (*m) or years (*y)
+                               to extend the key, from its creation date.


validity is a concept by itself in gnupg related to the trust of the key (https://www.gnupg.org/gph/en/manual/x334.html).

Maybe we can come up with a better name for this param. time? expiration_time?

meskio · 2017-02-09T12:36:43Z

gnupg/gnupg.py

@@ -496,6 +496,38 @@ def list_packets(self, raw_data):
                        result)
        return result

+    def sign_key(self, keyid, passphrase=None):
+        """ sign (an imported) public key - keyid, with default secret key


It will be handy to have a default_key param like in the sign method, to be able to select the secret key to be used to sign with. I will be ok with just adding a comment with a TODO if this requires some work.

meskio · 2017-02-09T12:51:35Z

gnupg/gnupg.py

+        result = self._result_map['signing'](self)
+        confirm_command = "%sy\n" % input_command
+        p.stdin.write(confirm_command)
+        self._collect_output(p, result, stdin=p.stdin)


Why not use self._handel_io here? I think it will deal with all the subprocess basically the same way that is done here. But I'm still confused why some commands use it and others not.

We saw that self._handle_io uses files and we're not reading from files. We're not sure if it makes sense to write to a stream just to use a method. What do you think?

Makes sense, let's keep it like that then.

@tuliocasagrande

with @tuliocasagrande

@tuliocasagrande

This better reflects that the expiration date will change independently of previous date. with @tuliocasagrande

meskio · 2017-02-16T11:16:11Z

gnupg/_parsers.py

@@ -878,13 +878,13 @@ def __init__(self, gpg):
        self._gpg = gpg
        self.status = 'ok'

-    def _handle_status(self, key, value):
+    def _handle_status(self, key):


I think this value is needed.

Now when I run it I get:

In [9]: gpg.expire('73102FE1', expiration_time='1w') Exception in thread Thread-548: Traceback (most recent call last): File "/usr/lib/python2.7/threading.py", line 801, in __bootstrap_inner self.run() File "/usr/lib/python2.7/threading.py", line 754, in run self.__target(*self.__args, **self.__kwargs) File "gnupg/_meta.py", line 650, in _read_response result._handle_status(keyword, value) TypeError: _handle_status() takes exactly 2 arguments (3 given) Out[9]: <gnupg._parsers.KeyExpirationResult at 0x7fe91a9d06d0>

@anikarni

with @anikarni

We decided to use list_sigs to get the number of subkeys, because the list_keys will return the whole keyring, which might be slow for big keyrings.

The expire method was raising IndexError when passing an inexisting keyid. I prefered to use a try..except block instead of checking the return of list_sigs, because this solution is easier to extend in case of list_sigs changes to raise KeyNotFound or something.

meskio · 2017-04-06T15:12:20Z

I've merged it into develop.

ayoyoness mentioned this pull request Dec 1, 2016

python-gnupg: key expiration date extension and public key signing pixelated/pixelated-user-agent#859

Closed

NavaL added 6 commits December 21, 2016 13:35

[feat] adding key expiry date extension functionality

41e6556

- use --edit-key but confining it to key expiry through an interface - only allow for 1 subkey extension - extension period is counted from the creation date

[feat] allowing extension of expired keys

c74cb23

[bug] refactoring and allowing passphrases with spaces

a1b4897

[feat] adds public key signing functionality

9464fe8

[feat] allowing extension of expired keys

4c68108

[bug] reverting KEY_NOT_FOUND output parsing, as it fails decryption …

a9f71c9

…when only signing verification fails

bwagnerr force-pushed the key_extension_and_sign branch from fe25279 to a9f71c9 Compare December 21, 2016 15:35

kalikaneko added enhancement next-release labels Dec 26, 2016

meskio requested changes Feb 9, 2017

View reviewed changes

Anike Arni added 2 commits February 14, 2017 16:09

[feat] Allow default key parameter for signing key

3cf0ef7

with @tuliocasagrande

[feat] Rename key extension to expiration

16392e8

This better reflects that the expiration date will change independently of previous date. with @tuliocasagrande

meskio reviewed Feb 16, 2017

View reviewed changes

[feat] Change validity parameter to expiration_time

1f90790

with @anikarni

tuliocasagrande force-pushed the key_extension_and_sign branch from dc759ed to 4c9623c Compare February 22, 2017 17:24

[feat] Change expiration time for all subkeys

2aafbc7

We decided to use list_sigs to get the number of subkeys, because the list_keys will return the whole keyring, which might be slow for big keyrings.

tuliocasagrande force-pushed the key_extension_and_sign branch from 4c9623c to 2aafbc7 Compare February 22, 2017 19:05

meskio approved these changes Apr 6, 2017

View reviewed changes

meskio closed this Apr 6, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feat] Extend Key expiration and sign public key #171

[feat] Extend Key expiration and sign public key #171

ayoyoness commented Dec 1, 2016

ayoyoness commented Dec 1, 2016

zariye commented Jan 10, 2017

meskio left a comment

meskio Feb 9, 2017

meskio Feb 9, 2017

tuliocasagrande Feb 14, 2017

meskio Feb 16, 2017

tuliocasagrande Feb 22, 2017 •

edited

meskio Feb 9, 2017

meskio Feb 9, 2017

meskio Feb 9, 2017

tuliocasagrande Feb 14, 2017

meskio Feb 16, 2017

meskio Feb 16, 2017

meskio commented Apr 6, 2017

[feat] Extend Key expiration and sign public key #171

[feat] Extend Key expiration and sign public key #171

Conversation

ayoyoness commented Dec 1, 2016

ayoyoness commented Dec 1, 2016

zariye commented Jan 10, 2017

meskio 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

tuliocasagrande Feb 22, 2017 • edited

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

meskio commented Apr 6, 2017

tuliocasagrande Feb 22, 2017 •

edited