CVE-2014-8275: Cryptographic Issues (CWE-310)
value: OpenSSL before 0.9.8zd, 1.0.0 before 1.0.0p, and 1.0.1 before 1.0.1k does not enforce certain constraints on certificate data, which allows remote attackers to defeat a fingerprint-based certificate-blacklist protection mechanism by including crafted data within a certificate's unsigned portion, related to crypto/asn1/a_verify.c, crypto/dsa/dsa_asn1.c, crypto/ecdsa/ecs_vrf.c, and crypto/x509/x_all.c.
CWE description: Weaknesses in this category are related to the design and implementation of data confidentiality and integrity. Frequently these deal with the use of encoding techniques, encryption libraries, and hashing algorithms. The weaknesses in this category could lead to a degradation of the quality data if they are not addressed.
Published: 2015-01-09T02:59Z
Repository: https://github.com/openssl/openssl
Language: C
Files changed: CHANGES, crypto/asn1/a_verify.c, crypto/dsa/dsa_asn1.c, crypto/ecdsa/ecs_vrf.c, crypto/x509/x_all.c
Commit date: 2015-01-05 14:35:19 +0000
Commit message: Fix various certificate fingerprint issues.

By using non-DER or invalid encodings outside the signed portion of a
certificate the fingerprint can be changed without breaking the signature.
Although no details of the signed portion of the certificate can be changed
this can cause problems with some applications: e.g. those using the
certificate fingerprint for blacklists.

1. Reject signatures with non zero unused bits.

If the BIT STRING containing the signature has non zero unused bits reject
the signature. All current signature algorithms require zero unused bits.

2. Check certificate algorithm consistency.

Check the AlgorithmIdentifier inside TBS matches the one in the
certificate signature. NB: this will result in signature failure
errors for some broken certificates.

3. Check DSA/ECDSA signatures use DER.

Reencode DSA/ECDSA signatures and compare with the original received
signature. Return an error if there is a mismatch.

This will reject various cases including garbage after signature
(thanks to Antti Karjalainen and Tuomo Untinen from the Codenomicon CROSS
program for discovering this case) and use of BER or invalid ASN.1 INTEGERs
(negative or with leading zeroes).

CVE-2014-8275
Reviewed-by: Emilia Käsper <emilia@openssl.org>

Diff: diff --git a/CHANGES b/CHANGES
index 0f37df74c..c076df8f2 100644
--- a/CHANGES
+++ b/CHANGES
@@ -657,10 +657,47 @@
      X509_CINF_set_modified, X509_CINF_get_issuer, X509_CINF_get_extensions and
      X509_CINF_get_signature were reverted post internal team review.
 
  Changes between 1.0.1j and 1.0.1k [xx XXX xxxx]
 
+  *) Fix various certificate fingerprint issues.
+
+     By using non-DER or invalid encodings outside the signed portion of a
+     certificate the fingerprint can be changed without breaking the signature.
+     Although no details of the signed portion of the certificate can be changed
+     this can cause problems with some applications: e.g. those using the
+     certificate fingerprint for blacklists.
+
+     1. Reject signatures with non zero unused bits.
+
+     If the BIT STRING containing the signature has non zero unused bits reject
+     the signature. All current signature algorithms require zero unused bits.
+
+     2. Check certificate algorithm consistency.
+
+     Check the AlgorithmIdentifier inside TBS matches the one in the
+     certificate signature. NB: this will result in signature failure
+     errors for some broken certificates.
+
+     Thanks to Konrad Kraszewski from Google for reporting this issue.
+
+     3. Check DSA/ECDSA signatures use DER.
+
+     Reencode DSA/ECDSA signatures and compare with the original received
+     signature. Return an error if there is a mismatch.
+
+     This will reject various cases including garbage after signature
+     (thanks to Antti Karjalainen and Tuomo Untinen from the Codenomicon CROSS
+     program for discovering this case) and use of BER or invalid ASN.1 INTEGERs
+     (negative or with leading zeroes).
+
+     Further analysis was conducted and fixes were developed by Stephen Henson
+     of the OpenSSL core team.
+
+     (CVE-2014-8275)
+     [Steve Henson]
+
    *) Do not resume sessions on the server if the negotiated protocol
       version does not match the session's version. Resuming with a different
       version, while not strictly forbidden by the RFC, is of questionable
       sanity and breaks all known clients.
       [David Benjamin, Emilia Käsper]
diff --git a/crypto/asn1/a_verify.c b/crypto/asn1/a_verify.c
index aacf4763b..fdeeef676 100644
--- a/crypto/asn1/a_verify.c
+++ b/crypto/asn1/a_verify.c
@@ -88,10 +88,16 @@ int ASN1_verify(i2d_of_void *i2d, X509_ALGOR *a, ASN1_BIT_STRING *signature,
 	if (type == NULL)
 		{
 		ASN1err(ASN1_F_ASN1_VERIFY,ASN1_R_UNKNOWN_MESSAGE_DIGEST_ALGORITHM);
 		goto err;
 		}
+
+	if (signature->type == V_ASN1_BIT_STRING && signature->flags & 0x7)
+		{
+		ASN1err(ASN1_F_ASN1_VERIFY, ASN1_R_INVALID_BIT_STRING_BITS_LEFT);
+		goto err;
+		}
 	
 	inl=i2d(data,NULL);
 	buf_in=OPENSSL_malloc((unsigned int)inl);
 	if (buf_in == NULL)
 		{
@@ -148,10 +154,16 @@ int ASN1_item_verify(const ASN1_ITEM *it, X509_ALGOR *a,
 		{
 		ASN1err(ASN1_F_ASN1_ITEM_VERIFY, ERR_R_PASSED_NULL_PARAMETER);
 		return -1;
 		}
 
+	if (signature->type == V_ASN1_BIT_STRING && signature->flags & 0x7)
+		{
+		ASN1err(ASN1_F_ASN1_VERIFY, ASN1_R_INVALID_BIT_STRING_BITS_LEFT);
+		return -1;
+		}
+
 	EVP_MD_CTX_init(&ctx);
 
 	/* Convert signature OID into digest and public key OIDs */
 	if (!OBJ_find_sigid_algs(OBJ_obj2nid(a->algorithm), &mdnid, &pknid))
 		{
diff --git a/crypto/dsa/dsa_asn1.c b/crypto/dsa/dsa_asn1.c
index 55c75b59c..58559e54b 100644
--- a/crypto/dsa/dsa_asn1.c
+++ b/crypto/dsa/dsa_asn1.c
@@ -175,15 +175,27 @@ int DSA_sign(int type, const unsigned char *dgst, int dlen, unsigned char *sig,
  */
 int DSA_verify(int type, const unsigned char *dgst, int dgst_len,
 	     const unsigned char *sigbuf, int siglen, DSA *dsa)
 	{
 	DSA_SIG *s;
+	const unsigned char *p = sigbuf;
+	unsigned char *der = NULL;
+	int derlen = -1;
 	int ret=-1;
 
 	s = DSA_SIG_new();
 	if (s == NULL) return(ret);
-	if (d2i_DSA_SIG(&s,&sigbuf,siglen) == NULL) goto err;
+	if (d2i_DSA_SIG(&s,&p,siglen) == NULL) goto err;
+	/* Ensure signature uses DER and doesn't have trailing garbage */
+	derlen = i2d_DSA_SIG(s, &der);
+	if (derlen != siglen || memcmp(sigbuf, der, derlen))
+		goto err;
 	ret=DSA_do_verify(dgst,dgst_len,s,dsa);
 err:
+	if (derlen > 0)
+		{
+		OPENSSL_cleanse(der, derlen);
+		OPENSSL_free(der);
+		}
 	DSA_SIG_free(s);
 	return(ret);
 	}
diff --git a/crypto/ecdsa/ecs_vrf.c b/crypto/ecdsa/ecs_vrf.c
index ae14625e4..7191b8ab0 100644
--- a/crypto/ecdsa/ecs_vrf.c
+++ b/crypto/ecdsa/ecs_vrf.c
@@ -55,10 +55,11 @@
  * Hudson (tjh@cryptsoft.com).
  *
  */
 
 #include "ecs_locl.h"
+#include "cryptlib.h"
 #ifndef OPENSSL_NO_ENGINE
 #include <openssl/engine.h>
 #endif
 
 /*-
@@ -84,15 +85,27 @@ int ECDSA_do_verify(const unsigned char *dgst, int dgst_len,
  */
 int ECDSA_verify(int type, const unsigned char *dgst, int dgst_len,
 		const unsigned char *sigbuf, int sig_len, EC_KEY *eckey)
  	{
 	ECDSA_SIG *s;
+	const unsigned char *p = sigbuf;
+	unsigned char *der = NULL;
+	int derlen = -1;
 	int ret=-1;
 
 	s = ECDSA_SIG_new();
 	if (s == NULL) return(ret);
-	if (d2i_ECDSA_SIG(&s, &sigbuf, sig_len) == NULL) goto err;
+	if (d2i_ECDSA_SIG(&s, &p, sig_len) == NULL) goto err;
+	/* Ensure signature uses DER and doesn't have trailing garbage */
+	derlen = i2d_ECDSA_SIG(s, &der);
+	if (derlen != sig_len || memcmp(sigbuf, der, derlen))
+		goto err;
 	ret=ECDSA_do_verify(dgst, dgst_len, s, eckey);
 err:
+	if (derlen > 0)
+		{
+		OPENSSL_cleanse(der, derlen);
+		OPENSSL_free(der);
+		}
 	ECDSA_SIG_free(s);
 	return(ret);
 	}
diff --git a/crypto/x509/x_all.c b/crypto/x509/x_all.c
index b2223ce93..d7229506f 100644
--- a/crypto/x509/x_all.c
+++ b/crypto/x509/x_all.c
@@ -71,10 +71,12 @@
 #include <openssl/dsa.h>
 #endif
 
 int X509_verify(X509 *a, EVP_PKEY *r)
 	{
+	if (X509_ALGOR_cmp(a->sig_alg, a->cert_info->signature))
+		return 0;
 	return(ASN1_item_verify(ASN1_ITEM_rptr(X509_CINF),a->sig_alg,
 		a->signature,a->cert_info,r));
 	}
 
 int X509_REQ_verify(X509_REQ *a, EVP_PKEY *r)

Vulnerable code: 	if (d2i_DSA_SIG(&s,&sigbuf,siglen) == NULL) goto err;
	if (d2i_ECDSA_SIG(&s, &sigbuf, sig_len) == NULL) goto err;
Fixed code:   *) Fix various certificate fingerprint issues.

     By using non-DER or invalid encodings outside the signed portion of a
     certificate the fingerprint can be changed without breaking the signature.
     Although no details of the signed portion of the certificate can be changed
     this can cause problems with some applications: e.g. those using the
     certificate fingerprint for blacklists.

     1. Reject signatures with non zero unused bits.

     If the BIT STRING containing the signature has non zero unused bits reject
     the signature. All current signature algorithms require zero unused bits.

     2. Check certificate algorithm consistency.

     Check the AlgorithmIdentifier inside TBS matches the one in the
     certificate signature. NB: this will result in signature failure
     errors for some broken certificates.

     Thanks to Konrad Kraszewski from Google for reporting this issue.

     3. Check DSA/ECDSA signatures use DER.

     Reencode DSA/ECDSA signatures and compare with the original received
     signature. Return an error if there is a mismatch.

     This will reject various cases including garbage after signature
     (thanks to Antti Karjalainen and Tuomo Untinen from the Codenomicon CROSS
     program for discovering this case) and use of BER or invalid ASN.1 INTEGERs
     (negative or with leading zeroes).

     Further analysis was conducted and fixes were developed by Stephen Henson
     of the OpenSSL core team.

     (CVE-2014-8275)
     [Steve Henson]


	if (signature->type == V_ASN1_BIT_STRING && signature->flags & 0x7)
		{
		ASN1err(ASN1_F_ASN1_VERIFY, ASN1_R_INVALID_BIT_STRING_BITS_LEFT);
		goto err;
		}
	if (signature->type == V_ASN1_BIT_STRING && signature->flags & 0x7)
		{
		ASN1err(ASN1_F_ASN1_VERIFY, ASN1_R_INVALID_BIT_STRING_BITS_LEFT);
		return -1;
		}

	const unsigned char *p = sigbuf;
	unsigned char *der = NULL;
	int derlen = -1;
	if (d2i_DSA_SIG(&s,&p,siglen) == NULL) goto err;
	/* Ensure signature uses DER and doesn't have trailing garbage */
	derlen = i2d_DSA_SIG(s, &der);
	if (derlen != siglen || memcmp(sigbuf, der, derlen))
		goto err;
	if (derlen > 0)
		{
		OPENSSL_cleanse(der, derlen);
		OPENSSL_free(der);
		}
#include "cryptlib.h"
	const unsigned char *p = sigbuf;
	unsigned char *der = NULL;
	int derlen = -1;
	if (d2i_ECDSA_SIG(&s, &p, sig_len) == NULL) goto err;
	/* Ensure signature uses DER and doesn't have trailing garbage */
	derlen = i2d_ECDSA_SIG(s, &der);
	if (derlen != sig_len || memcmp(sigbuf, der, derlen))
		goto err;
	if (derlen > 0)
		{
		OPENSSL_cleanse(der, derlen);
		OPENSSL_free(der);
		}
	if (X509_ALGOR_cmp(a->sig_alg, a->cert_info->signature))
		return 0;