CVE-2018-20954: Improper Authentication (CWE-287)
value: The "Security and Privacy" Encryption feature in Mailpile before 1.0.0rc4 does not exclude disabled, revoked, and expired keys.
CWE description: When an actor claims to have a given identity, the product does not prove or insufficiently proves that the claim is correct.
CVSS v3 base score: 7.5
Published: 2019-08-08T21:15Z
Repository: https://github.com/mailpile/Mailpile
Language: Python
Files changed: mailpile/crypto/gpgi.py, mailpile/plugins/crypto_gnupg.py, shared-data/default-theme/html/profiles/account-form.html
Commit date: 2018-09-21 12:17:11 -0400
Commit message: First working (but rough) version to omit revoked and disabled keys from
Add account-Security and Privacy-Encryption pulldown.

Diff: diff --git a/mailpile/crypto/gpgi.py b/mailpile/crypto/gpgi.py
index 7f026e39..6f26381b 100644
--- a/mailpile/crypto/gpgi.py
+++ b/mailpile/crypto/gpgi.py
@@ -808,10 +808,13 @@ class GnuPG:
 
         # Another unfortunate thing GPG does, is it hides the disabled
         # state when listing secret keys; it seems internally only the
         # public key is disabled. This makes it hard for us to reason about
         # which keys can actually be used, so we compensate...
+        # *** FIXME JackDca 2018-09-21 - Above behaviour not seen in 2.1.18 if
+        # --with-colons is used (but true for human-readable output) so this
+        # code could be deleted.
         list_keys = ["--fingerprint"]
         for fprint in set(secret_keys):
             list_keys += ["--list-keys", fprint]
         retvals = self.run(list_keys)
         public_keys = self.parse_keylist(retvals[1]["stdout"])
diff --git a/mailpile/plugins/crypto_gnupg.py b/mailpile/plugins/crypto_gnupg.py
index 7603dff6..f21fa603 100644
--- a/mailpile/plugins/crypto_gnupg.py
+++ b/mailpile/plugins/crypto_gnupg.py
@@ -385,15 +385,28 @@ class GPGKeyList(Command):
 
 class GPGKeyListSecret(Command):
     """List Secret GPG Keys"""
     ORDER = ('', 0)
     SYNOPSIS = (None, 'crypto/gpg/keylist/secret',
-                'crypto/gpg/keylist/secret', '<address>')
+                                    'crypto/gpg/keylist/secret', '[<check>]')
     HTTP_CALLABLE = ('GET', )
+    HTTP_QUERY_VARS = {'check': 'True to omit disabled, expired, revoked keys'}
 
     def command(self):
-        res = self._gnupg().list_secret_keys()
+        args = list(self.args)
+        if len(args) > 0:
+            check = args[0]
+        else:
+            check = self.data.get('check', '')
+        check = 'True' in check
+        
+        all = self._gnupg().list_secret_keys()
+        if check:
+            res = {fprint : all[fprint] for fprint in all
+                if not (all[fprint]['revoked'] or all[fprint]['disabled'])}
+        else:
+            res = all
         return self._success("Searched for secret keys", res)
 
 
 class GPGUsageStatistics(Search):
     """Get usage statistics from mail, given an address"""
diff --git a/shared-data/default-theme/html/profiles/account-form.html b/shared-data/default-theme/html/profiles/account-form.html
index 649552f4..7b7d8ae2 100644
--- a/shared-data/default-theme/html/profiles/account-form.html
+++ b/shared-data/default-theme/html/profiles/account-form.html
@@ -712,11 +712,11 @@
         <select class='fpa-pgp-key' style="width: 100%"
                 onchange="javascript:_fpa.select(this, 'security-opt');"
                 name="security-pgp-key">
           <option value="!CREATE:RSA2048">{{_("Create a new 2048 bit RSA key")}}</option>
           <option value="!CREATE:RSA3072" class="fpa-pgp-key-default">{{_("Create a new 3072 bit RSA key")}}</option>
-        {%- set pgp_keys = mailpile('crypto/gpg/keylist/secret').result %}
+        {%- set pgp_keys = mailpile('crypto/gpg/keylist/secret','True').result %}
         {%- for fingerprint in pgp_keys -%}
           {%- set key = pgp_keys[fingerprint] -%}
           {%- for uid in key.uids %}
           <option value="{{fingerprint}}" data-uid="{{ uid.email }}"
                   {%- if (fingerprint == result['security-pgp-key']) and (uid.email == result.email) %} selected{% endif %}

Vulnerable code:                 'crypto/gpg/keylist/secret', '<address>')
        res = self._gnupg().list_secret_keys()
        {%- set pgp_keys = mailpile('crypto/gpg/keylist/secret').result %}
Fixed code:         # *** FIXME JackDca 2018-09-21 - Above behaviour not seen in 2.1.18 if
        # --with-colons is used (but true for human-readable output) so this
        # code could be deleted.
                                    'crypto/gpg/keylist/secret', '[<check>]')
    HTTP_QUERY_VARS = {'check': 'True to omit disabled, expired, revoked keys'}
        args = list(self.args)
        if len(args) > 0:
            check = args[0]
        else:
            check = self.data.get('check', '')
        check = 'True' in check
        
        all = self._gnupg().list_secret_keys()
        if check:
            res = {fprint : all[fprint] for fprint in all
                if not (all[fprint]['revoked'] or all[fprint]['disabled'])}
        else:
            res = all
        {%- set pgp_keys = mailpile('crypto/gpg/keylist/secret','True').result %}