It's possible for multiple certificates to have the same subject name,
and if that happens then ssl3_output_cert_chain() may select the wrong
one because it just picks a certificate by name and doesn't actually
_check_ if it really is the right one.

There's a function which gets this right; X509_STORE_CTX_get1_issuer().
We should use it. dtls1_output_cert_chain() has the same problem.

We fix the check for self-signed certificates too, while we're at it.
Although that's much less likely to be a problem in practice.

Patch applies to 0.9.8 and HEAD.

Fix the 
Index: ssl/d1_both.c
===================================================================
RCS file: /home/dwmw2/openssl-cvs/openssl/ssl/d1_both.c,v
retrieving revision 1.4.2.9
diff -u -p -r1.4.2.9 d1_both.c
--- ssl/d1_both.c       2 Apr 2009 22:32:15 -0000       1.4.2.9
+++ ssl/d1_both.c       31 May 2009 17:55:41 -0000
@@ -808,7 +808,6 @@ unsigned long dtls1_output_cert_chain(SS
        unsigned long l= 3 + DTLS1_HM_HEADER_LENGTH;
        BUF_MEM *buf;
        X509_STORE_CTX xs_ctx;
-       X509_OBJECT obj;
 
        /* TLSv1 sends a chain with nothing in it, instead of an alert */
        buf=s->init_buf;
@@ -827,6 +826,7 @@ unsigned long dtls1_output_cert_chain(SS
 
                for (;;)
                        {
+                       X509 *issuer;
                        n=i2d_X509(x,NULL);
                        if (!BUF_MEM_grow_clean(buf,(int)(n+l+3)))
                                {
@@ -837,16 +837,18 @@ unsigned long dtls1_output_cert_chain(SS
                        l2n3(n,p);
                        i2d_X509(x,&p);
                        l+=n+3;
-                       if (X509_NAME_cmp(X509_get_subject_name(x),
-                               X509_get_issuer_name(x)) == 0) break;
 
-                       i=X509_STORE_get_by_subject(&xs_ctx,X509_LU_X509,
-                               X509_get_issuer_name(x),&obj);
+                       /* self-signed */
+                       if (xs_ctx.check_issued(&xs_ctx, x, x))
+                               break;
+
+                       i=X509_STORE_CTX_get1_issuer(&issuer, &xs_ctx, x);
                        if (i <= 0) break;
-                       x=obj.data.x509;
+
                        /* Count is one too high since the X509_STORE_get uped 
the
                         * ref count */
                        X509_free(x);
+                       x = issuer;
                        }
 
                X509_STORE_CTX_cleanup(&xs_ctx);
Index: ssl/s3_both.c
===================================================================
RCS file: /home/dwmw2/openssl-cvs/openssl/ssl/s3_both.c,v
retrieving revision 1.43
diff -u -p -r1.43 s3_both.c
--- ssl/s3_both.c       26 Apr 2005 16:02:39 -0000      1.43
+++ ssl/s3_both.c       31 May 2009 17:51:31 -0000
@@ -271,8 +271,6 @@ unsigned long ssl3_output_cert_chain(SSL
        unsigned long l=7;
        BUF_MEM *buf;
        X509_STORE_CTX xs_ctx;
-       X509_OBJECT obj;
-
        int no_chain;
 
        if ((s->mode & SSL_MODE_NO_AUTO_CHAIN) || s->ctx->extra_certs)
@@ -297,6 +295,7 @@ unsigned long ssl3_output_cert_chain(SSL
 
                for (;;)
                        {
+                       X509 *issuer;
                        n=i2d_X509(x,NULL);
                        if (!BUF_MEM_grow_clean(buf,(int)(n+l+3)))
                                {
@@ -311,16 +310,17 @@ unsigned long ssl3_output_cert_chain(SSL
                        if (no_chain)
                                break;
 
-                       if (X509_NAME_cmp(X509_get_subject_name(x),
-                               X509_get_issuer_name(x)) == 0) break;
+                       /* self-signed */
+                       if (xs_ctx.check_issued(&xs_ctx, x, x))
+                               break;
 
-                       i=X509_STORE_get_by_subject(&xs_ctx,X509_LU_X509,
-                               X509_get_issuer_name(x),&obj);
+                       i=X509_STORE_CTX_get1_issuer(&issuer, &xs_ctx, x);
                        if (i <= 0) break;
-                       x=obj.data.x509;
+
                        /* Count is one too high since the X509_STORE_get uped 
the
                         * ref count */
                        X509_free(x);
+                       x = issuer;
                        }
                if (!no_chain)
                        X509_STORE_CTX_cleanup(&xs_ctx);

-- 
dwmw2

______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       [email protected]
Automated List Manager                           [email protected]

Reply via email to