From 3621dbe52584bc8b417f61b5370ebaa5598db956 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Mon, 19 Jun 2017 17:50:02 +0200 Subject: gpg,gpgsm: Fix compliance check for DSA and avoid an assert. * 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 --- common/compliance.c | 21 +++++++++++---------- common/compliance.h | 12 ++++++++---- g10/mainproc.c | 13 +++++++------ sm/decrypt.c | 7 ++++--- 4 files changed, 30 insertions(+), 23 deletions(-) diff --git a/common/compliance.c b/common/compliance.c index 3c43fd821..8b9167758 100644 --- a/common/compliance.c +++ b/common/compliance.c @@ -154,10 +154,10 @@ gnupg_pk_is_compliant (enum gnupg_compliance_mode compliance, int algo, 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; @@ -171,7 +171,8 @@ gnupg_pk_is_compliant (enum gnupg_compliance_mode compliance, int algo, } result = (curvename - && algo != PUBKEY_ALGO_EDDSA + && (algo == PUBKEY_ALGO_ECDH + || algo == PUBKEY_ALGO_ECDSA) && (!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) { - 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 - && L == 256 - && (N == 2048 || N == 3072)) + && Q == 256 + && (P == 2048 || P == 3072)) || (use == PK_USE_VERIFICATION - && N < 2048)); + && P < 2048)); } else return 0; diff --git a/common/compliance.h b/common/compliance.h index 183f142e7..d55bbf3ac 100644 --- a/common/compliance.h +++ b/common/compliance.h @@ -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_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); -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); -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 { @@ -76,7 +79,8 @@ int gnupg_parse_compliance_option (const char *string, 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*/ diff --git a/g10/mainproc.c b/g10/mainproc.c index 2db8de1d5..c57925c9f 100644 --- a/g10/mainproc.c +++ b/g10/mainproc.c @@ -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. */ - 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 @@ -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. */ - && ((c->symkeys > 0) != (c->pkenc_list != NULL)) + && (c->symkeys != !!c->pkenc_list ) /* Overriding session key voids compliance. */ - && opt.override_session_key == NULL + && !opt.override_session_key /* 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); - 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. */ diff --git a/sm/decrypt.c b/sm/decrypt.c index 7d43405f4..16181df00 100644 --- a/sm/decrypt.c +++ b/sm/decrypt.c @@ -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. */ - 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: -- cgit v1.2.3