gpg,gpgsm: Fix compliance check for DSA and avoid an assert.
authorWerner Koch <wk@gnupg.org>
Mon, 19 Jun 2017 15:50:02 +0000 (17:50 +0200)
committerWerner Koch <wk@gnupg.org>
Mon, 19 Jun 2017 17:57:11 +0000 (19:57 +0200)
* common/compliance.c (gnupg_pk_is_compliant): Swap P and Q for DSA
check.  Explicitly check for allowed ECC algos.
(gnupg_pk_is_allowed): Swap P and Q for DSA check.
* g10/mainproc.c (proc_encrypted): Simplify SYMKEYS check.  Replace
assert by debug message.

--

Note that in mainproc.c SYMKEYS is unsigned and thus a greater than 0
condition is surprising because it leads to the assumption SYMKEYS
could be negative.  Better use a boolean test.

The assert could have lead to a regression for no good reason.  Not
being compliant is better than breaking existing users.

Signed-off-by: Werner Koch <wk@gnupg.org>
common/compliance.c
common/compliance.h
g10/mainproc.c
sm/decrypt.c

index 3c43fd8..8b91677 100644 (file)
@@ -154,10 +154,10 @@ gnupg_pk_is_compliant (enum gnupg_compliance_mode compliance, int algo,
        case is_dsa:
          if (key)
            {
        case is_dsa:
          if (key)
            {
-             size_t L = gcry_mpi_get_nbits (key[0] /* p */);
-             size_t N = gcry_mpi_get_nbits (key[1] /* q */);
-             result = (L == 256
-                       && (N == 2048 || N == 3072));
+             size_t P = gcry_mpi_get_nbits (key[0]);
+             size_t Q = gcry_mpi_get_nbits (key[1]);
+             result = (Q == 256
+                       && (P == 2048 || P == 3072));
            }
          break;
 
            }
          break;
 
@@ -171,7 +171,8 @@ gnupg_pk_is_compliant (enum gnupg_compliance_mode compliance, int algo,
             }
 
           result = (curvename
             }
 
           result = (curvename
-                    && algo != PUBKEY_ALGO_EDDSA
+                    && (algo == PUBKEY_ALGO_ECDH
+                        || algo == PUBKEY_ALGO_ECDSA)
                     && (!strcmp (curvename, "brainpoolP256r1")
                         || !strcmp (curvename, "brainpoolP384r1")
                         || !strcmp (curvename, "brainpoolP512r1")));
                     && (!strcmp (curvename, "brainpoolP256r1")
                         || !strcmp (curvename, "brainpoolP384r1")
                         || !strcmp (curvename, "brainpoolP512r1")));
@@ -238,13 +239,13 @@ gnupg_pk_is_allowed (enum gnupg_compliance_mode compliance,
        case PUBKEY_ALGO_DSA:
          if (key)
            {
        case PUBKEY_ALGO_DSA:
          if (key)
            {
-             size_t L = gcry_mpi_get_nbits (key[0] /* p */);
-             size_t N = gcry_mpi_get_nbits (key[1] /* q */);
+             size_t P = gcry_mpi_get_nbits (key[0]);
+             size_t Q = gcry_mpi_get_nbits (key[1]);
              return ((use == PK_USE_SIGNING
              return ((use == PK_USE_SIGNING
-                      && L == 256
-                      && (N == 2048 || N == 3072))
+                      && Q == 256
+                      && (P == 2048 || P == 3072))
                      || (use == PK_USE_VERIFICATION
                      || (use == PK_USE_VERIFICATION
-                         && N < 2048));
+                         && P < 2048));
            }
          else
            return 0;
            }
          else
            return 0;
index 183f142..d55bbf3 100644 (file)
@@ -57,14 +57,17 @@ int gnupg_pk_is_allowed (enum gnupg_compliance_mode compliance,
 int gnupg_cipher_is_compliant (enum gnupg_compliance_mode compliance,
                                cipher_algo_t cipher,
                                enum gcry_cipher_modes mode);
 int gnupg_cipher_is_compliant (enum gnupg_compliance_mode compliance,
                                cipher_algo_t cipher,
                                enum gcry_cipher_modes mode);
-int gnupg_cipher_is_allowed (enum gnupg_compliance_mode compliance, int producer,
+int gnupg_cipher_is_allowed (enum gnupg_compliance_mode compliance,
+                             int producer,
                              cipher_algo_t cipher,
                              enum gcry_cipher_modes mode);
 int gnupg_digest_is_compliant (enum gnupg_compliance_mode compliance,
                                digest_algo_t digest);
                              cipher_algo_t cipher,
                              enum gcry_cipher_modes mode);
 int gnupg_digest_is_compliant (enum gnupg_compliance_mode compliance,
                                digest_algo_t digest);
-int gnupg_digest_is_allowed (enum gnupg_compliance_mode compliance, int producer,
+int gnupg_digest_is_allowed (enum gnupg_compliance_mode compliance,
+                             int producer,
                              digest_algo_t digest);
                              digest_algo_t digest);
-const char *gnupg_status_compliance_flag (enum gnupg_compliance_mode compliance);
+const char *gnupg_status_compliance_flag (enum gnupg_compliance_mode
+                                          compliance);
 
 struct gnupg_compliance_option
 {
 
 struct gnupg_compliance_option
 {
@@ -76,7 +79,8 @@ int gnupg_parse_compliance_option (const char *string,
                                    struct gnupg_compliance_option options[],
                                    size_t length,
                                    int quiet);
                                    struct gnupg_compliance_option options[],
                                    size_t length,
                                    int quiet);
-const char *gnupg_compliance_option_string (enum gnupg_compliance_mode compliance);
+const char *gnupg_compliance_option_string (enum gnupg_compliance_mode
+                                            compliance);
 
 
 #endif /*GNUPG_COMMON_COMPLIANCE_H*/
 
 
 #endif /*GNUPG_COMMON_COMPLIANCE_H*/
index 2db8de1..c57925c 100644 (file)
@@ -94,7 +94,7 @@ struct mainproc_context
   kbnode_t list;    /* The current list of packets. */
   iobuf_t iobuf;    /* Used to get the filename etc. */
   int trustletter;  /* Temporary usage in list_node. */
   kbnode_t list;    /* The current list of packets. */
   iobuf_t iobuf;    /* Used to get the filename etc. */
   int trustletter;  /* Temporary usage in list_node. */
-  ulong symkeys;
+  ulong symkeys;    /* Number of symmetrically encrypted session keys.  */
   struct kidlist_item *pkenc_list; /* List of encryption packets. */
   struct {
     unsigned int sig_seen:1;      /* Set to true if a signature packet
   struct kidlist_item *pkenc_list; /* List of encryption packets. */
   struct {
     unsigned int sig_seen:1;      /* Set to true if a signature packet
@@ -603,18 +603,19 @@ proc_encrypted (CTX c, PACKET *pkt)
   /* Compute compliance with CO_DE_VS.  */
   if (!result && is_status_enabled ()
       /* Symmetric encryption and asymmetric encryption voids compliance.  */
   /* Compute compliance with CO_DE_VS.  */
   if (!result && is_status_enabled ()
       /* Symmetric encryption and asymmetric encryption voids compliance.  */
-      && ((c->symkeys > 0) != (c->pkenc_list != NULL))
+      && (c->symkeys != !!c->pkenc_list )
       /* Overriding session key voids compliance.  */
       /* Overriding session key voids compliance.  */
-      && opt.override_session_key == NULL
+      && !opt.override_session_key
       /* Check symmetric cipher.  */
       /* Check symmetric cipher.  */
-      && gnupg_cipher_is_compliant (CO_DE_VS, c->dek->algo, GCRY_CIPHER_MODE_CFB))
+      && gnupg_cipher_is_compliant (CO_DE_VS, c->dek->algo,
+                                    GCRY_CIPHER_MODE_CFB))
     {
       struct kidlist_item *i;
       int compliant = 1;
       PKT_public_key *pk = xmalloc (sizeof *pk);
 
     {
       struct kidlist_item *i;
       int compliant = 1;
       PKT_public_key *pk = xmalloc (sizeof *pk);
 
-      log_assert (c->pkenc_list || c->symkeys
-                  || !"where else did the session key come from!?");
+      if ( !(c->pkenc_list || c->symkeys) )
+        log_debug ("%s: where else did the session key come from?\n", __func__);
 
       /* Now check that every key used to encrypt the session key is
        * compliant.  */
 
       /* Now check that every key used to encrypt the session key is
        * compliant.  */
index 7d43405..16181df 100644 (file)
@@ -493,9 +493,10 @@ gpgsm_decrypt (ctrl_t ctrl, int in_fd, estream_t out_fp)
                       }
 
                     /* Check that all certs are compliant with CO_DE_VS.  */
                       }
 
                     /* Check that all certs are compliant with CO_DE_VS.  */
-                    is_de_vs = (is_de_vs
-                                && gnupg_pk_is_compliant (CO_DE_VS, pk_algo, NULL,
-                                                          nbits, NULL));
+                    is_de_vs =
+                      (is_de_vs
+                       && gnupg_pk_is_compliant (CO_DE_VS, pk_algo, NULL,
+                                                 nbits, NULL));
                   }
 
                 oops:
                   }
 
                 oops: