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 <vda.linux@googlemail.com>
This commit is contained in:
Denys Vlasenko
2018-02-06 13:33:00 +01:00
parent 5740e15de5
commit 98066662aa

View File

@@ -84,23 +84,23 @@
# define dbg_der(...) ((void)0) # define dbg_der(...) ((void)0)
#endif #endif
#define RECORD_TYPE_CHANGE_CIPHER_SPEC 20 #define RECORD_TYPE_CHANGE_CIPHER_SPEC 20 /* 0x14 */
#define RECORD_TYPE_ALERT 21 #define RECORD_TYPE_ALERT 21 /* 0x15 */
#define RECORD_TYPE_HANDSHAKE 22 #define RECORD_TYPE_HANDSHAKE 22 /* 0x16 */
#define RECORD_TYPE_APPLICATION_DATA 23 #define RECORD_TYPE_APPLICATION_DATA 23 /* 0x17 */
#define HANDSHAKE_HELLO_REQUEST 0 #define HANDSHAKE_HELLO_REQUEST 0 /* 0x00 */
#define HANDSHAKE_CLIENT_HELLO 1 #define HANDSHAKE_CLIENT_HELLO 1 /* 0x01 */
#define HANDSHAKE_SERVER_HELLO 2 #define HANDSHAKE_SERVER_HELLO 2 /* 0x02 */
#define HANDSHAKE_HELLO_VERIFY_REQUEST 3 #define HANDSHAKE_HELLO_VERIFY_REQUEST 3 /* 0x03 */
#define HANDSHAKE_NEW_SESSION_TICKET 4 #define HANDSHAKE_NEW_SESSION_TICKET 4 /* 0x04 */
#define HANDSHAKE_CERTIFICATE 11 #define HANDSHAKE_CERTIFICATE 11 /* 0x0b */
#define HANDSHAKE_SERVER_KEY_EXCHANGE 12 #define HANDSHAKE_SERVER_KEY_EXCHANGE 12 /* 0x0c */
#define HANDSHAKE_CERTIFICATE_REQUEST 13 #define HANDSHAKE_CERTIFICATE_REQUEST 13 /* 0x0d */
#define HANDSHAKE_SERVER_HELLO_DONE 14 #define HANDSHAKE_SERVER_HELLO_DONE 14 /* 0x0e */
#define HANDSHAKE_CERTIFICATE_VERIFY 15 #define HANDSHAKE_CERTIFICATE_VERIFY 15 /* 0x0f */
#define HANDSHAKE_CLIENT_KEY_EXCHANGE 16 #define HANDSHAKE_CLIENT_KEY_EXCHANGE 16 /* 0x10 */
#define HANDSHAKE_FINISHED 20 #define HANDSHAKE_FINISHED 20 /* 0x14 */
#define SSL_NULL_WITH_NULL_NULL 0x0000 #define SSL_NULL_WITH_NULL_NULL 0x0000
#define SSL_RSA_WITH_NULL_MD5 0x0001 #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); bb_error_msg("got bad TLS record (len:%d) while expecting %s", len, expected);
if (len > 0) { if (len > 0) {
uint8_t *p = tls->inbuf; 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++); fprintf(stderr, " %02x", *p++);
len--; len--;
} } while (len != 0);
fputc('\n', stderr); fputc('\n', stderr);
} }
xfunc_die(); 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_128_CBC Block 16 16 16
// AES_256_CBC Block 32 16 16 // AES_256_CBC Block 32 16 16
/* Fill IV and padding in outbuf */
tls_get_random(buf - AES_BLOCKSIZE, AES_BLOCKSIZE); /* IV */ 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: // RFC is talking nonsense:
// "Padding that is added to force the length of the plaintext to be // "Padding that is added to force the length of the plaintext to be
// an integral multiple of the block cipher's block length." // an integral multiple of the block cipher's block length."
@@ -773,7 +777,7 @@ static const char *alert_text(int code)
return itoa(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; struct record_hdr *xhdr;
int sz; int sz;
@@ -796,13 +800,16 @@ static int tls_xread_record(tls_state_t *tls)
if (total >= RECHDR_LEN && target == MAX_INBUF) { if (total >= RECHDR_LEN && target == MAX_INBUF) {
xhdr = (void*)tls->inbuf; xhdr = (void*)tls->inbuf;
target = RECHDR_LEN + (0x100 * xhdr->len16_hi + xhdr->len16_lo); target = RECHDR_LEN + (0x100 * xhdr->len16_hi + xhdr->len16_lo);
if (target > MAX_INBUF) {
/* malformed input (too long): yell and die */ if (target > MAX_INBUF /* malformed input (too long) */
tls->buffered_size = 0; || xhdr->proto_maj != TLS_MAJ
tls->ofs_to_buffered = total; || xhdr->proto_min != TLS_MIN
tls_error_die(tls); ) {
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", dbg("xhdr type:%d ver:%d.%d len:%d\n",
xhdr->type, xhdr->proto_maj, xhdr->proto_min, xhdr->type, xhdr->proto_maj, xhdr->proto_min,
0x100 * xhdr->len16_hi + xhdr->len16_lo 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) static int tls_xread_handshake_block(tls_state_t *tls, int min_len)
{ {
struct record_hdr *xhdr; struct record_hdr *xhdr;
int len = tls_xread_record(tls); int len = tls_xread_record(tls, "handshake record");
xhdr = (void*)tls->inbuf; xhdr = (void*)tls->inbuf;
if (len < min_len if (len < min_len
|| xhdr->type != RECORD_TYPE_HANDSHAKE || xhdr->type != RECORD_TYPE_HANDSHAKE
|| xhdr->proto_maj != TLS_MAJ
|| xhdr->proto_min != TLS_MIN
) { ) {
bad_record_die(tls, "handshake record", len); 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 // 0023 0000 - session_ticket
// 000a 0008 0006001700180019 - supported_groups // 000a 0008 0006001700180019 - supported_groups
// 000b 0002 0100 - ec_point_formats // 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; struct client_hello *record;
int len; int len;
@@ -1354,7 +1361,7 @@ static void get_server_cert(tls_state_t *tls)
xhdr = (void*)tls->inbuf; xhdr = (void*)tls->inbuf;
certbuf = (void*)(xhdr + 1); certbuf = (void*)(xhdr + 1);
if (certbuf[0] != HANDSHAKE_CERTIFICATE) if (certbuf[0] != HANDSHAKE_CERTIFICATE)
tls_error_die(tls); bad_record_die(tls, "certificate", len);
dbg("<< CERTIFICATE\n"); dbg("<< CERTIFICATE\n");
// 4392 bytes: // 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... // 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 // <------- Finished
// Application Data <------> Application Data // Application Data <------> Application Data
int len; int len;
int got_cert_req;
send_client_hello_and_alloc_hsd(tls, sni); send_client_hello_and_alloc_hsd(tls, sni);
get_server_hello(tls); 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); 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"); dbg("<< CERTIFICATE_REQUEST\n");
// RFC 5246: "If no suitable certificate is available, // RFC 5246: "If no suitable certificate is available,
// the client MUST send a certificate message containing no // 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 // Client certificates are sent using the Certificate structure
// defined in Section 7.4.2." // defined in Section 7.4.2."
// (i.e. the same format as server certs) // (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); 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) // 0e 000000 (len:0)
dbg("<< SERVER_HELLO_DONE\n"); dbg("<< SERVER_HELLO_DONE\n");
if (got_cert_req)
send_empty_client_cert(tls);
send_client_key_exchange(tls); send_client_key_exchange(tls);
send_change_cipher_spec(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); send_client_finished(tls);
/* Get CHANGE_CIPHER_SPEC */ /* 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) if (len != 1 || memcmp(tls->inbuf, rec_CHANGE_CIPHER_SPEC, 6) != 0)
bad_record_die(tls, "switch to encrypted traffic", len); bad_record_die(tls, "switch to encrypted traffic", len);
dbg("<< CHANGE_CIPHER_SPEC\n"); 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 */ /* 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) if (len < 4 || tls->inbuf[RECHDR_LEN] != HANDSHAKE_FINISHED)
tls_error_die(tls); bad_record_die(tls, "'server finished'", len);
dbg("<< FINISHED\n"); dbg("<< FINISHED\n");
/* application data can be sent/received */ /* 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) { if (pfds[1].revents) {
dbg("NETWORK HAS DATA\n"); dbg("NETWORK HAS DATA\n");
read_record: read_record:
nread = tls_xread_record(tls); nread = tls_xread_record(tls, "encrypted data");
if (nread < 1) { if (nread < 1) {
/* TLS protocol has no real concept of one-sided shutdowns: /* TLS protocol has no real concept of one-sided shutdowns:
* if we get "TLS EOF" from the peer, writes will fail too * 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; break;
} }
if (tls->inbuf[0] != RECORD_TYPE_APPLICATION_DATA) 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); xwrite(STDOUT_FILENO, tls->inbuf + RECHDR_LEN, nread);
/* We may already have a complete next record buffered, /* We may already have a complete next record buffered,
* can process it without network reads (and possible blocking) * can process it without network reads (and possible blocking)