kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Remove buffered_app_data as well.

Browse Source 
This conceivably has a use, but NSS doesn't do this buffer either and it still
suffers from the same problems as the other uses of record_pqueue. This removes
the last use of record_pqueue. It also opens the door to removing pqueue
altogether as it isn't the right data structure for either of the remaining
uses either. (It's not clear it was right for record_pqueue either, but I don't
feel like digging into this code.)

Change-Id: If8a43e7332b3cd11a78a516f3e8ebf828052316f
Reviewed-on: https://boringssl-review.googlesource.com/4239
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2015-04-05 04:17:25 -04:00 committed by Adam Langley
parent 5933723b7b
commit 4417d055e2
4 changed files with 12 additions and 148 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
include/openssl/dtls1.h
View File

@ -105,11 +105,6 @@ struct hm_header_st {
uint16_t epoch;
};
typedef struct record_pqueue_st {
uint16_t epoch;
pqueue q;
} record_pqueue;
/* TODO(davidben): This structure is used for both incoming messages and
* outgoing messages. |fragment| and |reassembly| are only used in the former
* and should be moved elsewhere. */
@ -157,10 +152,6 @@ typedef struct dtls1_state_st {
* TODO(davidben): This data structure may as well be a STACK_OF(T). */
pqueue sent_messages;
/* Buffered application records. Only for records between CCS and Finished to
* prevent either protocol violation or unnecessary message loss. */
record_pqueue buffered_app_data;
unsigned int mtu; /* max DTLS packet size */
struct hm_header_st w_msg_hdr;
@ -180,13 +171,6 @@ typedef struct dtls1_state_st {
unsigned int change_cipher_spec_ok;
} DTLS1_STATE;
typedef struct dtls1_record_data_st {
uint8_t *packet;
unsigned int packet_length;
SSL3_BUFFER rbuf;
SSL3_RECORD rrec;
} DTLS1_RECORD_DATA;
#ifdef __cplusplus
} /* extern C */

18
ssl/d1_lib.c
View File

@ -98,19 +98,14 @@ int dtls1_new(SSL *s) {
d1->buffered_messages = pqueue_new();
d1->sent_messages = pqueue_new();
d1->buffered_app_data.q = pqueue_new();
if (!d1->buffered_messages || !d1->sent_messages ||
!d1->buffered_app_data.q) {
if (!d1->buffered_messages || !d1->sent_messages) {
if (d1->buffered_messages) {
pqueue_free(d1->buffered_messages);
}
if (d1->sent_messages) {
pqueue_free(d1->sent_messages);
}
if (d1->buffered_app_data.q) {
pqueue_free(d1->buffered_app_data.q);
}
OPENSSL_free(d1);
ssl3_free(s);
return 0;
@ -130,7 +125,6 @@ int dtls1_new(SSL *s) {
static void dtls1_clear_queues(SSL *s) {
pitem *item = NULL;
hm_fragment *frag = NULL;
DTLS1_RECORD_DATA *rdata;
while ((item = pqueue_pop(s->d1->buffered_messages)) != NULL) {
frag = (hm_fragment *)item->data;
@ -143,15 +137,6 @@ static void dtls1_clear_queues(SSL *s) {
dtls1_hm_fragment_free(frag);
pitem_free(item);
}
while ((item = pqueue_pop(s->d1->buffered_app_data.q)) != NULL) {
rdata = (DTLS1_RECORD_DATA *)item->data;
if (rdata->rbuf.buf) {
OPENSSL_free(rdata->rbuf.buf);
}
OPENSSL_free(item->data);
pitem_free(item);
}
}
void dtls1_free(SSL *s) {
@ -165,7 +150,6 @@ void dtls1_free(SSL *s) {
pqueue_free(s->d1->buffered_messages);
pqueue_free(s->d1->sent_messages);
pqueue_free(s->d1->buffered_app_data.q);
OPENSSL_free(s->d1);
s->d1 = NULL;

103
ssl/d1_pkt.c
View File

@ -183,90 +183,10 @@ static int satsub64be(const uint8_t *v1, const uint8_t *v2) {
static int dtls1_record_replay_check(SSL *s, DTLS1_BITMAP *bitmap);
static void dtls1_record_bitmap_update(SSL *s, DTLS1_BITMAP *bitmap);
static int dtls1_buffer_record(SSL *s, record_pqueue *q,
uint8_t *priority);
static int dtls1_process_record(SSL *s);
static int do_dtls1_write(SSL *s, int type, const uint8_t *buf,
unsigned int len);
/* copy buffered record into SSL structure */
static int dtls1_copy_record(SSL *s, pitem *item) {
DTLS1_RECORD_DATA *rdata;
rdata = (DTLS1_RECORD_DATA *)item->data;
if (s->s3->rbuf.buf != NULL) {
OPENSSL_free(s->s3->rbuf.buf);
}
s->packet = rdata->packet;
s->packet_length = rdata->packet_length;
memcpy(&(s->s3->rbuf), &(rdata->rbuf), sizeof(SSL3_BUFFER));
memcpy(&(s->s3->rrec), &(rdata->rrec), sizeof(SSL3_RECORD));
/* Set proper sequence number for mac calculation */
memcpy(&(s->s3->read_sequence[2]), &(rdata->packet[5]), 6);
return 1;
}
static int dtls1_buffer_record(SSL *s, record_pqueue *queue,
uint8_t *priority) {
DTLS1_RECORD_DATA *rdata;
pitem *item;
/* Limit the size of the queue to prevent DOS attacks */
if (pqueue_size(queue->q) >= 100) {
return 0;
}
rdata = OPENSSL_malloc(sizeof(DTLS1_RECORD_DATA));
item = pitem_new(priority, rdata);
if (rdata == NULL || item == NULL) {
if (rdata != NULL) {
OPENSSL_free(rdata);
}
if (item != NULL) {
pitem_free(item);
}
OPENSSL_PUT_ERROR(SSL, dtls1_buffer_record, ERR_R_INTERNAL_ERROR);
return -1;
}
rdata->packet = s->packet;
rdata->packet_length = s->packet_length;
memcpy(&(rdata->rbuf), &(s->s3->rbuf), sizeof(SSL3_BUFFER));
memcpy(&(rdata->rrec), &(s->s3->rrec), sizeof(SSL3_RECORD));
item->data = rdata;
s->packet = NULL;
s->packet_length = 0;
memset(&(s->s3->rbuf), 0, sizeof(SSL3_BUFFER));
memset(&(s->s3->rrec), 0, sizeof(SSL3_RECORD));
if (!ssl3_setup_buffers(s)) {
goto internal_error;
}
/* insert should not fail, since duplicates are dropped */
if (pqueue_insert(queue->q, item) == NULL) {
goto internal_error;
}
return 1;
internal_error:
OPENSSL_PUT_ERROR(SSL, dtls1_buffer_record, ERR_R_INTERNAL_ERROR);
if (rdata->rbuf.buf != NULL) {
OPENSSL_free(rdata->rbuf.buf);
}
OPENSSL_free(rdata);
pitem_free(item);
return -1;
}
static int dtls1_process_record(SSL *s) {
int al;
SSL3_RECORD *rr;
@ -540,21 +460,6 @@ start:
* s->s3->rrec.length - number of bytes. */
rr = &s->s3->rrec;
/* We are not handshaking and have no data yet,
* so process data buffered during the last handshake
* in advance, if any.
*/
if (s->state == SSL_ST_OK && rr->length == 0) {
pitem *item;
item = pqueue_pop(s->d1->buffered_app_data.q);
if (item) {
dtls1_copy_record(s, item);
OPENSSL_free(item->data);
pitem_free(item);
}
}
/* Check for timeout */
if (dtls1_handle_timeout(s) > 0) {
goto start;
@ -581,12 +486,8 @@ start:
if (s->s3->change_cipher_spec && rr->type != SSL3_RT_HANDSHAKE &&
rr->type != SSL3_RT_ALERT) {
/* We now have an unexpected record between CCS and Finished. Most likely
* the packets were reordered on their way, so buffer the application data
* for later processing rather than dropping the connection. */
if (dtls1_buffer_record(s, &(s->d1->buffered_app_data), rr->seq_num) < 0) {
OPENSSL_PUT_ERROR(SSL, dtls1_read_bytes, ERR_R_INTERNAL_ERROR);
return -1;
}
* the packets were reordered on their way. DTLS is unreliable, so drop the
* packet and expect the peer to retransmit. */
rr->length = 0;
goto start;
}

23
ssl/test/runner/runner.go
View File

@ -695,6 +695,8 @@ var testCases = []testCase{
AppDataAfterChangeCipherSpec: []byte("TEST MESSAGE"),
},
},
// BoringSSL's DTLS implementation will drop the out-of-order
// application data.
},
{
name: "AlertAfterChangeCipherSpec",
@ -1174,21 +1176,14 @@ func doExchange(test *testCase, config *Config, conn net.Conn, messageLen int, i
return err
}
var testMessage []byte
if config.Bugs.AppDataAfterChangeCipherSpec != nil {
// We've already sent a message. Expect the shim to echo it
// back.
testMessage = config.Bugs.AppDataAfterChangeCipherSpec
} else {
if messageLen == 0 {
messageLen = 32
}
testMessage = make([]byte, messageLen)
for i := range testMessage {
testMessage[i] = 0x42
}
tlsConn.Write(testMessage)
if messageLen == 0 {
messageLen = 32
}
testMessage := make([]byte, messageLen)
for i := range testMessage {
testMessage[i] = 0x42
}
tlsConn.Write(testMessage)
buf := make([]byte, len(testMessage))
if test.protocol == dtls {