> From dcd0803f05a6c5e933da668d5ff48c7d248d9507 Mon Sep 17 00:00:00 2001 > From: tstack <tst...@tstack-dev2.(none)>
Oops, looks like the author git configuration option hasn't been configured. Also, please sign off this patch by adding your name/email with a Signed-off-by: line. (See http://www.kernel.org/doc/Documentation/SubmittingPatches Section 12 "Sign your work" for details.) > Date: Tue, 22 Jun 2010 10:22:29 -0700 > Subject: [PATCH] Fix tls handshake handling in gPXE > > The handshake record in TLS can contain multiple messages, however, > gPXE was only expecting one message. This seems to work fine with > openssl, but the java ssl code seems to send multiple message blocks > in a single record. So, we need to iterate through them. > > For reference: http://en.wikipedia.org/wiki/Transport_Layer_Security > --- > src/include/gpxe/tls.h | 7 ++++ > src/net/tls.c | 90 ++++++++++++++++++++++++++--------------------- > 2 files changed, 57 insertions(+), 40 deletions(-) > > diff --git a/src/include/gpxe/tls.h b/src/include/gpxe/tls.h > index e2da046..5126bea 100644 > --- a/src/include/gpxe/tls.h > +++ b/src/include/gpxe/tls.h > @@ -52,6 +52,13 @@ struct tls_header { > /** Application data content type */ > #define TLS_TYPE_DATA 23 > > +/** Handshake message type */ > +struct tls_handshake_msg { > + uint8_t type; > + uint8_t length[3]; > + uint8_t payload[0]; > +} __attribute__ (( packed )); > + > /* Handshake message types */ > #define TLS_HELLO_REQUEST 0 > #define TLS_CLIENT_HELLO 1 > diff --git a/src/net/tls.c b/src/net/tls.c > index a5b126e..ecc863f 100644 > --- a/src/net/tls.c > +++ b/src/net/tls.c > @@ -964,49 +964,59 @@ static int tls_new_finished ( struct tls_session *tls, > */ > static int tls_new_handshake ( struct tls_session *tls, > void *data, size_t len ) { > - struct { > - uint8_t type; > - uint8_t length[3]; > - uint8_t payload[0]; > - } __attribute__ (( packed )) *handshake = data; > - void *payload = &handshake->payload; > - size_t payload_len = tls_uint24 ( handshake->length ); > - void *end = ( payload + payload_len ); > - int rc; > + void *data_end = ( data + len ); > + int rc = 0; > + > + while ( data < data_end ) { > + struct tls_handshake_msg *handshake = data; > + void *payload = &handshake->payload; > + size_t payload_len = tls_uint24 ( handshake->length ); > + > + DBGC ( tls, "TLS %p received Handshake %d %p %d\n", > + tls, > + handshake->type, > + payload, > + payload_len ); payload_len is size_t so its format specifier needs to be "%zu" (size_t unsigned). A similar issue recently came up and it results in a warning when building in 64-bit mode. At this point a sanity check is needed to ensure that payload + payload_len (user input) is at most data_end. Otherwise the functions called below would try to use bogus/malicious payload_len values. > + > + switch ( handshake->type ) { > + case TLS_SERVER_HELLO: > + rc = tls_new_server_hello ( tls, payload, payload_len ); > + break; > + case TLS_CERTIFICATE: > + rc = tls_new_certificate ( tls, payload, payload_len ); > + break; > + case TLS_SERVER_HELLO_DONE: > + rc = tls_new_server_hello_done ( tls, payload, > payload_len ); > + break; > + case TLS_FINISHED: > + rc = tls_new_finished ( tls, payload, payload_len ); > + break; > + default: > + DBGC ( tls, "TLS %p ignoring handshake type %d\n", > + tls, handshake->type ); > + rc = 0; > + break; > + } > + > + /* Add to handshake digest (except for Hello Requests, which > + * are explicitly excluded). > + */ > + if ( handshake->type != TLS_HELLO_REQUEST ) > + tls_add_handshake ( tls, data, > + sizeof ( *handshake ) + > + payload_len ); > > - /* Sanity check */ > - if ( end != ( data + len ) ) { > - DBGC ( tls, "TLS %p received overlength Handshake\n", tls ); > - DBGC_HD ( tls, data, len ); > - return -EINVAL; > + data += sizeof ( *handshake ) + payload_len; > } > > - switch ( handshake->type ) { > - case TLS_SERVER_HELLO: > - rc = tls_new_server_hello ( tls, payload, payload_len ); > - break; > - case TLS_CERTIFICATE: > - rc = tls_new_certificate ( tls, payload, payload_len ); > - break; > - case TLS_SERVER_HELLO_DONE: > - rc = tls_new_server_hello_done ( tls, payload, payload_len ); > - break; > - case TLS_FINISHED: > - rc = tls_new_finished ( tls, payload, payload_len ); > - break; > - default: > - DBGC ( tls, "TLS %p ignoring handshake type %d\n", > - tls, handshake->type ); > - rc = 0; > - break; > + if ( data != data_end ) { > + DBGC ( tls, "TLS %p received overlength Handshake %p %p\n", > + tls, > + data, > + data_end ); > + DBGC_HD ( tls, data, len ); > } This check is no longer needed here (see comment above). > - > - /* Add to handshake digest (except for Hello Requests, which > - * are explicitly excluded). > - */ > - if ( handshake->type != TLS_HELLO_REQUEST ) > - tls_add_handshake ( tls, data, len ); > - > + > return rc; > } > > -- > 1.6.3.3 > Stefan _______________________________________________ gPXE-devel mailing list [email protected] http://etherboot.org/mailman/listinfo/gpxe-devel
