ENGINE-487: ENGINE-508: fix in, but buggy ENGINE-487
authorKrista 'DarthMama' Bennett <krista@pep.foundation>
Sun, 20 Jan 2019 17:27:48 +0100
branchENGINE-487
changeset 32280d6ffda184f7
parent 3226 906f09d51761
child 3229 6d555032f84c
ENGINE-487: ENGINE-508: fix in, but buggy
src/pgp_gpg.c
test/Makefile
test/src/SuiteMaker.cc
     1.1 --- a/src/pgp_gpg.c	Wed Jan 16 17:50:35 2019 +0100
     1.2 +++ b/src/pgp_gpg.c	Sun Jan 20 17:27:48 2019 +0100
     1.3 @@ -2003,31 +2003,63 @@
     1.4              *comm_type = PEP_ct_key_revoked;            
     1.5          else if (key->expired)
     1.6              *comm_type = PEP_ct_key_expired;
     1.7 +        else if (!key->subkeys)
     1.8 +            *comm_type = PEP_ct_key_b0rken;
     1.9          else {
    1.10              // Ok, so we now need to check subkeys. Normally, we could just
    1.11              // shortcut this by looking at key->can_sign and key->can_encrypt,
    1.12              // but we want the REASON we can't use a key, so this gets ugly.
    1.13              PEP_comm_type max_comm_type = *comm_type;
    1.14 -                        
    1.15 -            PEP_comm_type best_sign = PEP_ct_no_encryption;
    1.16 -            PEP_comm_type best_enc = PEP_ct_no_encryption;
    1.17 -            
    1.18 +
    1.19 +            // NOTE: 
    1.20 +            // PEP_ct_pEp functions here as an unreachable top;
    1.21 +            // it is impossible on just a key.
    1.22 +            // IF THIS CHANGES, we must choose something else.
    1.23 +            PEP_comm_type worst_sign = PEP_ct_pEp;
    1.24 +            PEP_comm_type worst_enc = PEP_ct_pEp;
    1.25 +
    1.26 +            PEP_comm_type error_sign = PEP_ct_unknown;
    1.27 +            PEP_comm_type error_enc = PEP_ct_unknown;
    1.28 +
    1.29 +            // We require that the underlying client NOT force-use expired or revoked
    1.30 +            // subkeys instead of a valid one.
    1.31 +            //
    1.32 +            // So here we check all the subkeys; we make note of the existence
    1.33 +            // of an expired, revoked, or invalid subkey, in case there is no
    1.34 +            // other alternative (we want to return useful information).
    1.35 +            // At the same time, we try to evaluate the least strong useable keys 
    1.36 +            // for signing and encryption. If there is a useable one of both,
    1.37 +            // the key comm_type corresponds to the lesser of these two least strong
    1.38 +            // keys
    1.39              for (gpgme_subkey_t sk = key->subkeys; sk != NULL; sk = sk->next) {
    1.40 +                
    1.41 +                // Only evaluate signing keys or encryption keys
    1.42                  if (sk->can_sign || sk->can_encrypt) {
    1.43                      PEP_comm_type curr_sign = PEP_ct_no_encryption;
    1.44                      PEP_comm_type curr_enc = PEP_ct_no_encryption;
    1.45 -                    
    1.46 -                    if (sk->length < 1024) {
    1.47 +
    1.48 +#ifdef GPGME_PK_ECC                    
    1.49 +                    if ((sk->pubkey_algo != GPGME_PK_ECC && sk->length < 1024) 
    1.50 +                        || (sk->pubkey_algo == GPGME_PK_ECC && sk->length < 160)) {
    1.51 +#else
    1.52 +                    if (sk->length < 1024) {                        
    1.53 +#endif                        
    1.54                          if (sk->can_sign)
    1.55                              curr_sign = PEP_ct_key_too_short;
    1.56                          if (sk->can_encrypt)                               
    1.57                              curr_enc = PEP_ct_key_too_short;
    1.58                      }
    1.59 -                    else if (
    1.60 -                        ((sk->pubkey_algo == GPGME_PK_RSA)
    1.61 -                        || (sk->pubkey_algo == GPGME_PK_RSA_E)
    1.62 -                        || (sk->pubkey_algo == GPGME_PK_RSA_S))
    1.63 -                        && sk->length == 1024) {
    1.64 +                    else if 
    1.65 +                        (
    1.66 +                            (((sk->pubkey_algo == GPGME_PK_RSA)
    1.67 +                                || (sk->pubkey_algo == GPGME_PK_RSA_E)
    1.68 +                                || (sk->pubkey_algo == GPGME_PK_RSA_S))
    1.69 +                                && sk->length == 1024)
    1.70 +#ifdef GPGME_PK_ECC                    
    1.71 +                            || (sk->pubkey_algo == GPGME_PK_ECC
    1.72 +                                && sk->length == 160)
    1.73 +#endif                             
    1.74 +                        ) {
    1.75                          if (sk->can_sign)
    1.76                              curr_sign = PEP_ct_OpenPGP_weak_unconfirmed;
    1.77                          if (sk->can_encrypt)                               
    1.78 @@ -2057,18 +2089,39 @@
    1.79                          if (sk->can_encrypt)                               
    1.80                              curr_enc = PEP_ct_key_revoked;
    1.81                      }
    1.82 -                    if (sk->can_sign)
    1.83 -                        best_sign = _MAX(curr_sign, best_sign);
    1.84 -                    if (sk->can_encrypt)
    1.85 -                        best_enc = _MAX(curr_enc, best_enc);
    1.86 +                    switch (curr_sign) {
    1.87 +                        case PEP_ct_key_b0rken:
    1.88 +                        case PEP_ct_key_expired:
    1.89 +                        case PEP_ct_key_revoked:
    1.90 +                            error_sign = curr_sign;
    1.91 +                            break;
    1.92 +                        default:    
    1.93 +                            if (sk->can_sign)
    1.94 +                                worst_sign = _MIN(curr_sign, worst_sign);
    1.95 +                            break;
    1.96 +                    }
    1.97 +                    switch (curr_enc) {
    1.98 +                        case PEP_ct_key_b0rken:
    1.99 +                        case PEP_ct_key_expired:
   1.100 +                        case PEP_ct_key_revoked:
   1.101 +                            error_sign = curr_sign;
   1.102 +                            break;
   1.103 +                        default:    
   1.104 +                            if (sk->can_encrypt)
   1.105 +                                worst_enc = _MAX(curr_enc, worst_enc);
   1.106 +                            break;
   1.107 +                    }                    
   1.108                  }    
   1.109              }
   1.110 -            if (best_enc == PEP_ct_no_encryption ||
   1.111 -                best_sign == PEP_ct_no_encryption) {
   1.112 -                *comm_type = PEP_ct_key_b0rken;
   1.113 +            if (worst_enc == PEP_ct_pEp ||
   1.114 +                worst_sign == PEP_ct_pEp) {
   1.115 +                // No valid key was found for one or both; return a useful 
   1.116 +                // error comm_type
   1.117 +                PEP_comm_type error_ct = _MAX(error_enc, error_sign);    
   1.118 +                *comm_type = (error_ct == PEP_ct_unknown ? PEP_ct_key_b0rken : error_ct);
   1.119              }
   1.120              else {
   1.121 -                *comm_type = _MIN(best_sign, _MIN(max_comm_type, best_enc));
   1.122 +                *comm_type = _MIN(max_comm_type, _MIN(worst_sign, worst_enc));
   1.123              }                
   1.124          }
   1.125          break;
     2.1 --- a/test/Makefile	Wed Jan 16 17:50:35 2019 +0100
     2.2 +++ b/test/Makefile	Sun Jan 20 17:27:48 2019 +0100
     2.3 @@ -20,7 +20,7 @@
     2.4  INC_DIRS := ./include /usr/local/include ../src ../sync ../asn.1
     2.5  INC_FLAGS := $(addprefix -I,$(INC_DIRS)) $(GPGME_INC) $(CPPUNIT_INC)
     2.6  
     2.7 -LDFLAGS += -L/usr/local/lib
     2.8 +LDFLAGS += -L/usr/local/lib -L/opt/local/lib
     2.9  
    2.10  CFLAGS += -Wno-deprecated
    2.11  CXXFLAGS += -Wno-deprecated
     3.1 --- a/test/src/SuiteMaker.cc	Wed Jan 16 17:50:35 2019 +0100
     3.2 +++ b/test/src/SuiteMaker.cc	Sun Jan 20 17:27:48 2019 +0100
     3.3 @@ -34,6 +34,7 @@
     3.4  #include "Engine358Tests.h"
     3.5  #include "BlacklistAcceptNewKeyTests.h"
     3.6  #include "DecryptAttachPrivateKeyUntrustedTests.h"
     3.7 +#include "ReturnMistrustFprTests.h"
     3.8  #include "BlacklistTests.h"
     3.9  #include "RevokeRegenAttachTests.h"
    3.10  #include "PepSubjectReceivedTests.h"
    3.11 @@ -80,6 +81,7 @@
    3.12      "Engine358Tests",
    3.13      "BlacklistAcceptNewKeyTests",
    3.14      "DecryptAttachPrivateKeyUntrustedTests",
    3.15 +    "ReturnMistrustFprTests",
    3.16      "BlacklistTests",
    3.17      "RevokeRegenAttachTests",
    3.18      "PepSubjectReceivedTests",
    3.19 @@ -104,7 +106,7 @@
    3.20  };
    3.21  
    3.22  // This file is generated, so magic constants are ok.
    3.23 -int SuiteMaker::num_suites = 43;
    3.24 +int SuiteMaker::num_suites = 44;
    3.25  
    3.26  void SuiteMaker::suitemaker_build(const char* test_class_name, const char* test_home, Test::Suite** test_suite) {
    3.27      if (strcmp(test_class_name, "MimeTests") == 0)
    3.28 @@ -151,6 +153,8 @@
    3.29          *test_suite = new BlacklistAcceptNewKeyTests(test_class_name, test_home);
    3.30      else if (strcmp(test_class_name, "DecryptAttachPrivateKeyUntrustedTests") == 0)
    3.31          *test_suite = new DecryptAttachPrivateKeyUntrustedTests(test_class_name, test_home);
    3.32 +    else if (strcmp(test_class_name, "ReturnMistrustFprTests") == 0)
    3.33 +        *test_suite = new ReturnMistrustFprTests(test_class_name, test_home);
    3.34      else if (strcmp(test_class_name, "BlacklistTests") == 0)
    3.35          *test_suite = new BlacklistTests(test_class_name, test_home);
    3.36      else if (strcmp(test_class_name, "RevokeRegenAttachTests") == 0)