[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Fri Oct 19 13:33:15 UTC 2018
The branch master has been updated
via 079ef6bd534d2f708d8013cfcd8ea0d2f600c788 (commit)
via 2fc4c77c3f06443f4c476f6f58d83e5e108d1dce (commit)
from edcd29efd32c51f298ad5ab438e2d4cc5411e9a9 (commit)
- Log -----------------------------------------------------------------
commit 079ef6bd534d2f708d8013cfcd8ea0d2f600c788
Author: Matt Caswell <matt at openssl.org>
Date: Tue Oct 9 10:22:06 2018 +0100
Buffer a ClientHello with a cookie received via DTLSv1_listen
Previously when a ClientHello arrives with a valid cookie using
DTLSv1_listen() we only "peeked" at the message and left it on the
underlying fd. This works fine for single threaded applications but for
multi-threaded apps this does not work since the fd is typically reused for
the server thread, while a new fd is created and connected for the client.
By "peeking" we leave the message on the server fd, and consequently we
think we've received another valid ClientHello and so we create yet another
fd for the client, and so on until we run out of fds.
In this new approach we remove the ClientHello and buffer it in the SSL
object.
Fixes #6934
Reviewed-by: Ben Kaduk <kaduk at mit.edu>
(Merged from https://github.com/openssl/openssl/pull/7375)
commit 2fc4c77c3f06443f4c476f6f58d83e5e108d1dce
Author: Matt Caswell <matt at openssl.org>
Date: Mon Oct 8 15:46:51 2018 +0100
Use the read and write buffers in DTLSv1_listen()
Rather than using init_buf we use the record layer read and write buffers
in DTLSv1_listen(). These seem more appropriate anyway and will help with
the next commit.
Reviewed-by: Ben Kaduk <kaduk at mit.edu>
(Merged from https://github.com/openssl/openssl/pull/7375)
-----------------------------------------------------------------------
Summary of changes:
ssl/d1_lib.c | 91 +++++++++++++++++++-----------------------------
ssl/record/record.h | 4 +++
ssl/record/record_locl.h | 2 --
ssl/record/ssl3_record.c | 25 +++++++++++++
4 files changed, 65 insertions(+), 57 deletions(-)
diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c
index f808512..7a7a4be 100644
--- a/ssl/d1_lib.c
+++ b/ssl/d1_lib.c
@@ -445,15 +445,14 @@ static void get_current_time(struct timeval *t)
#ifndef OPENSSL_NO_SOCK
int DTLSv1_listen(SSL *s, BIO_ADDR *client)
{
- int next, n, ret = 0, clearpkt = 0;
+ int next, n, ret = 0;
unsigned char cookie[DTLS1_COOKIE_LENGTH];
unsigned char seq[SEQ_NUM_SIZE];
const unsigned char *data;
- unsigned char *buf;
- size_t fragoff, fraglen, msglen;
+ unsigned char *buf, *wbuf;
+ size_t fragoff, fraglen, msglen, reclen, align = 0;
unsigned int rectype, versmajor, msgseq, msgtype, clientvers, cookielen;
BIO *rbio, *wbio;
- BUF_MEM *bufm;
BIO_ADDR *tmpclient = NULL;
PACKET pkt, msgpkt, msgpayload, session, cookiepkt;
@@ -477,13 +476,6 @@ int DTLSv1_listen(SSL *s, BIO_ADDR *client)
}
/*
- * We only peek at incoming ClientHello's until we're sure we are going to
- * to respond with a HelloVerifyRequest. If its a ClientHello with a valid
- * cookie then we leave it in the BIO for accept to handle.
- */
- BIO_ctrl(SSL_get_rbio(s), BIO_CTRL_DGRAM_SET_PEEK_MODE, 1, NULL);
-
- /*
* Note: This check deliberately excludes DTLS1_BAD_VER because that version
* requires the MAC to be calculated *including* the first ClientHello
* (without the cookie). Since DTLSv1_listen is stateless that cannot be
@@ -495,35 +487,32 @@ int DTLSv1_listen(SSL *s, BIO_ADDR *client)
return -1;
}
- if (s->init_buf == NULL) {
- if ((bufm = BUF_MEM_new()) == NULL) {
- SSLerr(SSL_F_DTLSV1_LISTEN, ERR_R_MALLOC_FAILURE);
- return -1;
- }
-
- if (!BUF_MEM_grow(bufm, SSL3_RT_MAX_PLAIN_LENGTH)) {
- BUF_MEM_free(bufm);
- SSLerr(SSL_F_DTLSV1_LISTEN, ERR_R_MALLOC_FAILURE);
- return -1;
- }
- s->init_buf = bufm;
+ if (!ssl3_setup_buffers(s)) {
+ /* SSLerr already called */
+ return -1;
}
- buf = (unsigned char *)s->init_buf->data;
+ buf = RECORD_LAYER_get_rbuf(&s->rlayer)->buf;
+ wbuf = RECORD_LAYER_get_wbuf(&s->rlayer)[0].buf;
+#if defined(SSL3_ALIGN_PAYLOAD)
+# if SSL3_ALIGN_PAYLOAD != 0
+ /*
+ * Using SSL3_RT_HEADER_LENGTH here instead of DTLS1_RT_HEADER_LENGTH for
+ * consistency with ssl3_read_n. In practice it should make no difference
+ * for sensible values of SSL3_ALIGN_PAYLOAD because the difference between
+ * SSL3_RT_HEADER_LENGTH and DTLS1_RT_HEADER_LENGTH is exactly 8
+ */
+ align = (size_t)buf + SSL3_RT_HEADER_LENGTH;
+ align = SSL3_ALIGN_PAYLOAD - 1 - ((align - 1) % SSL3_ALIGN_PAYLOAD);
+# endif
+#endif
+ buf += align;
do {
/* Get a packet */
clear_sys_error();
- /*
- * Technically a ClientHello could be SSL3_RT_MAX_PLAIN_LENGTH
- * + DTLS1_RT_HEADER_LENGTH bytes long. Normally init_buf does not store
- * the record header as well, but we do here. We've set up init_buf to
- * be the standard size for simplicity. In practice we shouldn't ever
- * receive a ClientHello as long as this. If we do it will get dropped
- * in the record length check below.
- */
- n = BIO_read(rbio, buf, SSL3_RT_MAX_PLAIN_LENGTH);
-
+ n = BIO_read(rbio, buf, SSL3_RT_MAX_PLAIN_LENGTH
+ + DTLS1_RT_HEADER_LENGTH);
if (n <= 0) {
if (BIO_should_retry(rbio)) {
/* Non-blocking IO */
@@ -532,9 +521,6 @@ int DTLSv1_listen(SSL *s, BIO_ADDR *client)
return -1;
}
- /* If we hit any problems we need to clear this packet from the BIO */
- clearpkt = 1;
-
if (!PACKET_buf_init(&pkt, buf, n)) {
SSLerr(SSL_F_DTLSV1_LISTEN, ERR_R_INTERNAL_ERROR);
return -1;
@@ -587,6 +573,7 @@ int DTLSv1_listen(SSL *s, BIO_ADDR *client)
SSLerr(SSL_F_DTLSV1_LISTEN, SSL_R_LENGTH_MISMATCH);
goto end;
}
+ reclen = PACKET_remaining(&msgpkt);
/*
* We allow data remaining at the end of the packet because there could
* be a second record (but we ignore it)
@@ -706,14 +693,6 @@ int DTLSv1_listen(SSL *s, BIO_ADDR *client)
* to resend, we just drop it.
*/
- /*
- * Dump the read packet, we don't need it any more. Ignore return
- * value
- */
- BIO_ctrl(SSL_get_rbio(s), BIO_CTRL_DGRAM_SET_PEEK_MODE, 0, NULL);
- BIO_read(rbio, buf, SSL3_RT_MAX_PLAIN_LENGTH);
- BIO_ctrl(SSL_get_rbio(s), BIO_CTRL_DGRAM_SET_PEEK_MODE, 1, NULL);
-
/* Generate the cookie */
if (s->ctx->app_gen_cookie_cb == NULL ||
s->ctx->app_gen_cookie_cb(s, cookie, &cookielen) == 0 ||
@@ -732,7 +711,11 @@ int DTLSv1_listen(SSL *s, BIO_ADDR *client)
: s->version;
/* Construct the record and message headers */
- if (!WPACKET_init(&wpkt, s->init_buf)
+ if (!WPACKET_init_static_len(&wpkt,
+ wbuf,
+ ssl_get_max_send_fragment(s)
+ + DTLS1_RT_HEADER_LENGTH,
+ 0)
|| !WPACKET_put_bytes_u8(&wpkt, SSL3_RT_HANDSHAKE)
|| !WPACKET_put_bytes_u16(&wpkt, version)
/*
@@ -790,8 +773,8 @@ int DTLSv1_listen(SSL *s, BIO_ADDR *client)
* plus one byte for the message content type. The source is the
* last 3 bytes of the message header
*/
- memcpy(&buf[DTLS1_RT_HEADER_LENGTH + 1],
- &buf[DTLS1_RT_HEADER_LENGTH + DTLS1_HM_HEADER_LENGTH - 3],
+ memcpy(&wbuf[DTLS1_RT_HEADER_LENGTH + 1],
+ &wbuf[DTLS1_RT_HEADER_LENGTH + DTLS1_HM_HEADER_LENGTH - 3],
3);
if (s->msg_callback)
@@ -815,7 +798,7 @@ int DTLSv1_listen(SSL *s, BIO_ADDR *client)
tmpclient = NULL;
/* TODO(size_t): convert this call */
- if (BIO_write(wbio, buf, wreclen) < (int)wreclen) {
+ if (BIO_write(wbio, wbuf, wreclen) < (int)wreclen) {
if (BIO_should_retry(wbio)) {
/*
* Non-blocking IO...but we're stateless, so we're just
@@ -865,15 +848,13 @@ int DTLSv1_listen(SSL *s, BIO_ADDR *client)
if (BIO_dgram_get_peer(rbio, client) <= 0)
BIO_ADDR_clear(client);
+ /* Buffer the record in the processed_rcds queue */
+ if (!dtls_buffer_listen_record(s, reclen, seq, align))
+ return -1;
+
ret = 1;
- clearpkt = 0;
end:
BIO_ADDR_free(tmpclient);
- BIO_ctrl(SSL_get_rbio(s), BIO_CTRL_DGRAM_SET_PEEK_MODE, 0, NULL);
- if (clearpkt) {
- /* Dump this packet. Ignore return value */
- BIO_read(rbio, buf, SSL3_RT_MAX_PLAIN_LENGTH);
- }
return ret;
}
#endif
diff --git a/ssl/record/record.h b/ssl/record/record.h
index 32db821..76a2b81 100644
--- a/ssl/record/record.h
+++ b/ssl/record/record.h
@@ -188,6 +188,8 @@ typedef struct record_layer_st {
((rl)->d->processed_rcds)
#define DTLS_RECORD_LAYER_get_unprocessed_rcds(rl) \
((rl)->d->unprocessed_rcds)
+#define RECORD_LAYER_get_rbuf(rl) (&(rl)->rbuf)
+#define RECORD_LAYER_get_wbuf(rl) ((rl)->wbuf)
void RECORD_LAYER_init(RECORD_LAYER *rl, SSL *s);
void RECORD_LAYER_clear(RECORD_LAYER *rl);
@@ -230,3 +232,5 @@ __owur int dtls1_write_bytes(SSL *s, int type, const void *buf, size_t len,
int do_dtls1_write(SSL *s, int type, const unsigned char *buf,
size_t len, int create_empty_fragment, size_t *written);
void dtls1_reset_seq_numbers(SSL *s, int rw);
+int dtls_buffer_listen_record(SSL *s, size_t len, unsigned char *seq,
+ size_t off);
diff --git a/ssl/record/record_locl.h b/ssl/record/record_locl.h
index 07fd7ab..5e8dd7f 100644
--- a/ssl/record/record_locl.h
+++ b/ssl/record/record_locl.h
@@ -18,8 +18,6 @@
/* Functions/macros provided by the RECORD_LAYER component */
-#define RECORD_LAYER_get_rbuf(rl) (&(rl)->rbuf)
-#define RECORD_LAYER_get_wbuf(rl) ((rl)->wbuf)
#define RECORD_LAYER_get_rrec(rl) ((rl)->rrec)
#define RECORD_LAYER_set_packet(rl, p) ((rl)->packet = (p))
#define RECORD_LAYER_reset_packet_length(rl) ((rl)->packet_length = 0)
diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index a616bf0..e59ac5a 100644
--- a/ssl/record/ssl3_record.c
+++ b/ssl/record/ssl3_record.c
@@ -2030,3 +2030,28 @@ int dtls1_get_record(SSL *s)
return 1;
}
+
+int dtls_buffer_listen_record(SSL *s, size_t len, unsigned char *seq, size_t off)
+{
+ SSL3_RECORD *rr;
+
+ rr = RECORD_LAYER_get_rrec(&s->rlayer);
+ memset(rr, 0, sizeof(SSL3_RECORD));
+
+ rr->length = len;
+ rr->type = SSL3_RT_HANDSHAKE;
+ memcpy(rr->seq_num, seq, sizeof(rr->seq_num));
+ rr->off = off;
+
+ s->rlayer.packet = RECORD_LAYER_get_rbuf(&s->rlayer)->buf;
+ s->rlayer.packet_length = DTLS1_RT_HEADER_LENGTH + len;
+ rr->data = s->rlayer.packet + DTLS1_RT_HEADER_LENGTH;
+
+ if (dtls1_buffer_record(s, &(s->rlayer.d->processed_rcds),
+ SSL3_RECORD_get_seq_num(s->rlayer.rrec)) <= 0) {
+ /* SSLfatal() already called */
+ return 0;
+ }
+
+ return 1;
+}
More information about the openssl-commits
mailing list