[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Tue Aug 4 12:29:49 UTC 2015


The branch master has been updated
       via  0bc09ecd263acb25f04f373f31a50f50af8541bb (commit)
       via  44128847e8965ec64384ac48c65f5d28126b3666 (commit)
      from  8d11b7c7ee84ad0aa243476088285d15b22c5470 (commit)


- Log -----------------------------------------------------------------
commit 0bc09ecd263acb25f04f373f31a50f50af8541bb
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Aug 4 11:44:52 2015 +0100

    PACKETise ClientCertificate processing
    
    Use the PACKET API for processing ClientCertificate messages
    
    Reviewed-by: Tim Hudson <tjh at openssl.org>

commit 44128847e8965ec64384ac48c65f5d28126b3666
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Aug 4 13:03:20 2015 +0100

    Fix a bug in the new PACKET implementation
    
    Some of the PACKET functions were returning incorrect data. An unfortunate
    choice of test data in the unit test was masking the failure.
    
    Reviewed-by: Tim Hudson <tjh at openssl.org>

-----------------------------------------------------------------------

Summary of changes:
 ssl/packet_locl.h | 16 ++++++++--------
 ssl/s3_srvr.c     | 33 ++++++++++++++++++++-------------
 test/packettest.c | 49 +++++++++++++++++++++++++------------------------
 3 files changed, 53 insertions(+), 45 deletions(-)

diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h
index 4aab5cb..80d0b93 100644
--- a/ssl/packet_locl.h
+++ b/ssl/packet_locl.h
@@ -176,8 +176,8 @@ __owur static inline int PACKET_peek_net_3(PACKET *pkt, unsigned long *data)
         return 0;
 
     *data  = ((unsigned long)(*pkt->curr)) << 16;
-    *data |= ((unsigned long)(*pkt->curr + 1)) <<  8;
-    *data |= *pkt->curr + 2;
+    *data |= ((unsigned long)(*(pkt->curr + 1))) <<  8;
+    *data |= *(pkt->curr + 2);
 
     return 1;
 }
@@ -203,9 +203,9 @@ __owur static inline int PACKET_peek_net_4(PACKET *pkt, unsigned long *data)
         return 0;
 
     *data  = ((unsigned long)(*pkt->curr)) << 24;
-    *data |= ((unsigned long)(*pkt->curr + 1)) << 16;
-    *data |= ((unsigned long)(*pkt->curr + 2)) <<  8;
-    *data |= *pkt->curr+3;
+    *data |= ((unsigned long)(*(pkt->curr + 1))) << 16;
+    *data |= ((unsigned long)(*(pkt->curr + 2))) <<  8;
+    *data |= *(pkt->curr+3);
 
     return 1;
 }
@@ -254,9 +254,9 @@ __owur static inline int PACKET_peek_4(PACKET *pkt, unsigned long *data)
         return 0;
 
     *data  = *pkt->curr;
-    *data |= ((unsigned long)(*pkt->curr + 1)) <<  8;
-    *data |= ((unsigned long)(*pkt->curr + 2)) << 16;
-    *data |= ((unsigned long)(*pkt->curr + 3)) << 24;
+    *data |= ((unsigned long)(*(pkt->curr + 1))) <<  8;
+    *data |= ((unsigned long)(*(pkt->curr + 2))) << 16;
+    *data |= ((unsigned long)(*(pkt->curr + 3))) << 24;
 
     return 1;
 }
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index fd4c87e..079d9be 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -3012,10 +3012,11 @@ int ssl3_get_client_certificate(SSL *s)
 {
     int i, ok, al, ret = -1;
     X509 *x = NULL;
-    unsigned long l, nc, llen, n;
-    const unsigned char *p, *q;
-    unsigned char *d;
+    unsigned long l, llen, n;
+    const unsigned char *certstart;
+    unsigned char *certbytes;
     STACK_OF(X509) *sk = NULL;
+    PACKET pkt, spkt;
 
     n = s->method->ssl_get_message(s,
                                    SSL3_ST_SR_CERT_A,
@@ -3051,35 +3052,42 @@ int ssl3_get_client_certificate(SSL *s)
         SSLerr(SSL_F_SSL3_GET_CLIENT_CERTIFICATE, SSL_R_WRONG_MESSAGE_TYPE);
         goto f_err;
     }
-    p = d = (unsigned char *)s->init_msg;
+
+    if (!PACKET_buf_init(&pkt, s->init_msg, n)) {
+        al = SSL_AD_INTERNAL_ERROR;
+        SSLerr(SSL_F_SSL3_GET_CLIENT_CERTIFICATE, ERR_R_INTERNAL_ERROR);
+        goto f_err;
+    }
 
     if ((sk = sk_X509_new_null()) == NULL) {
         SSLerr(SSL_F_SSL3_GET_CLIENT_CERTIFICATE, ERR_R_MALLOC_FAILURE);
         goto done;
     }
 
-    n2l3(p, llen);
-    if (llen + 3 != n) {
+    if (!PACKET_get_net_3(&pkt, &llen)
+            || !PACKET_get_sub_packet(&pkt, &spkt, llen)
+            || PACKET_remaining(&pkt) != 0) {
         al = SSL_AD_DECODE_ERROR;
         SSLerr(SSL_F_SSL3_GET_CLIENT_CERTIFICATE, SSL_R_LENGTH_MISMATCH);
         goto f_err;
     }
-    for (nc = 0; nc < llen;) {
-        n2l3(p, l);
-        if ((l + nc + 3) > llen) {
+
+    while (PACKET_remaining(&spkt) > 0) {
+        if (!PACKET_get_net_3(&spkt, &l)
+                || !PACKET_get_bytes(&spkt, &certbytes, l)) {
             al = SSL_AD_DECODE_ERROR;
             SSLerr(SSL_F_SSL3_GET_CLIENT_CERTIFICATE,
                    SSL_R_CERT_LENGTH_MISMATCH);
             goto f_err;
         }
 
-        q = p;
-        x = d2i_X509(NULL, &p, l);
+        certstart = certbytes;
+        x = d2i_X509(NULL, (const unsigned char **)&certbytes, l);
         if (x == NULL) {
             SSLerr(SSL_F_SSL3_GET_CLIENT_CERTIFICATE, ERR_R_ASN1_LIB);
             goto done;
         }
-        if (p != (q + l)) {
+        if (certbytes != (certstart + l)) {
             al = SSL_AD_DECODE_ERROR;
             SSLerr(SSL_F_SSL3_GET_CLIENT_CERTIFICATE,
                    SSL_R_CERT_LENGTH_MISMATCH);
@@ -3090,7 +3098,6 @@ int ssl3_get_client_certificate(SSL *s)
             goto done;
         }
         x = NULL;
-        nc += l + 3;
     }
 
     if (sk_X509_num(sk) <= 0) {
diff --git a/test/packettest.c b/test/packettest.c
index 92181e6..1ddb837 100644
--- a/test/packettest.c
+++ b/test/packettest.c
@@ -81,10 +81,10 @@ static int test_PACKET_get_1(PACKET *pkt, size_t start)
 
     if (       !PACKET_goto_bookmark(pkt, start)
             || !PACKET_get_1(pkt, &i)
-            ||  i != 0x01
+            ||  i != 0x02
             || !PACKET_forward(pkt, BUF_LEN - 2)
             || !PACKET_get_1(pkt, &i)
-            ||  i != 0xff
+            ||  i != 0xfe
             ||  PACKET_get_1(pkt, &i)) {
         fprintf(stderr, "test_PACKET_get_1() failed\n");
         return 0;
@@ -99,10 +99,10 @@ static int test_PACKET_get_4(PACKET *pkt, size_t start)
 
     if (       !PACKET_goto_bookmark(pkt, start)
             || !PACKET_get_4(pkt, &i)
-            ||  i != 0x04030201UL
+            ||  i != 0x08060402UL
             || !PACKET_forward(pkt, BUF_LEN - 8)
             || !PACKET_get_4(pkt, &i)
-            ||  i != 0xfffefdfcUL
+            ||  i != 0xfefcfaf8UL
             ||  PACKET_get_4(pkt, &i)) {
         fprintf(stderr, "test_PACKET_get_4() failed\n");
         return 0;
@@ -117,10 +117,10 @@ static int test_PACKET_get_net_2(PACKET *pkt, size_t start)
 
     if (       !PACKET_goto_bookmark(pkt, start)
             || !PACKET_get_net_2(pkt, &i)
-            ||  i != 0x0102
+            ||  i != 0x0204
             || !PACKET_forward(pkt, BUF_LEN - 4)
             || !PACKET_get_net_2(pkt, &i)
-            ||  i != 0xfeff
+            ||  i != 0xfcfe
             ||  PACKET_get_net_2(pkt, &i)) {
         fprintf(stderr, "test_PACKET_get_net_2() failed\n");
         return 0;
@@ -135,11 +135,12 @@ static int test_PACKET_get_net_3(PACKET *pkt, size_t start)
 
     if (       !PACKET_goto_bookmark(pkt, start)
             || !PACKET_get_net_3(pkt, &i)
-            ||  i != 0x010203UL
+            ||  i != 0x020406UL
             || !PACKET_forward(pkt, BUF_LEN - 6)
             || !PACKET_get_net_3(pkt, &i)
-            ||  i != 0xfdfeffUL
+            ||  i != 0xfafcfeUL
             ||  PACKET_get_net_3(pkt, &i)) {
+        fprintf(stderr, "i is %ld\n", i);
         fprintf(stderr, "test_PACKET_get_net_3() failed\n");
         return 0;
     }
@@ -153,10 +154,10 @@ static int test_PACKET_get_net_4(PACKET *pkt, size_t start)
 
     if (       !PACKET_goto_bookmark(pkt, start)
             || !PACKET_get_net_4(pkt, &i)
-            ||  i != 0x01020304UL
+            ||  i != 0x02040608UL
             || !PACKET_forward(pkt, BUF_LEN - 8)
             || !PACKET_get_net_4(pkt, &i)
-            ||  i != 0xfcfdfeffUL
+            ||  i != 0xf8fafcfeUL
             ||  PACKET_get_net_4(pkt, &i)) {
         fprintf(stderr, "test_PACKET_get_net_4() failed\n");
         return 0;
@@ -173,12 +174,12 @@ static int test_PACKET_get_sub_packet(PACKET *pkt, size_t start)
     if (       !PACKET_goto_bookmark(pkt, start)
             || !PACKET_get_sub_packet(pkt, &subpkt, 4)
             || !PACKET_get_net_4(&subpkt, &i)
-            ||  i != 0x01020304UL
+            ||  i != 0x02040608UL
             ||  PACKET_remaining(&subpkt)
             || !PACKET_forward(pkt, BUF_LEN - 8)
             || !PACKET_get_sub_packet(pkt, &subpkt, 4)
             || !PACKET_get_net_4(&subpkt, &i)
-            ||  i != 0xfcfdfeffUL
+            ||  i != 0xf8fafcfeUL
             ||  PACKET_remaining(&subpkt)
             ||  PACKET_get_sub_packet(pkt, &subpkt, 4)) {
         fprintf(stderr, "test_PACKET_get_sub_packet() failed\n");
@@ -194,13 +195,13 @@ static int test_PACKET_get_bytes(PACKET *pkt, size_t start)
 
     if (       !PACKET_goto_bookmark(pkt, start)
             || !PACKET_get_bytes(pkt, &bytes, 4)
-            ||  bytes[0] != 1 || bytes[1] != 2
-            ||  bytes[2] != 3 || bytes[3] != 4
+            ||  bytes[0] != 2 || bytes[1] != 4
+            ||  bytes[2] != 6 || bytes[3] != 8
             ||  PACKET_remaining(pkt) != BUF_LEN -4
             || !PACKET_forward(pkt, BUF_LEN - 8)
             || !PACKET_get_bytes(pkt, &bytes, 4)
-            ||  bytes[0] != 0xfc || bytes[1] != 0xfd
-            ||  bytes[2] != 0xfe || bytes[3] != 0xff
+            ||  bytes[0] != 0xf8 || bytes[1] != 0xfa
+            ||  bytes[2] != 0xfc || bytes[3] != 0xfe
             ||  PACKET_remaining(pkt)) {
         fprintf(stderr, "test_PACKET_get_bytes() failed\n");
         return 0;
@@ -215,13 +216,13 @@ static int test_PACKET_copy_bytes(PACKET *pkt, size_t start)
 
     if (       !PACKET_goto_bookmark(pkt, start)
             || !PACKET_copy_bytes(pkt, bytes, 4)
-            ||  bytes[0] != 1 || bytes[1] != 2
-            ||  bytes[2] != 3 || bytes[3] != 4
+            ||  bytes[0] != 2 || bytes[1] != 4
+            ||  bytes[2] != 6 || bytes[3] != 8
             ||  PACKET_remaining(pkt) != BUF_LEN - 4
             || !PACKET_forward(pkt, BUF_LEN - 8)
             || !PACKET_copy_bytes(pkt, bytes, 4)
-            ||  bytes[0] != 0xfc || bytes[1] != 0xfd
-            ||  bytes[2] != 0xfe || bytes[3] != 0xff
+            ||  bytes[0] != 0xf8 || bytes[1] != 0xfa
+            ||  bytes[2] != 0xfc || bytes[3] != 0xfe
             ||  PACKET_remaining(pkt)) {
         fprintf(stderr, "test_PACKET_copy_bytes() failed\n");
         return 0;
@@ -239,16 +240,16 @@ static int test_PACKET_move_funcs(PACKET *pkt, size_t start)
             ||  PACKET_back(pkt, 1)
             || !PACKET_forward(pkt, 1)
             || !PACKET_get_bytes(pkt, &byte, 1)
-            ||  byte[0] != 2
+            ||  byte[0] != 4
             || !PACKET_get_bookmark(pkt, &bm)
             || !PACKET_forward(pkt, BUF_LEN - 2)
             ||  PACKET_forward(pkt, 1)
             || !PACKET_back(pkt, 1)
             || !PACKET_get_bytes(pkt, &byte, 1)
-            ||  byte[0] != 0xff
+            ||  byte[0] != 0xfe
             || !PACKET_goto_bookmark(pkt, bm)
             || !PACKET_get_bytes(pkt, &byte, 1)
-            ||  byte[0] != 3) {
+            ||  byte[0] != 6) {
         fprintf(stderr, "test_PACKET_move_funcs() failed\n");
         return 0;
     }
@@ -289,7 +290,7 @@ int main(int argc, char **argv)
     PACKET pkt;
 
     for (i=1; i<=BUF_LEN; i++) {
-        buf[i-1] = i;
+        buf[i-1] = (i * 2) & 0xff;
     }
     i = 0;
 


More information about the openssl-commits mailing list