From 720b299e82bcbb35e9a4e54362da431982841cb9 Mon Sep 17 00:00:00 2001 From: Ben McGinnes Date: Fri, 6 Feb 2015 18:11:52 +1100 Subject: [PATCH 1/4] Clearsign authorisation via subkey. Replacement code which might work to enable advanced keys with signing subkeys to be correctly handled by the bot by associating the subkey with the relevant master key. Signing format still only clearsigning, the key details are more important and auth via encrypted token and decryption is likely to be more reliable anyway as there is far less chance of some other protocol messing with the signed content. Effectively no chance, though the odd corrupted packet here and there is still possible. Whereas with clearsigning it can be broken by all manner of rewriting in transit (which happens often enough with signed email as it is). See also Issue #1045 for greater detail of what needs to be fixed and what is to be done about it. --- plugins/User/plugin.py | 56 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 10 deletions(-) diff --git a/plugins/User/plugin.py b/plugins/User/plugin.py index 87968b455..61e6a70b2 100644 --- a/plugins/User/plugin.py +++ b/plugins/User/plugin.py @@ -500,6 +500,7 @@ class User(callbacks.Plugin): r'-----BEGIN PGP SIGNATURE-----\r?\n.*' r'\r?\n-----END PGP SIGNATURE-----', re.S) + @internationalizeDocstring def auth(self, irc, msg, args, url): """ @@ -524,20 +525,55 @@ class User(callbacks.Plugin): verified = gpg.keyring.verify(data) if verified and verified.valid: keyid = verified.key_id + fprint = verified.pubkey_fingerprint + kprint = fprint[-16:] prefix, expiry = self._tokens.pop(token) found = False - for (id, user) in ircdb.users.items(): - if keyid in [x[-len(keyid):] for x in user.gpgkeys]: - try: - user.addAuth(msg.prefix) - except ValueError: - irc.error(_('Your secure flag is true and your ' - 'hostmask doesn\'t match any of your ' - 'known hostmasks.'), Raise=True) + pkeys = gpg.list_keys(False) + pnum = len(pkeys) + for x in range(pnum): + if keyid or kprint in pkeys[x]["keyid"] and keyid in user.gpgkeys and if keyid is kprint: + user.addAuth(msg.prefix) ircdb.users.setUser(user, flush=False) - irc.reply(_('You are now authenticated as %s.') % - user.name) + irc.reply(_('You are now authenticated as %s with %s.') + % (user.name, keyid)) return + elif keyid or kprint in pkeys[x]["keyid"] and keyid not in user.gpgkeys and kprint is in user.gpgkeys and keyid is not kprint: + user.addAuth(msg.prefix) + ircdb.users.setUser(user, flush=False) + irc.reply(_('You are now authenticated as %s with %s using the %s subkey.') + % (user.name, keyid, kprint)) + return + elif keyid or kprint in pkeys[x]["keyid"] and keyid is kprint and keyid not in user.gpgkeys: + irc.error(_('I have a record of key %s, but it is not associated with the %s account.') % (keyid, user.name)) + return + elif keyid or kprint in pkeys[x]["keyid"] and keyid is not kprint and keyid not in user.gpgkeys and kprint not in user.gpgkeys: + irc.error(_('I have a record of key %s, but it is not associated with any account.') % (keyid)) + return + elif keyid is kprint and keyid not in pkeys[x]["keyid"] and keyid in user.gpgkeys: + irc.error(_('The %s key is registered to the %s account, but not currently available to me. Please add the key again') % (keyid, user.name)) + # Possibly replace this with key retrieval attempt. + # try: + # code to retrieve key from server + # except AnErrorOfSomeKind: + # the current error message. + return + elif keyid and kprint not in pkeys[x]["keyid"] and keyid is not kprint and keyid not in user.gpgkeys and kprint not in user.gpgkeys: + irc.error(_('Unknown GPG key.'), Raise=True) + return + #for (id, user) in ircdb.users.items(): + # if keyid in [x[-len(keyid):] for x in user.gpgkeys]: + # user.addAuth(msg.prefix) + # try: + # user.addAuth(msg.prefix) + # except ValueError: + # irc.error(_('Your secure flag is true and your ' + # 'hostmask doesn\'t match any of your ' + # 'known hostmasks.'), Raise=True) + # ircdb.users.setUser(user, flush=False) + # irc.reply(_('You are now authenticated as %s.') % + # user.name) + # return irc.error(_('Unknown GPG key.'), Raise=True) else: irc.error(_('Signature could not be verified. Make sure ' From 432b8f8fb536207970c37ef8c9f0f56df64b83f2 Mon Sep 17 00:00:00 2001 From: Ben McGinnes Date: Fri, 6 Feb 2015 21:33:30 +1100 Subject: [PATCH 2/4] Solved the subkey selection issue. Changes one line and adds six to do this: * change keyid = verified.keyid to be keyid0; * added an if/else check to see if it's the subkey or master key; and * then set keyid according the result of that check; * then continues as normal. --- plugins/User/plugin.py | 58 +++++++++++------------------------------- 1 file changed, 15 insertions(+), 43 deletions(-) diff --git a/plugins/User/plugin.py b/plugins/User/plugin.py index 61e6a70b2..3c16614bb 100644 --- a/plugins/User/plugin.py +++ b/plugins/User/plugin.py @@ -524,56 +524,28 @@ class User(callbacks.Plugin): 'Authentication aborted.'), Raise=True) verified = gpg.keyring.verify(data) if verified and verified.valid: - keyid = verified.key_id + keyid0 = verified.key_id fprint = verified.pubkey_fingerprint kprint = fprint[-16:] prefix, expiry = self._tokens.pop(token) found = False - pkeys = gpg.list_keys(False) - pnum = len(pkeys) - for x in range(pnum): - if keyid or kprint in pkeys[x]["keyid"] and keyid in user.gpgkeys and if keyid is kprint: + if keyid0 == kprint: + keyid = keyid0 + else: + keyid = kprint + for (id, user) in ircdb.users.items(): + if keyid in [x[-len(keyid):] for x in user.gpgkeys]: user.addAuth(msg.prefix) + try: + user.addAuth(msg.prefix) + except ValueError: + irc.error(_('Your secure flag is true and your ' + 'hostmask doesn\'t match any of your ' + 'known hostmasks.'), Raise=True) ircdb.users.setUser(user, flush=False) - irc.reply(_('You are now authenticated as %s with %s.') - % (user.name, keyid)) + irc.reply(_('You are now authenticated as %s.') % + user.name) return - elif keyid or kprint in pkeys[x]["keyid"] and keyid not in user.gpgkeys and kprint is in user.gpgkeys and keyid is not kprint: - user.addAuth(msg.prefix) - ircdb.users.setUser(user, flush=False) - irc.reply(_('You are now authenticated as %s with %s using the %s subkey.') - % (user.name, keyid, kprint)) - return - elif keyid or kprint in pkeys[x]["keyid"] and keyid is kprint and keyid not in user.gpgkeys: - irc.error(_('I have a record of key %s, but it is not associated with the %s account.') % (keyid, user.name)) - return - elif keyid or kprint in pkeys[x]["keyid"] and keyid is not kprint and keyid not in user.gpgkeys and kprint not in user.gpgkeys: - irc.error(_('I have a record of key %s, but it is not associated with any account.') % (keyid)) - return - elif keyid is kprint and keyid not in pkeys[x]["keyid"] and keyid in user.gpgkeys: - irc.error(_('The %s key is registered to the %s account, but not currently available to me. Please add the key again') % (keyid, user.name)) - # Possibly replace this with key retrieval attempt. - # try: - # code to retrieve key from server - # except AnErrorOfSomeKind: - # the current error message. - return - elif keyid and kprint not in pkeys[x]["keyid"] and keyid is not kprint and keyid not in user.gpgkeys and kprint not in user.gpgkeys: - irc.error(_('Unknown GPG key.'), Raise=True) - return - #for (id, user) in ircdb.users.items(): - # if keyid in [x[-len(keyid):] for x in user.gpgkeys]: - # user.addAuth(msg.prefix) - # try: - # user.addAuth(msg.prefix) - # except ValueError: - # irc.error(_('Your secure flag is true and your ' - # 'hostmask doesn\'t match any of your ' - # 'known hostmasks.'), Raise=True) - # ircdb.users.setUser(user, flush=False) - # irc.reply(_('You are now authenticated as %s.') % - # user.name) - # return irc.error(_('Unknown GPG key.'), Raise=True) else: irc.error(_('Signature could not be verified. Make sure ' From 861efee8f2439bd638a70b405cda765ef82e74c7 Mon Sep 17 00:00:00 2001 From: Ben McGinnes Date: Fri, 6 Feb 2015 21:44:20 +1100 Subject: [PATCH 3/4] Removed a relic of poor coding options. --- plugins/User/plugin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/User/plugin.py b/plugins/User/plugin.py index 3c16614bb..172bdecc4 100644 --- a/plugins/User/plugin.py +++ b/plugins/User/plugin.py @@ -535,7 +535,6 @@ class User(callbacks.Plugin): keyid = kprint for (id, user) in ircdb.users.items(): if keyid in [x[-len(keyid):] for x in user.gpgkeys]: - user.addAuth(msg.prefix) try: user.addAuth(msg.prefix) except ValueError: From a7bbc46eb9979ec4d35694d046bc8c52209592cc Mon Sep 17 00:00:00 2001 From: Ben McGinnes Date: Sat, 7 Feb 2015 04:27:51 +1100 Subject: [PATCH 4/4] Streamlining the patch back down to a single line. Since the keyid should always match the master key, regardless of whether there's a subkey or not, reduced this to simply make keyid be the last 16 chars of the master key's fingerprint. --- plugins/User/plugin.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/plugins/User/plugin.py b/plugins/User/plugin.py index 172bdecc4..39a03a089 100644 --- a/plugins/User/plugin.py +++ b/plugins/User/plugin.py @@ -524,15 +524,9 @@ class User(callbacks.Plugin): 'Authentication aborted.'), Raise=True) verified = gpg.keyring.verify(data) if verified and verified.valid: - keyid0 = verified.key_id - fprint = verified.pubkey_fingerprint - kprint = fprint[-16:] + keyid = verified.pubkey_fingerprint[-16:] prefix, expiry = self._tokens.pop(token) found = False - if keyid0 == kprint: - keyid = keyid0 - else: - keyid = kprint for (id, user) in ircdb.users.items(): if keyid in [x[-len(keyid):] for x in user.gpgkeys]: try: