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>
(cherry picked from commit 208a6012be3077d83df4475f32dd1b1446f3a02e)

Conflicts:
	crypto/dsa/dsa_vrf.c

diff --git a/CHANGES b/CHANGES
index d236eea832c144a3d68ac888c010761486f1dea2..60a45965d3ed1a4332614f58020c4488b09c5c0b 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -4,7 +4,42 @@
 
  Changes between 0.9.8zc and 0.9.8zd [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]
 
  Changes between 0.9.8zb and 0.9.8zc [15 Oct 2014]
 

diff --git a/crypto/asn1/a_verify.c b/crypto/asn1/a_verify.c
index 7ded69b170f665825ee1d58135c12862c756f5f2..3ef363d7604df24589b6cc8c7c1f600fba01257c 100644 (file)
--- a/crypto/asn1/a_verify.c
+++ b/crypto/asn1/a_verify.c
@@ -89,6 +89,12 @@ int ASN1_verify(i2d_of_void *i2d, X509_ALGOR *a, ASN1_BIT_STRING *signature,
                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);
@@ -144,6 +150,12 @@ int ASN1_item_verify(const ASN1_ITEM *it, X509_ALGOR *a, ASN1_BIT_STRING *signat
                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);
        i=OBJ_obj2nid(a->algorithm);
        type=EVP_get_digestbyname(OBJ_nid2sn(i));

diff --git a/crypto/dsa/dsa_asn1.c b/crypto/dsa/dsa_asn1.c
index bc7d7a02a347dd3a66be17f09388cd8e546ac12a..08d47720fa1e5b17a0fe58a591b5616aadc2ff4a 100644 (file)
--- a/crypto/dsa/dsa_asn1.c
+++ b/crypto/dsa/dsa_asn1.c
@@ -200,7 +200,11 @@ 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;
+
 #ifdef OPENSSL_FIPS
        if(FIPS_mode() && !(dsa->flags & DSA_FLAG_NON_FIPS_ALLOW))
                {
@@ -211,10 +215,18 @@ int DSA_verify(int type, const unsigned char *dgst, int dgst_len,
 
        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 ef9acf7b610284d697bcea815061ef29a67fce0e..2836efe5eff228bf9ef0e707f723f1b6e8ba937e 100644 (file)
--- a/crypto/ecdsa/ecs_vrf.c
+++ b/crypto/ecdsa/ecs_vrf.c
@@ -57,6 +57,7 @@
  */
 
 #include "ecs_locl.h"
+#include "cryptlib.h"
 #ifndef OPENSSL_NO_ENGINE
 #include <openssl/engine.h>
 #endif
@@ -84,13 +85,25 @@ 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 c7b07f7c9d58d366459cf91083afb491d1d5d228..f4c68fc17c23fb492b97aa62f5b09df640cb9e08 100644 (file)
--- a/crypto/x509/x_all.c
+++ b/crypto/x509/x_all.c
@@ -73,6 +73,8 @@
 
 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));
        }