From c0d5c673542b3d517c33fe1a9ab26bcda1a5a95f Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 15 Apr 2020 11:05:41 +0200 Subject: [PATCH] sm,dirmngr: Restrict allowed parameters used with rsaPSS. * sm/certcheck.c (extract_pss_params): Check the used PSS params. * dirmngr/crlcache.c (finish_sig_check): Ditto. * dirmngr/validate.c (check_cert_sig): Ditto. -- GnuPG-bug-id: 4538 # ------------------------ >8 ------------------------ See https://www.metzdowd.com/pipermail/cryptography/2019-November/035449.html Signed-off-by: Werner Koch --- dirmngr/crlcache.c | 23 +++++++++++++++++++++++ dirmngr/validate.c | 26 +++++++++++++++++++++++++- sm/certcheck.c | 40 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/dirmngr/crlcache.c b/dirmngr/crlcache.c index 73944667d..131a0bd8d 100644 --- a/dirmngr/crlcache.c +++ b/dirmngr/crlcache.c @@ -1731,6 +1731,29 @@ finish_sig_check (ksba_crl_t crl, gcry_md_hd_t md, int algo, algo, hashalgo); return gpg_error (GPG_ERR_INV_CRL); } + /* Add some restrictions; see ../sm/certcheck.c for details. */ + switch (algo) + { + case GCRY_MD_SHA1: + case GCRY_MD_SHA256: + case GCRY_MD_SHA384: + case GCRY_MD_SHA512: + case GCRY_MD_SHA3_256: + case GCRY_MD_SHA3_384: + case GCRY_MD_SHA3_512: + break; + default: + log_error ("PSS hash algorithm '%s' rejected\n", + gcry_md_algo_name (algo)); + return gpg_error (GPG_ERR_DIGEST_ALGO); + } + + if (gcry_md_get_algo_dlen (algo) != saltlen) + { + log_error ("PSS hash algorithm '%s' rejected due to salt length %u\n", + gcry_md_algo_name (algo), saltlen); + return gpg_error (GPG_ERR_DIGEST_ALGO); + } } diff --git a/dirmngr/validate.c b/dirmngr/validate.c index 4f893b3ff..901c165ec 100644 --- a/dirmngr/validate.c +++ b/dirmngr/validate.c @@ -1014,7 +1014,31 @@ check_cert_sig (ksba_cert_t issuer_cert, ksba_cert_t cert) gcry_sexp_release (s_sig); return gpg_error (GPG_ERR_DIGEST_ALGO); } - /* log_debug ("PSS hash=%d saltlen=%u\n", algo, saltlen); */ + /* Add some restrictions; see ../sm/certcheck.c for details. */ + switch (algo) + { + case GCRY_MD_SHA1: + case GCRY_MD_SHA256: + case GCRY_MD_SHA384: + case GCRY_MD_SHA512: + case GCRY_MD_SHA3_256: + case GCRY_MD_SHA3_384: + case GCRY_MD_SHA3_512: + break; + default: + log_error ("PSS hash algorithm '%s' rejected\n", + gcry_md_algo_name (algo)); + gcry_sexp_release (s_sig); + return gpg_error (GPG_ERR_DIGEST_ALGO); + } + + if (gcry_md_get_algo_dlen (algo) != saltlen) + { + log_error ("PSS hash algorithm '%s' rejected due to salt length %u\n", + gcry_md_algo_name (algo), saltlen); + gcry_sexp_release (s_sig); + return gpg_error (GPG_ERR_DIGEST_ALGO); + } } algo_name = hash_algo_to_string (algo); diff --git a/sm/certcheck.c b/sm/certcheck.c index 68185332e..14f78dbe6 100644 --- a/sm/certcheck.c +++ b/sm/certcheck.c @@ -300,7 +300,45 @@ extract_pss_params (gcry_sexp_t s_sig, int *r_algo, unsigned int *r_saltlen) { return gpg_error (GPG_ERR_DIGEST_ALGO); } - /* log_debug ("PSS hash=%d saltlen=%u\n", *r_algo, *r_saltlen); */ + + /* PSS has no hash function firewall like PKCS#1 and thus offers + * a path for hash algorithm replacement. To avoid this it makes + * sense to restrict the allowed hash algorithms and also allow only + * matching salt lengths. According to Peter Gutmann: + * "Beware of bugs in the above signature scheme; + * I have only proved it secure, not implemented it" + * - Apologies to Donald Knuth. + * https://www.metzdowd.com/pipermail/cryptography/2019-November/035449.html + * + * Given the set of supported algorithms currently available in + * Libgcrypt and the extra hash checks we have in some compliance + * modes, it would be hard to trick gpgsm to verify a forged + * signature. However, if eventually someone adds the xor256 hash + * algorithm (1.3.6.1.4.1.3029.3.2) to Libgcrypt we would be doomed. + */ + switch (*r_algo) + { + case GCRY_MD_SHA1: + case GCRY_MD_SHA256: + case GCRY_MD_SHA384: + case GCRY_MD_SHA512: + case GCRY_MD_SHA3_256: + case GCRY_MD_SHA3_384: + case GCRY_MD_SHA3_512: + break; + default: + log_error ("PSS hash algorithm '%s' rejected\n", + gcry_md_algo_name (*r_algo)); + return gpg_error (GPG_ERR_DIGEST_ALGO); + } + + if (gcry_md_get_algo_dlen (*r_algo) != *r_saltlen) + { + log_error ("PSS hash algorithm '%s' rejected due to salt length %u\n", + gcry_md_algo_name (*r_algo), *r_saltlen); + return gpg_error (GPG_ERR_DIGEST_ALGO); + } + return 0; }