From 98066662aa15a21627ef8a3fb7318b6ee301df22 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 6 Feb 2018 13:33:00 +0100 Subject: tls: fix hash calculations if client cert is requested and sent Symptoms: connecting to openssl s_server -cert vsftpd.pem -port 990 -debug -cipher AES128-SHA works, but with "-verify 1" option added it does not. function old new delta tls_xread_record 474 499 +25 tls_handshake 1582 1607 +25 bad_record_die 98 110 +12 tls_run_copy_loop 282 293 +11 tls_xread_handshake_block 58 51 -7 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 4/1 up/down: 73/-7) Total: 66 bytes Signed-off-by: Denys Vlasenko --- networking/tls.c | 94 ++++++++++++++++++++++++++++++++------------------------ 1 file changed, 54 insertions(+), 40 deletions(-) diff --git a/networking/tls.c b/networking/tls.c index fd3cb0d..7936afc 100644 --- a/networking/tls.c +++ b/networking/tls.c @@ -84,23 +84,23 @@ # define dbg_der(...) ((void)0) #endif -#define RECORD_TYPE_CHANGE_CIPHER_SPEC 20 -#define RECORD_TYPE_ALERT 21 -#define RECORD_TYPE_HANDSHAKE 22 -#define RECORD_TYPE_APPLICATION_DATA 23 - -#define HANDSHAKE_HELLO_REQUEST 0 -#define HANDSHAKE_CLIENT_HELLO 1 -#define HANDSHAKE_SERVER_HELLO 2 -#define HANDSHAKE_HELLO_VERIFY_REQUEST 3 -#define HANDSHAKE_NEW_SESSION_TICKET 4 -#define HANDSHAKE_CERTIFICATE 11 -#define HANDSHAKE_SERVER_KEY_EXCHANGE 12 -#define HANDSHAKE_CERTIFICATE_REQUEST 13 -#define HANDSHAKE_SERVER_HELLO_DONE 14 -#define HANDSHAKE_CERTIFICATE_VERIFY 15 -#define HANDSHAKE_CLIENT_KEY_EXCHANGE 16 -#define HANDSHAKE_FINISHED 20 +#define RECORD_TYPE_CHANGE_CIPHER_SPEC 20 /* 0x14 */ +#define RECORD_TYPE_ALERT 21 /* 0x15 */ +#define RECORD_TYPE_HANDSHAKE 22 /* 0x16 */ +#define RECORD_TYPE_APPLICATION_DATA 23 /* 0x17 */ + +#define HANDSHAKE_HELLO_REQUEST 0 /* 0x00 */ +#define HANDSHAKE_CLIENT_HELLO 1 /* 0x01 */ +#define HANDSHAKE_SERVER_HELLO 2 /* 0x02 */ +#define HANDSHAKE_HELLO_VERIFY_REQUEST 3 /* 0x03 */ +#define HANDSHAKE_NEW_SESSION_TICKET 4 /* 0x04 */ +#define HANDSHAKE_CERTIFICATE 11 /* 0x0b */ +#define HANDSHAKE_SERVER_KEY_EXCHANGE 12 /* 0x0c */ +#define HANDSHAKE_CERTIFICATE_REQUEST 13 /* 0x0d */ +#define HANDSHAKE_SERVER_HELLO_DONE 14 /* 0x0e */ +#define HANDSHAKE_CERTIFICATE_VERIFY 15 /* 0x0f */ +#define HANDSHAKE_CLIENT_KEY_EXCHANGE 16 /* 0x10 */ +#define HANDSHAKE_FINISHED 20 /* 0x14 */ #define SSL_NULL_WITH_NULL_NULL 0x0000 #define SSL_RSA_WITH_NULL_MD5 0x0001 @@ -512,10 +512,12 @@ static void bad_record_die(tls_state_t *tls, const char *expected, int len) bb_error_msg("got bad TLS record (len:%d) while expecting %s", len, expected); if (len > 0) { uint8_t *p = tls->inbuf; - while (len > 0) { + if (len > 99) + len = 99; /* don't flood, a few lines should be enough */ + do { fprintf(stderr, " %02x", *p++); len--; - } + } while (len != 0); fputc('\n', stderr); } xfunc_die(); @@ -671,9 +673,11 @@ static void xwrite_encrypted(tls_state_t *tls, unsigned size, unsigned type) // AES_128_CBC Block 16 16 16 // AES_256_CBC Block 32 16 16 - /* Fill IV and padding in outbuf */ tls_get_random(buf - AES_BLOCKSIZE, AES_BLOCKSIZE); /* IV */ - dbg("before crypt: 5 hdr + %u data + %u hash bytes\n", size, tls->MAC_size); + dbg("before crypt: 5 hdr + %u data + %u hash bytes\n", + size - tls->MAC_size, tls->MAC_size); + + /* Fill IV and padding in outbuf */ // RFC is talking nonsense: // "Padding that is added to force the length of the plaintext to be // an integral multiple of the block cipher's block length." @@ -773,7 +777,7 @@ static const char *alert_text(int code) return itoa(code); } -static int tls_xread_record(tls_state_t *tls) +static int tls_xread_record(tls_state_t *tls, const char *expected) { struct record_hdr *xhdr; int sz; @@ -796,13 +800,16 @@ static int tls_xread_record(tls_state_t *tls) if (total >= RECHDR_LEN && target == MAX_INBUF) { xhdr = (void*)tls->inbuf; target = RECHDR_LEN + (0x100 * xhdr->len16_hi + xhdr->len16_lo); - if (target > MAX_INBUF) { - /* malformed input (too long): yell and die */ - tls->buffered_size = 0; - tls->ofs_to_buffered = total; - tls_error_die(tls); + + if (target > MAX_INBUF /* malformed input (too long) */ + || xhdr->proto_maj != TLS_MAJ + || xhdr->proto_min != TLS_MIN + ) { + sz = total < target ? total : target; + if (sz > 24) + sz = 24; /* don't flood */ + bad_record_die(tls, expected, sz); } - /* can also check type/proto_maj/proto_min here */ dbg("xhdr type:%d ver:%d.%d len:%d\n", xhdr->type, xhdr->proto_maj, xhdr->proto_min, 0x100 * xhdr->len16_hi + xhdr->len16_lo @@ -1137,13 +1144,11 @@ static void find_key_in_der_cert(tls_state_t *tls, uint8_t *der, int len) static int tls_xread_handshake_block(tls_state_t *tls, int min_len) { struct record_hdr *xhdr; - int len = tls_xread_record(tls); + int len = tls_xread_record(tls, "handshake record"); xhdr = (void*)tls->inbuf; if (len < min_len || xhdr->type != RECORD_TYPE_HANDSHAKE - || xhdr->proto_maj != TLS_MAJ - || xhdr->proto_min != TLS_MIN ) { bad_record_die(tls, "handshake record", len); } @@ -1195,7 +1200,9 @@ static void send_client_hello_and_alloc_hsd(tls_state_t *tls, const char *sni) // 0023 0000 - session_ticket // 000a 0008 0006001700180019 - supported_groups // 000b 0002 0100 - ec_point_formats -// 000d 0016 00140401040305010503060106030301030302010203 - signature_algorithms +// 000d 0016 0014 0401 0403 0501 0503 0601 0603 0301 0303 0201 0203 - signature_algorithms +// wolfssl library sends this option, RFC 7627 (closes a security weakness, some servers may require it. TODO?): +// 0017 0000 - extended master secret }; struct client_hello *record; int len; @@ -1354,7 +1361,7 @@ static void get_server_cert(tls_state_t *tls) xhdr = (void*)tls->inbuf; certbuf = (void*)(xhdr + 1); if (certbuf[0] != HANDSHAKE_CERTIFICATE) - tls_error_die(tls); + bad_record_die(tls, "certificate", len); dbg("<< CERTIFICATE\n"); // 4392 bytes: // 0b 00|11|24 00|11|21 00|05|b0 30|82|05|ac|30|82|04|94|a0|03|02|01|02|02|11|00|9f|85|bf|66|4b|0c|dd|af|ca|50|86|79|50|1b|2b|e4|30|0d... @@ -1611,6 +1618,7 @@ void FAST_FUNC tls_handshake(tls_state_t *tls, const char *sni) // <------- Finished // Application Data <------> Application Data int len; + int got_cert_req; send_client_hello_and_alloc_hsd(tls, sni); get_server_hello(tls); @@ -1638,7 +1646,8 @@ void FAST_FUNC tls_handshake(tls_state_t *tls, const char *sni) len = tls_xread_handshake_block(tls, 4); } - if (tls->inbuf[RECHDR_LEN] == HANDSHAKE_CERTIFICATE_REQUEST) { + got_cert_req = (tls->inbuf[RECHDR_LEN] == HANDSHAKE_CERTIFICATE_REQUEST); + if (got_cert_req) { dbg("<< CERTIFICATE_REQUEST\n"); // RFC 5246: "If no suitable certificate is available, // the client MUST send a certificate message containing no @@ -1647,7 +1656,9 @@ void FAST_FUNC tls_handshake(tls_state_t *tls, const char *sni) // Client certificates are sent using the Certificate structure // defined in Section 7.4.2." // (i.e. the same format as server certs) - send_empty_client_cert(tls); + + /*send_empty_client_cert(tls); - WRONG (breaks handshake hash calc) */ + /* need to hash _all_ server replies first, up to ServerHelloDone */ len = tls_xread_handshake_block(tls, 4); } @@ -1657,6 +1668,9 @@ void FAST_FUNC tls_handshake(tls_state_t *tls, const char *sni) // 0e 000000 (len:0) dbg("<< SERVER_HELLO_DONE\n"); + if (got_cert_req) + send_empty_client_cert(tls); + send_client_key_exchange(tls); send_change_cipher_spec(tls); @@ -1667,7 +1681,7 @@ void FAST_FUNC tls_handshake(tls_state_t *tls, const char *sni) send_client_finished(tls); /* Get CHANGE_CIPHER_SPEC */ - len = tls_xread_record(tls); + len = tls_xread_record(tls, "switch to encrypted traffic"); if (len != 1 || memcmp(tls->inbuf, rec_CHANGE_CIPHER_SPEC, 6) != 0) bad_record_die(tls, "switch to encrypted traffic", len); dbg("<< CHANGE_CIPHER_SPEC\n"); @@ -1685,9 +1699,9 @@ void FAST_FUNC tls_handshake(tls_state_t *tls, const char *sni) } /* Get (encrypted) FINISHED from the server */ - len = tls_xread_record(tls); + len = tls_xread_record(tls, "'server finished'"); if (len < 4 || tls->inbuf[RECHDR_LEN] != HANDSHAKE_FINISHED) - tls_error_die(tls); + bad_record_die(tls, "'server finished'", len); dbg("<< FINISHED\n"); /* application data can be sent/received */ @@ -1763,7 +1777,7 @@ void FAST_FUNC tls_run_copy_loop(tls_state_t *tls) if (pfds[1].revents) { dbg("NETWORK HAS DATA\n"); read_record: - nread = tls_xread_record(tls); + nread = tls_xread_record(tls, "encrypted data"); if (nread < 1) { /* TLS protocol has no real concept of one-sided shutdowns: * if we get "TLS EOF" from the peer, writes will fail too @@ -1775,7 +1789,7 @@ void FAST_FUNC tls_run_copy_loop(tls_state_t *tls) break; } if (tls->inbuf[0] != RECORD_TYPE_APPLICATION_DATA) - bb_error_msg_and_die("unexpected record type %d", tls->inbuf[0]); + bad_record_die(tls, "encrypted data", nread); xwrite(STDOUT_FILENO, tls->inbuf + RECHDR_LEN, nread); /* We may already have a complete next record buffered, * can process it without network reads (and possible blocking) -- cgit v1.1