strongswan: backport upstream fixes for CVEs in gmp plugin 7146/head
authorMagnus Kroken <mkroken@gmail.com>
Fri, 5 Oct 2018 23:23:32 +0000 (01:23 +0200)
committerMagnus Kroken <mkroken@gmail.com>
Fri, 5 Oct 2018 23:31:10 +0000 (01:31 +0200)
This fixes:
* CVE-2018-16151
* CVE-2018-16152
* CVE-2018-17540

Details:
https://strongswan.org/blog/2018/09/24/strongswan-vulnerability-(cve-2018-16151,-cve-2018-16152).html
https://strongswan.org/blog/2018/10/01/strongswan-vulnerability-(cve-2018-17540).html

Signed-off-by: Magnus Kroken <mkroken@gmail.com>
net/strongswan/Makefile
net/strongswan/patches/010-gmp-cve-2018-16151-16152.patch [new file with mode: 0644]
net/strongswan/patches/011-gmp-cve-2018-17540.patch [new file with mode: 0644]

index 7bccc9fc1d4281723c10558649b2fdea2102af54..105f989b5e920484bd805a47ae9a2f25caa4b2b7 100644 (file)
@@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk
 
 PKG_NAME:=strongswan
 PKG_VERSION:=5.6.3
-PKG_RELEASE:=2
+PKG_RELEASE:=3
 
 PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.bz2
 PKG_HASH:=c3c7dc8201f40625bba92ffd32eb602a8909210d8b3fac4d214c737ce079bf24
diff --git a/net/strongswan/patches/010-gmp-cve-2018-16151-16152.patch b/net/strongswan/patches/010-gmp-cve-2018-16151-16152.patch
new file mode 100644 (file)
index 0000000..63446a1
--- /dev/null
@@ -0,0 +1,322 @@
+From 511563ce763b1e98e3fb3b3e3addfef550ff99b2 Mon Sep 17 00:00:00 2001
+From: Tobias Brunner <tobias@strongswan.org>
+Date: Tue, 28 Aug 2018 11:26:24 +0200
+Subject: [PATCH] gmp: Don't parse PKCS1 v1.5 RSA signatures to verify them
+
+Instead we generate the expected signature encoding and compare it to the
+decrypted value.
+
+Due to the lenient nature of the previous parsing code (minimum padding
+length was not enforced, the algorithmIdentifier/OID parser accepts arbitrary
+data after OIDs and in the parameters field etc.) it was susceptible to
+Daniel Bleichenbacher's low-exponent attack (from 2006!), which allowed
+forging signatures for keys that use low public exponents (i.e. e=3).
+
+Since the public exponent is usually set to 0x10001 (65537) since quite a
+while, the flaws in the previous code should not have had that much of a
+practical impact in recent years.
+
+Fixes: CVE-2018-16151, CVE-2018-16152
+---
+ .../plugins/gmp/gmp_rsa_private_key.c              |  66 +++++----
+ src/libstrongswan/plugins/gmp/gmp_rsa_public_key.c | 156 ++-------------------
+ 2 files changed, 52 insertions(+), 170 deletions(-)
+
+diff --git a/src/libstrongswan/plugins/gmp/gmp_rsa_private_key.c b/src/libstrongswan/plugins/gmp/gmp_rsa_private_key.c
+index 241ef7d3b12b..edc178e96a4f 100644
+--- a/src/libstrongswan/plugins/gmp/gmp_rsa_private_key.c
++++ b/src/libstrongswan/plugins/gmp/gmp_rsa_private_key.c
+@@ -264,14 +264,15 @@ static chunk_t rsasp1(private_gmp_rsa_private_key_t *this, chunk_t data)
+ }
+ /**
+- * Build a signature using the PKCS#1 EMSA scheme
++ * Hashes the data and builds the plaintext signature value with EMSA
++ * PKCS#1 v1.5 padding.
++ *
++ * Allocates the signature data.
+  */
+-static bool build_emsa_pkcs1_signature(private_gmp_rsa_private_key_t *this,
+-                                                                         hash_algorithm_t hash_algorithm,
+-                                                                         chunk_t data, chunk_t *signature)
++bool gmp_emsa_pkcs1_signature_data(hash_algorithm_t hash_algorithm,
++                                                                 chunk_t data, size_t keylen, chunk_t *em)
+ {
+       chunk_t digestInfo = chunk_empty;
+-      chunk_t em;
+       if (hash_algorithm != HASH_UNKNOWN)
+       {
+@@ -295,43 +296,56 @@ static bool build_emsa_pkcs1_signature(private_gmp_rsa_private_key_t *this,
+               /* build DER-encoded digestInfo */
+               digestInfo = asn1_wrap(ASN1_SEQUENCE, "mm",
+                                               asn1_algorithmIdentifier(hash_oid),
+-                                              asn1_simple_object(ASN1_OCTET_STRING, hash)
+-                                        );
+-              chunk_free(&hash);
++                                              asn1_wrap(ASN1_OCTET_STRING, "m", hash));
++
+               data = digestInfo;
+       }
+-      if (data.len > this->k - 3)
++      if (data.len > keylen - 11)
+       {
+-              free(digestInfo.ptr);
+-              DBG1(DBG_LIB, "unable to sign %d bytes using a %dbit key", data.len,
+-                       mpz_sizeinbase(this->n, 2));
++              chunk_free(&digestInfo);
++              DBG1(DBG_LIB, "signature value of %zu bytes is too long for key of "
++                       "%zu bytes", data.len, keylen);
+               return FALSE;
+       }
+-      /* build chunk to rsa-decrypt:
+-       * EM = 0x00 || 0x01 || PS || 0x00 || T.
+-       * PS = 0xFF padding, with length to fill em
++      /* EM = 0x00 || 0x01 || PS || 0x00 || T.
++       * PS = 0xFF padding, with length to fill em (at least 8 bytes)
+        * T = encoded_hash
+        */
+-      em.len = this->k;
+-      em.ptr = malloc(em.len);
++      *em = chunk_alloc(keylen);
+       /* fill em with padding */
+-      memset(em.ptr, 0xFF, em.len);
++      memset(em->ptr, 0xFF, em->len);
+       /* set magic bytes */
+-      *(em.ptr) = 0x00;
+-      *(em.ptr+1) = 0x01;
+-      *(em.ptr + em.len - data.len - 1) = 0x00;
+-      /* set DER-encoded hash */
+-      memcpy(em.ptr + em.len - data.len, data.ptr, data.len);
++      *(em->ptr) = 0x00;
++      *(em->ptr+1) = 0x01;
++      *(em->ptr + em->len - data.len - 1) = 0x00;
++      /* set encoded hash */
++      memcpy(em->ptr + em->len - data.len, data.ptr, data.len);
++
++      chunk_clear(&digestInfo);
++      return TRUE;
++}
++
++/**
++ * Build a signature using the PKCS#1 EMSA scheme
++ */
++static bool build_emsa_pkcs1_signature(private_gmp_rsa_private_key_t *this,
++                                                                         hash_algorithm_t hash_algorithm,
++                                                                         chunk_t data, chunk_t *signature)
++{
++      chunk_t em;
++
++      if (!gmp_emsa_pkcs1_signature_data(hash_algorithm, data, this->k, &em))
++      {
++              return FALSE;
++      }
+       /* build signature */
+       *signature = rsasp1(this, em);
+-      free(digestInfo.ptr);
+-      free(em.ptr);
+-
++      chunk_free(&em);
+       return TRUE;
+ }
+diff --git a/src/libstrongswan/plugins/gmp/gmp_rsa_public_key.c b/src/libstrongswan/plugins/gmp/gmp_rsa_public_key.c
+index 52bc9fb38046..ce9a80fadf2a 100644
+--- a/src/libstrongswan/plugins/gmp/gmp_rsa_public_key.c
++++ b/src/libstrongswan/plugins/gmp/gmp_rsa_public_key.c
+@@ -70,7 +70,9 @@ struct private_gmp_rsa_public_key_t {
+ /**
+  * Shared functions defined in gmp_rsa_private_key.c
+  */
+-extern chunk_t gmp_mpz_to_chunk(const mpz_t value);
++chunk_t gmp_mpz_to_chunk(const mpz_t value);
++bool gmp_emsa_pkcs1_signature_data(hash_algorithm_t hash_algorithm,
++                                                                 chunk_t data, size_t keylen, chunk_t *em);
+ /**
+  * RSAEP algorithm specified in PKCS#1.
+@@ -115,26 +117,13 @@ static chunk_t rsavp1(private_gmp_rsa_public_key_t *this, chunk_t data)
+ }
+ /**
+- * ASN.1 definition of digestInfo
+- */
+-static const asn1Object_t digestInfoObjects[] = {
+-      { 0, "digestInfo",                      ASN1_SEQUENCE,          ASN1_OBJ  }, /*  0 */
+-      { 1,   "digestAlgorithm",       ASN1_EOC,                       ASN1_RAW  }, /*  1 */
+-      { 1,   "digest",                        ASN1_OCTET_STRING,      ASN1_BODY }, /*  2 */
+-      { 0, "exit",                            ASN1_EOC,                       ASN1_EXIT }
+-};
+-#define DIGEST_INFO                                   0
+-#define DIGEST_INFO_ALGORITHM         1
+-#define DIGEST_INFO_DIGEST                    2
+-
+-/**
+  * Verification of an EMSA PKCS1 signature described in PKCS#1
+  */
+ static bool verify_emsa_pkcs1_signature(private_gmp_rsa_public_key_t *this,
+                                                                               hash_algorithm_t algorithm,
+                                                                               chunk_t data, chunk_t signature)
+ {
+-      chunk_t em_ori, em;
++      chunk_t em_expected, em;
+       bool success = FALSE;
+       /* remove any preceding 0-bytes from signature */
+@@ -148,140 +137,19 @@ static bool verify_emsa_pkcs1_signature(private_gmp_rsa_public_key_t *this,
+               return FALSE;
+       }
+-      /* unpack signature */
+-      em_ori = em = rsavp1(this, signature);
+-
+-      /* result should look like this:
+-       * EM = 0x00 || 0x01 || PS || 0x00 || T.
+-       * PS = 0xFF padding, with length to fill em
+-       * T = oid || hash
+-       */
+-
+-      /* check magic bytes */
+-      if (em.len < 2 || *(em.ptr) != 0x00 || *(em.ptr+1) != 0x01)
++      /* generate expected signature value */
++      if (!gmp_emsa_pkcs1_signature_data(algorithm, data, this->k, &em_expected))
+       {
+-              goto end;
+-      }
+-      em = chunk_skip(em, 2);
+-
+-      /* find magic 0x00 */
+-      while (em.len > 0)
+-      {
+-              if (*em.ptr == 0x00)
+-              {
+-                      /* found magic byte, stop */
+-                      em = chunk_skip(em, 1);
+-                      break;
+-              }
+-              else if (*em.ptr != 0xFF)
+-              {
+-                      /* bad padding, decryption failed ?!*/
+-                      goto end;
+-              }
+-              em = chunk_skip(em, 1);
+-      }
+-
+-      if (em.len == 0)
+-      {
+-              /* no digestInfo found */
+-              goto end;
+-      }
+-
+-      if (algorithm == HASH_UNKNOWN)
+-      {   /* IKEv1 signatures without digestInfo */
+-              if (em.len != data.len)
+-              {
+-                      DBG1(DBG_LIB, "hash size in signature is %u bytes instead of"
+-                               " %u bytes", em.len, data.len);
+-                      goto end;
+-              }
+-              success = memeq_const(em.ptr, data.ptr, data.len);
++              return FALSE;
+       }
+-      else
+-      {   /* IKEv2 and X.509 certificate signatures */
+-              asn1_parser_t *parser;
+-              chunk_t object;
+-              int objectID;
+-              hash_algorithm_t hash_algorithm = HASH_UNKNOWN;
+-
+-              DBG2(DBG_LIB, "signature verification:");
+-              parser = asn1_parser_create(digestInfoObjects, em);
+-              while (parser->iterate(parser, &objectID, &object))
+-              {
+-                      switch (objectID)
+-                      {
+-                              case DIGEST_INFO:
+-                              {
+-                                      if (em.len > object.len)
+-                                      {
+-                                              DBG1(DBG_LIB, "digestInfo field in signature is"
+-                                                       " followed by %u surplus bytes",
+-                                                       em.len - object.len);
+-                                              goto end_parser;
+-                                      }
+-                                      break;
+-                              }
+-                              case DIGEST_INFO_ALGORITHM:
+-                              {
+-                                      int hash_oid = asn1_parse_algorithmIdentifier(object,
+-                                                                               parser->get_level(parser)+1, NULL);
+-
+-                                      hash_algorithm = hasher_algorithm_from_oid(hash_oid);
+-                                      if (hash_algorithm == HASH_UNKNOWN || hash_algorithm != algorithm)
+-                                      {
+-                                              DBG1(DBG_LIB, "expected hash algorithm %N, but found"
+-                                                       " %N (OID: %#B)", hash_algorithm_names, algorithm,
+-                                                       hash_algorithm_names, hash_algorithm,  &object);
+-                                              goto end_parser;
+-                                      }
+-                                      break;
+-                              }
+-                              case DIGEST_INFO_DIGEST:
+-                              {
+-                                      chunk_t hash;
+-                                      hasher_t *hasher;
+-
+-                                      hasher = lib->crypto->create_hasher(lib->crypto, hash_algorithm);
+-                                      if (hasher == NULL)
+-                                      {
+-                                              DBG1(DBG_LIB, "hash algorithm %N not supported",
+-                                                       hash_algorithm_names, hash_algorithm);
+-                                              goto end_parser;
+-                                      }
+-
+-                                      if (object.len != hasher->get_hash_size(hasher))
+-                                      {
+-                                              DBG1(DBG_LIB, "hash size in signature is %u bytes"
+-                                                       " instead of %u bytes", object.len,
+-                                                       hasher->get_hash_size(hasher));
+-                                              hasher->destroy(hasher);
+-                                              goto end_parser;
+-                                      }
+-
+-                                      /* build our own hash and compare */
+-                                      if (!hasher->allocate_hash(hasher, data, &hash))
+-                                      {
+-                                              hasher->destroy(hasher);
+-                                              goto end_parser;
+-                                      }
+-                                      hasher->destroy(hasher);
+-                                      success = memeq_const(object.ptr, hash.ptr, hash.len);
+-                                      free(hash.ptr);
+-                                      break;
+-                              }
+-                              default:
+-                                      break;
+-                      }
+-              }
++      /* unpack signature */
++      em = rsavp1(this, signature);
+-end_parser:
+-              success &= parser->success(parser);
+-              parser->destroy(parser);
+-      }
++      success = chunk_equals_const(em_expected, em);
+-end:
+-      free(em_ori.ptr);
++      chunk_free(&em_expected);
++      chunk_free(&em);
+       return success;
+ }
+-- 
+2.7.4
+
diff --git a/net/strongswan/patches/011-gmp-cve-2018-17540.patch b/net/strongswan/patches/011-gmp-cve-2018-17540.patch
new file mode 100644 (file)
index 0000000..225a5c8
--- /dev/null
@@ -0,0 +1,38 @@
+From 129ab919a8c3abfc17bea776f0774e0ccf33ca09 Mon Sep 17 00:00:00 2001
+From: Tobias Brunner <tobias@strongswan.org>
+Date: Tue, 25 Sep 2018 14:50:08 +0200
+Subject: [PATCH] gmp: Fix buffer overflow with very small RSA keys
+
+Because `keylen` is unsigned the subtraction results in an integer
+underflow if the key length is < 11 bytes.
+
+This is only a problem when verifying signatures with a public key (for
+private keys the plugin enforces a minimum modulus length) and to do so
+we usually only use trusted keys.  However, the x509 plugin actually
+calls issued_by() on a parsed certificate to check if it is self-signed,
+which is the reason this issue was found by OSS-Fuzz in the first place.
+So, unfortunately, this can be triggered by sending an invalid client
+cert to a peer.
+
+Fixes: 5955db5b124a ("gmp: Don't parse PKCS1 v1.5 RSA signatures to verify them")
+Fixes: CVE-2018-17540
+---
+ src/libstrongswan/plugins/gmp/gmp_rsa_private_key.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/src/libstrongswan/plugins/gmp/gmp_rsa_private_key.c b/src/libstrongswan/plugins/gmp/gmp_rsa_private_key.c
+index e9a83fdf49a1..a255a40abce2 100644
+--- a/src/libstrongswan/plugins/gmp/gmp_rsa_private_key.c
++++ b/src/libstrongswan/plugins/gmp/gmp_rsa_private_key.c
+@@ -301,7 +301,7 @@ bool gmp_emsa_pkcs1_signature_data(hash_algorithm_t hash_algorithm,
+               data = digestInfo;
+       }
+-      if (data.len > keylen - 11)
++      if (keylen < 11 || data.len > keylen - 11)
+       {
+               chunk_free(&digestInfo);
+               DBG1(DBG_LIB, "signature value of %zu bytes is too long for key of "
+-- 
+2.7.4
+