Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DTLS 1.3: Place start of ClientHello correctly when calculating binder #24426

Closed
wants to merge 9 commits into from
11 changes: 9 additions & 2 deletions crypto/packet.c
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,12 @@ int WPACKET_finish(WPACKET *pkt)
}

int WPACKET_start_sub_packet_len__(WPACKET *pkt, size_t lenbytes)
{
return WPACKET_start_sub_packet_at_offset_len__(pkt, lenbytes, 0);
}

int WPACKET_start_sub_packet_at_offset_len__(WPACKET *pkt, size_t lenbytes,
size_t offset)
{
WPACKET_SUB *sub;
unsigned char *lenchars;
Expand All @@ -381,7 +387,7 @@ int WPACKET_start_sub_packet_len__(WPACKET *pkt, size_t lenbytes)

sub->parent = pkt->subs;
pkt->subs = sub;
sub->pwritten = pkt->written + lenbytes;
sub->pwritten = pkt->written + lenbytes + offset;
sub->lenbytes = lenbytes;

if (lenbytes == 0) {
Expand All @@ -391,7 +397,8 @@ int WPACKET_start_sub_packet_len__(WPACKET *pkt, size_t lenbytes)

sub->packet_len = pkt->written;

if (!WPACKET_allocate_bytes(pkt, lenbytes, &lenchars))
if (!WPACKET_allocate_bytes(pkt, lenbytes, &lenchars)
|| (offset > 0 && !WPACKET_allocate_bytes(pkt, offset, &lenchars)))
return 0;

return 1;
Expand Down
4 changes: 4 additions & 0 deletions include/internal/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ __owur static ossl_inline int ossl_assert_int(int expr, const char *exprstr,
(c)[1]=(unsigned char)(((l)>> 8)&0xff), \
(c)[2]=(unsigned char)(((l) )&0xff)),(c)+=3)

#define l3n2(c,l) (l =((uint64_t)(*((c)++)))<<16, \
l|=((uint64_t)(*((c)++)))<< 8, \
l|=((uint64_t)(*((c)++))))

static ossl_inline int ossl_ends_with_dirsep(const char *path)
{
if (*path != '\0')
Expand Down
12 changes: 12 additions & 0 deletions include/internal/packet.h
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,18 @@ int WPACKET_finish(WPACKET *pkt);
*/
int WPACKET_fill_lengths(WPACKET *pkt);

/*
* Initialise a new sub-packet. Additionally |lenbytes| of data is preallocated
* at the current position in |pkt| to store the sub-packets length once we know
* it. The sub-packet start is set at |offset| from current position in |pkt|
* Don't call this directly. Use the convenience macros below instead.
*/
int WPACKET_start_sub_packet_at_offset_len__(WPACKET *pkt, size_t lenbytes,
size_t offset);

#define WPACKET_start_sub_packet_u24_at_offset(pkt, offset) \
WPACKET_start_sub_packet_at_offset_len__((pkt), 3, (offset))

/*
* Initialise a new sub-packet. Additionally |lenbytes| of data is preallocated
* at the start of the sub-packet to store its length once we know it. Don't
Expand Down
22 changes: 19 additions & 3 deletions ssl/statem/extensions.c
Original file line number Diff line number Diff line change
Expand Up @@ -1535,8 +1535,8 @@ int tls_psk_do_binder(SSL_CONNECTION *s, const EVP_MD *md,
static const unsigned char resumption_label[] = "\x72\x65\x73\x20\x62\x69\x6E\x64\x65\x72";
/* ASCII: "ext binder", in hex for EBCDIC compatibility */
static const unsigned char external_label[] = "\x65\x78\x74\x20\x62\x69\x6E\x64\x65\x72";
const unsigned char *label;
size_t bindersize, labelsize, hashsize;
const unsigned char *label, *msgbodystart;
size_t bindersize, labelsize, hashsize, msgbodylen;
int hashsizei = EVP_MD_get_size(md);
int ret = -1;
int usepskfored = 0;
Expand Down Expand Up @@ -1654,7 +1654,23 @@ int tls_psk_do_binder(SSL_CONNECTION *s, const EVP_MD *md,
}
}

if (EVP_DigestUpdate(mctx, msgstart, binderoffset) <= 0
if (SSL_CONNECTION_IS_DTLS(s)) {
msgbodystart = msgstart + DTLS1_HM_HEADER_LENGTH;
msgbodylen = binderoffset - DTLS1_HM_HEADER_LENGTH;
} else {
msgbodystart = msgstart + SSL3_HM_HEADER_LENGTH;
msgbodylen = binderoffset - SSL3_HM_HEADER_LENGTH;
}

/*
* RFC9147 (DTLSv1.3)
* The transcript consists of complete TLS Handshake messages
* (reassembled as necessary). Note that this requires removing the
* message_seq, fragment_offset, and fragment_length fields to create
* the Handshake structure.
*/
if (EVP_DigestUpdate(mctx, msgstart, SSL3_HM_HEADER_LENGTH) <= 0
|| EVP_DigestUpdate(mctx, msgbodystart, msgbodylen) <= 0
|| EVP_DigestFinal_ex(mctx, hash, NULL) <= 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
Expand Down
5 changes: 2 additions & 3 deletions ssl/statem/extensions_clnt.c
Original file line number Diff line number Diff line change
Expand Up @@ -1206,9 +1206,8 @@ EXT_RETURN tls_construct_ctos_psk(SSL_CONNECTION *s, WPACKET *pkt,

msgstart = WPACKET_get_curr(pkt) - msglen;

if (dores
&& tls_psk_do_binder(s, mdres, msgstart, binderoffset, NULL,
resbinder, s->session, 1, 0) != 1) {
if (dores && tls_psk_do_binder(s, mdres, msgstart, binderoffset, NULL,
resbinder, s->session, 1, 0) != 1) {
/* SSLfatal() already called */
return EXT_RETURN_FAIL;
}
Expand Down
Loading
Loading