diff options
author | David Benjamin <davidben@chromium.org> | 2015-02-25 07:45:43 +0300 |
---|---|---|
committer | Adam Langley <agl@google.com> | 2015-02-26 00:23:48 +0300 |
commit | bcb2d91e1002985a26623fb4a6b4f896db5ae6c2 (patch) | |
tree | 33f5a36a334709e0242292a8440e8fdce9603ee4 | |
parent | 6f5c0f4471469f290c91af9b35c6df063f041de2 (diff) |
Actually check that the message has the expected type in DTLS.
That might be a reasonable check to make, maybe.
DTLS handshake message reading has a ton of other bugs and needs a complete
rewrite. But let's fix this and get a test in now.
Change-Id: I4981fc302feb9125908bb6161ed1a18288c39e2b
Reviewed-on: https://boringssl-review.googlesource.com/3600
Reviewed-by: Adam Langley <agl@google.com>
-rw-r--r-- | ssl/d1_both.c | 16 | ||||
-rw-r--r-- | ssl/test/runner/common.go | 4 | ||||
-rw-r--r-- | ssl/test/runner/handshake_server.go | 8 | ||||
-rw-r--r-- | ssl/test/runner/runner.go | 23 |
4 files changed, 44 insertions, 7 deletions
diff --git a/ssl/d1_both.c b/ssl/d1_both.c index 1390863d..16c3e084 100644 --- a/ssl/d1_both.c +++ b/ssl/d1_both.c @@ -359,10 +359,10 @@ int dtls1_do_write(SSL *s, int type) { } -/* Obtain handshake message of message type 'mt' (any if mt == -1), maximum - * acceptable body length 'max'. Read an entire handshake message. Handshake - * messages arrive in fragments. */ -long dtls1_get_message(SSL *s, int st1, int stn, int mt, long max, +/* dtls1_get_message reads a handshake message of message type |msg_type| (any + * if |msg_type| == -1), maximum acceptable body length |max|. Read an entire + * handshake message. Handshake messages arrive in fragments. */ +long dtls1_get_message(SSL *s, int st1, int stn, int msg_type, long max, int hash_message, int *ok) { int i, al; struct hm_header_st *msg_hdr; @@ -377,7 +377,7 @@ long dtls1_get_message(SSL *s, int st1, int stn, int mt, long max, * would have to have been applied to the previous call. */ assert(hash_message != SSL_GET_MESSAGE_DONT_HASH_MESSAGE); s->s3->tmp.reuse_message = 0; - if (mt >= 0 && s->s3->tmp.message_type != mt) { + if (msg_type >= 0 && s->s3->tmp.message_type != msg_type) { al = SSL_AD_UNEXPECTED_MESSAGE; OPENSSL_PUT_ERROR(SSL, dtls1_get_message, SSL_R_UNEXPECTED_MESSAGE); goto f_err; @@ -401,6 +401,12 @@ again: return i; } + if (msg_type >= 0 && msg_hdr->type != msg_type) { + al = SSL_AD_UNEXPECTED_MESSAGE; + OPENSSL_PUT_ERROR(SSL, dtls1_get_message, SSL_R_UNEXPECTED_MESSAGE); + goto f_err; + } + p = (uint8_t *)s->init_buf->data; msg_len = msg_hdr->msg_len; diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index e8df1aa9..b505c8f7 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go @@ -625,6 +625,10 @@ type ProtocolBugs struct { // SendInvalidRecordType, if true, causes a record with an invalid // content type to be sent immediately following the handshake. SendInvalidRecordType bool + + // WrongCertificateMessageType, if true, causes Certificate message to + // be sent with the wrong message type. + WrongCertificateMessageType bool } func (c *Config) serverInit() { diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 9c920d6e..50cc20bb 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go @@ -490,8 +490,12 @@ func (hs *serverHandshakeState) doFullHandshake() error { certMsg := new(certificateMsg) certMsg.certificates = hs.cert.Certificate if !config.Bugs.UnauthenticatedECDH { - hs.writeServerHash(certMsg.marshal()) - c.writeRecord(recordTypeHandshake, certMsg.marshal()) + certMsgBytes := certMsg.marshal() + if config.Bugs.WrongCertificateMessageType { + certMsgBytes[0] += 42 + } + hs.writeServerHash(certMsgBytes) + c.writeRecord(recordTypeHandshake, certMsgBytes) } } diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 40798634..68573586 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -788,6 +788,29 @@ var testCases = []testCase{ expectedError: ":CONNECTION_REJECTED:", expectedLocalError: "remote error: access denied", }, + { + name: "WrongMessageType", + config: Config{ + Bugs: ProtocolBugs{ + WrongCertificateMessageType: true, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }, + { + protocol: dtls, + name: "WrongMessageType-DTLS", + config: Config{ + Bugs: ProtocolBugs{ + WrongCertificateMessageType: true, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_MESSAGE:", + expectedLocalError: "remote error: unexpected message", + }, } func doExchange(test *testCase, config *Config, conn net.Conn, messageLen int, isResume bool) error { |