[git] GPGME - branch, dkg/fix-T4271, created. gpgme-1.12.0-97-gac8d723

by Daniel Kahn Gillmor cvs at cvs.gnupg.org
Wed Dec 5 00:50:09 CET 2018


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GnuPG Made Easy".

The branch, dkg/fix-T4271 has been created
        at  ac8d7238dbf165950c9844e5cb41da8eb4d37bc0 (commit)

- Log -----------------------------------------------------------------
commit ac8d7238dbf165950c9844e5cb41da8eb4d37bc0
Author: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
Date:   Tue Dec 4 20:44:35 2018 +0300

    python: overhaul logic of Context.decrypt()
    
    * lang/python/src/core.py (Context.decrypt): simplify and clarify the
    logic behind handling verify=False.
    * lang/python/tests/t-decrypt.py: ensure that we test verify=False
    
    --
    
    The function-internal variables were pretty unclear to the reader, and
    the logic caused pretty nasty breakage when verify=False.
    
    GnuPG-Bug-Id: 4271
    Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>

diff --git a/lang/python/src/core.py b/lang/python/src/core.py
index f3202eb..c096ee7 100644
--- a/lang/python/src/core.py
+++ b/lang/python/src/core.py
@@ -370,8 +370,8 @@ class Context(GpgmeWrapper):
         GPGMEError	-- as signaled by the underlying library
 
         """
-        sink_result = False
-        verify_sigs = False
+        do_sig_verification = False
+        required_keys = None
         plaintext = sink if sink else Data()
 
         if passphrase is not None:
@@ -385,22 +385,25 @@ class Context(GpgmeWrapper):
             self.set_passphrase_cb(passphrase_cb)
 
         try:
-            if verify is not None:
-                if isinstance(verify, bool) is True:
-                    if verify is False:
-                        verify = True
-                        sink_result = True
-                elif isinstance(verify, list) is True:
-                    if len(verify) > 0:
-                        verify_sigs = True
-                else:
-                    verify = True
+            if isinstance(verify, bool):
+                do_sig_verification = verify
+            elif verify is None:
+                warnings.warn(
+                    "ctx.decrypt called with verify=None, should be bool or iterable (treating as False).",
+                    category=DeprecationWarning)
+                do_sig_verification = False
+            else:
+                # we hope this is an iterable:
+                required_keys = verify
+                do_sig_verification = True
+
+            if do_sig_verification:
                 self.op_decrypt_verify(ciphertext, plaintext)
             else:
                 self.op_decrypt(ciphertext, plaintext)
         except errors.GPGMEError as e:
             result = self.op_decrypt_result()
-            if verify is not None and not sink_result:
+            if do_sig_verification:
                 verify_result = self.op_verify_result()
             else:
                 verify_result = None
@@ -415,7 +418,7 @@ class Context(GpgmeWrapper):
 
         result = self.op_decrypt_result()
 
-        if verify is not None and not sink_result:
+        if do_sig_verification:
             verify_result = self.op_verify_result()
         else:
             verify_result = None
@@ -426,7 +429,7 @@ class Context(GpgmeWrapper):
             raise errors.UnsupportedAlgorithm(result.unsupported_algorithm,
                                               results=results)
 
-        if verify:
+        if do_sig_verification:
             # FIXME: should we really throw BadSignature, even if
             # we've encountered some good signatures?  as above, once
             # we hit this error, there is no way to accept it and
@@ -434,25 +437,24 @@ class Context(GpgmeWrapper):
             if any(s.status != errors.NO_ERROR
                    for s in verify_result.signatures):
                 raise errors.BadSignatures(verify_result, results=results)
-
-        if verify_sigs:
-            missing = []
-            for key in verify:
-                ok = False
-                for subkey in key.subkeys:
-                    for sig in verify_result.signatures:
-                        if sig.summary & constants.SIGSUM_VALID == 0:
-                            continue
-                        if subkey.can_sign and subkey.fpr == sig.fpr:
-                            ok = True
+            if required_keys is not None:
+                missing = []
+                for key in required_keys:
+                    ok = False
+                    for subkey in key.subkeys:
+                        for sig in verify_result.signatures:
+                            if sig.summary & constants.SIGSUM_VALID == 0:
+                                continue
+                            if subkey.can_sign and subkey.fpr == sig.fpr:
+                                ok = True
                             break
-                    if ok:
-                        break
-                if not ok:
-                    missing.append(key)
-            if missing:
-                raise errors.MissingSignatures(verify_result, missing,
-                                               results=results)
+                        if ok:
+                            break
+                    if not ok:
+                        missing.append(key)
+                if missing:
+                    raise errors.MissingSignatures(verify_result, missing,
+                                                   results=results)
 
         return results
 
diff --git a/lang/python/tests/t-decrypt.py b/lang/python/tests/t-decrypt.py
index 44743da..c72b51a 100755
--- a/lang/python/tests/t-decrypt.py
+++ b/lang/python/tests/t-decrypt.py
@@ -38,7 +38,7 @@ support.print_data(sink)
 
 # Idiomatic interface.
 with gpg.Context() as c:
-    plaintext, _, _ = c.decrypt(open(support.make_filename("cipher-1.asc")))
+    plaintext, _, _ = c.decrypt(open(support.make_filename("cipher-1.asc")), verify=False)
     assert len(plaintext) > 0
     assert plaintext.find(b'Wenn Sie dies lesen k') >= 0, \
         'Plaintext not found'

commit 5e21e61cfef809a7194ff94fb3aac525169a26b5
Author: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
Date:   Tue Dec 4 20:29:40 2018 +0300

    python: ctx.decrypt() has problematic error handling
    
    * lang/python/src/core.py (Context.decrypt): document odd
    error-handling behavior as a potential problem to be addressed.
    
    Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>

diff --git a/lang/python/src/core.py b/lang/python/src/core.py
index 3bd6f71..f3202eb 100644
--- a/lang/python/src/core.py
+++ b/lang/python/src/core.py
@@ -427,6 +427,10 @@ class Context(GpgmeWrapper):
                                               results=results)
 
         if verify:
+            # FIXME: should we really throw BadSignature, even if
+            # we've encountered some good signatures?  as above, once
+            # we hit this error, there is no way to accept it and
+            # continue to process the remaining signatures.
             if any(s.status != errors.NO_ERROR
                    for s in verify_result.signatures):
                 raise errors.BadSignatures(verify_result, results=results)

commit fefa46173e506b0e4156e73a250d6915fe229223
Author: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
Date:   Tue Dec 4 20:27:15 2018 +0300

    python: Clarify the meaning of ctx.decrypt(verify=[])
    
    * lang/python/src/core.py (Context.decrypt): docstring clarification
    of what it means to pass an empty list to the verify argument.
    
    Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>

diff --git a/lang/python/src/core.py b/lang/python/src/core.py
index 87cfd6b..3bd6f71 100644
--- a/lang/python/src/core.py
+++ b/lang/python/src/core.py
@@ -342,7 +342,10 @@ class Context(GpgmeWrapper):
 
         Decrypt the given ciphertext and verify any signatures.  If
         VERIFY is an iterable of keys, the ciphertext must be signed
-        by all those keys, otherwise an error is raised.
+        by all those keys, otherwise an error is raised.  Note: if
+        VERIFY is an empty iterable, that is treated the same as
+        passing verify=True (that is, do verify signatures, but no
+        specific keys are required).
 
         If the ciphertext is symmetrically encrypted using a
         passphrase, that passphrase can be given as parameter, using a

commit 30ddb2cabcd6e24eaa7f73d1ec099ddff5e41646
Author: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
Date:   Wed Nov 28 01:51:24 2018 -0500

    python: gpg.Context.decrypt verify_sigs and sink_result are bools
    
    Both of these function-internal variables are never used for anything
    other than a binary state.  Implement them as the booleans they are.
    Otherwise, casual readers of the code might think that they're
    supposed to represent something other than a flag (e.g. "verify_sigs"
    could mean "the signatures to verify", and "sink_result" could mean
    "the place where we sink the result").
    
    Signed-Off-By: Daniel Kahn Gillmor <dkg at fifthhorseman.net>

diff --git a/lang/python/src/core.py b/lang/python/src/core.py
index 0b7b7e0..87cfd6b 100644
--- a/lang/python/src/core.py
+++ b/lang/python/src/core.py
@@ -367,8 +367,8 @@ class Context(GpgmeWrapper):
         GPGMEError	-- as signaled by the underlying library
 
         """
-        sink_result = None
-        verify_sigs = None
+        sink_result = False
+        verify_sigs = False
         plaintext = sink if sink else Data()
 
         if passphrase is not None:
@@ -397,7 +397,7 @@ class Context(GpgmeWrapper):
                 self.op_decrypt(ciphertext, plaintext)
         except errors.GPGMEError as e:
             result = self.op_decrypt_result()
-            if verify is not None and sink_result is None:
+            if verify is not None and not sink_result:
                 verify_result = self.op_verify_result()
             else:
                 verify_result = None
@@ -412,7 +412,7 @@ class Context(GpgmeWrapper):
 
         result = self.op_decrypt_result()
 
-        if verify is not None and sink_result is None:
+        if verify is not None and not sink_result:
             verify_result = self.op_verify_result()
         else:
             verify_result = None
@@ -428,7 +428,7 @@ class Context(GpgmeWrapper):
                    for s in verify_result.signatures):
                 raise errors.BadSignatures(verify_result, results=results)
 
-        if verify_sigs is not None:
+        if verify_sigs:
             missing = []
             for key in verify:
                 ok = False

commit 9a1903cc42924851ce2452d7be2e5132f2f26332
Author: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
Date:   Wed Nov 28 01:47:20 2018 -0500

    python: clarify documentation for verify argument for Context.decrypt()
    
    It's easy to miss that verify can take a list of keys.  Make it more
    obvious to the average python dev who reads docstrings.
    
    Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>

diff --git a/lang/python/src/core.py b/lang/python/src/core.py
index f7e843f..0b7b7e0 100644
--- a/lang/python/src/core.py
+++ b/lang/python/src/core.py
@@ -352,7 +352,8 @@ class Context(GpgmeWrapper):
         Keyword arguments:
         sink		-- write result to sink instead of returning it
         passphrase	-- for symmetric decryption
-        verify		-- check signatures (default True)
+        verify		-- check signatures (boolean or iterable of keys,
+                           see above) (default True)
 
         Returns:
         plaintext	-- the decrypted data (or None if sink is given)

commit 827a2f3ad562a5927ceb89a842779b4b8c57c367
Author: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
Date:   Wed Nov 28 01:22:13 2018 -0500

    python: simplify Context.decrypt()
    
    In the course of trying to address https://dev.gnupg.org/T4271, i
    discovered that gpg.Context.decrypt() has a bit of superfluous code.
    This changeset is intended to simplify the code without making any
    functional changes.
    
    Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>

diff --git a/lang/python/src/core.py b/lang/python/src/core.py
index 6e92592..f7e843f 100644
--- a/lang/python/src/core.py
+++ b/lang/python/src/core.py
@@ -386,13 +386,9 @@ class Context(GpgmeWrapper):
                     if verify is False:
                         verify = True
                         sink_result = True
-                    else:
-                        pass
                 elif isinstance(verify, list) is True:
                     if len(verify) > 0:
                         verify_sigs = True
-                    else:
-                        pass
                 else:
                     verify = True
                 self.op_decrypt_verify(ciphertext, plaintext)
@@ -447,29 +443,8 @@ class Context(GpgmeWrapper):
                 if not ok:
                     missing.append(key)
             if missing:
-                try:
-                    raise errors.MissingSignatures(verify_result, missing,
-                                                   results=results)
-                except errors.MissingSignatures as e:
-                    raise e
-                    # mse = e
-                    # mserr = "gpg.errors.MissingSignatures:"
-                    # print(mserr, miss_e, "\n")
-                    # # The full details can then be found in mse.results,
-                    # # mse.result, mse.missing if necessary.
-                    # mse_list = []
-                    # msp = "Missing signatures from: \n".format()
-                    # print(msp)
-                    # for key in mse.missing:
-                    #     mse_list.append(key.fpr)
-                    #     msl = []
-                    #     msl.append(key.fpr)
-                    #     for user in key.uids:
-                    #         msl.append(user.name)
-                    #         msl.append(user.email)
-                    #         # msl.append(user.uid)
-                    #     print(" ".join(msl))
-                    # raise mse
+                raise errors.MissingSignatures(verify_result, missing,
+                                               results=results)
 
         return results
 

-----------------------------------------------------------------------


hooks/post-receive
-- 
GnuPG Made Easy
http://git.gnupg.org




More information about the Gnupg-commits mailing list