Skip to content

Commit 817cd0d

Browse files
tmshortRich Salz
tmshort
authored and
Rich Salz
committed
GH787: Fix ALPN
* Perform ALPN after the SNI callback; the SSL_CTX may change due to that processing * Add flags to indicate that we actually sent ALPN, to properly error out if unexpectedly received. * clean up ssl3_free() no need to explicitly clear when doing memset * document ALPN functions Signed-off-by: Rich Salz <[email protected]> Reviewed-by: Emilia Käsper <[email protected]>
1 parent f18ce93 commit 817cd0d

13 files changed

Lines changed: 432 additions & 151 deletions

File tree

‎CHANGES‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44

55
Changes between 1.0.2g and 1.1.0 [xx XXX xxxx]
66

7+
*) Modify behavior of ALPN to invoke callback after SNI/servername
8+
callback, such that updates to the SSL_CTX affect ALPN.
9+
[Todd Short]
10+
711
*) Changes to the DEFAULT cipherlist:
812
- Prefer (EC)DHE handshakes over plain RSA.
913
- Prefer AEAD ciphers over legacy ciphers.

‎apps/apps.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1960,7 +1960,7 @@ void policies_print(X509_STORE_CTX *ctx)
19601960
*
19611961
* returns: a malloced buffer or NULL on failure.
19621962
*/
1963-
unsigned char *next_protos_parse(unsigned short *outlen, const char *in)
1963+
unsigned char *next_protos_parse(size_t *outlen, const char *in)
19641964
{
19651965
size_t len;
19661966
unsigned char *out;

‎apps/apps.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,7 @@ int do_X509_CRL_sign(X509_CRL *x, EVP_PKEY *pkey, const EVP_MD *md,
565565
extern char *psk_key;
566566
# endif
567567

568-
unsigned char *next_protos_parse(unsigned short *outlen, const char *in);
568+
unsigned char *next_protos_parse(size_t *outlen, const char *in);
569569

570570
void print_cert_checks(BIO *bio, X509 *x,
571571
const char *checkhost,

‎apps/s_client.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,7 @@ static char *srtp_profiles = NULL;
445445
/* This the context that we pass to next_proto_cb */
446446
typedef struct tlsextnextprotoctx_st {
447447
unsigned char *data;
448-
unsigned short len;
448+
size_t len;
449449
int status;
450450
} tlsextnextprotoctx;
451451

@@ -1634,7 +1634,7 @@ int s_client_main(int argc, char **argv)
16341634
SSL_CTX_set_next_proto_select_cb(ctx, next_proto_cb, &next_proto);
16351635
#endif
16361636
if (alpn_in) {
1637-
unsigned short alpn_len;
1637+
size_t alpn_len;
16381638
unsigned char *alpn = next_protos_parse(&alpn_len, alpn_in);
16391639

16401640
if (alpn == NULL) {

‎apps/s_server.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,7 @@ static int next_proto_cb(SSL *s, const unsigned char **data,
743743
/* This the context that we pass to alpn_cb */
744744
typedef struct tlsextalpnctx_st {
745745
unsigned char *data;
746-
unsigned short len;
746+
size_t len;
747747
} tlsextalpnctx;
748748

749749
static int alpn_cb(SSL *s, const unsigned char **out, unsigned char *outlen,
@@ -753,7 +753,7 @@ static int alpn_cb(SSL *s, const unsigned char **out, unsigned char *outlen,
753753

754754
if (!s_quiet) {
755755
/* We can assume that |in| is syntactically valid. */
756-
unsigned i;
756+
unsigned int i;
757757
BIO_printf(bio_s_out, "ALPN protocols advertised by the client: ");
758758
for (i = 0; i < inlen;) {
759759
if (i)
@@ -1620,7 +1620,7 @@ int s_server_main(int argc, char *argv[])
16201620
}
16211621
#if !defined(OPENSSL_NO_NEXTPROTONEG)
16221622
if (next_proto_neg_in) {
1623-
unsigned short len;
1623+
size_t len;
16241624
next_proto.data = next_protos_parse(&len, next_proto_neg_in);
16251625
if (next_proto.data == NULL)
16261626
goto end;
@@ -1631,7 +1631,7 @@ int s_server_main(int argc, char *argv[])
16311631
#endif
16321632
alpn_ctx.data = NULL;
16331633
if (alpn_in) {
1634-
unsigned short len;
1634+
size_t len;
16351635
alpn_ctx.data = next_protos_parse(&len, alpn_in);
16361636
if (alpn_ctx.data == NULL)
16371637
goto end;

‎doc/ssl/SSL_CTX_set_alpn_select_cb.pod‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
=pod
2+
3+
=head1 NAME
4+
5+
SSL_CTX_set_alpn_select_cb, SSL_CTX_set_alpn_protos, SSL_set_alpn_protos,
6+
SSL_get0_alpn_selected, SSL_select_next_proto - handle application layer
7+
protocol negotiation (ALPN)
8+
9+
=head1 SYNOPSIS
10+
11+
#include <openssl/ssl.h>
12+
13+
int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos,
14+
unsigned int protos_len);
15+
int SSL_set_alpn_protos(SSL *ssl, const unsigned char *protos,
16+
unsigned int protos_len);
17+
void SSL_CTX_set_alpn_select_cb(SSL_CTX *ctx,
18+
int (*cb) (SSL *ssl,
19+
const unsigned char **out,
20+
unsigned char *outlen,
21+
const unsigned char *in,
22+
unsigned int inlen,
23+
void *arg), void *arg);
24+
int SSL_select_next_proto(unsigned char **out, unsigned char *outlen,
25+
const unsigned char *server,
26+
unsigned int server_len,
27+
const unsigned char *client,
28+
unsigned int client_len)
29+
void SSL_get0_alpn_selected(const SSL *ssl, const unsigned char **data,
30+
unsigned int *len);
31+
32+
=head1 DESCRIPTION
33+
34+
SSL_CTX_set_alpn_protos() and SSL_set_alpn_protos() are used by the client to
35+
set the list of protocols available to be negotiated. The B<protos> must be in
36+
protocol-list format, described below. The length of B<protos> is specified in
37+
B<protos_len>.
38+
39+
SSL_CTX_set_alpn_select_cb() sets the application callback B<cb> used by a
40+
server to select which protocol to use for the incoming connection. When B<cb>
41+
is NULL, no ALPN is not used. The B<arg> value is pointer which is passed to
42+
the application callback.
43+
44+
B<cb> is the application defined callback. The B<in>, B<inlen> parameters are a
45+
vector in protocol-list format. The value of the B<out>, B<outlen> vector
46+
should be set to the value of a single protocol contained with in the B<in>,
47+
B<inlen> vector. The B<arg> parameter is the pointer set via
48+
SSL_CTX_set_alpn_select_cb().
49+
50+
SSL_select_next_proto() is a helper function used to select protocols. It
51+
implements the standard protocol selection. It is expected that this function
52+
is called from the application callback B<cb>. The protocol data in B<server>,
53+
B<server_len> and B<client>, B<client_len> must be in protocol-list format
54+
described below. The first item in the B<server>, B<server_len> list that
55+
matches an item in the B<client>, B<client_len> list is selected, and returned
56+
in B<out>, B<outlen>. The B<out> value will point into either B<server> or
57+
B<client>, so it should be copied immediately. If no match is found, the first
58+
item in B<client>, B<client_len> is returned in B<out>, B<outlen>. This
59+
function can also be used in the NPN callback.
60+
61+
SSL_get0_alpn_selected() returns a pointer to the selected protocol in B<data>
62+
with length B<len>. It is not NUL-terminated. B<data> is set to NULL and B<len>
63+
is set to 0 if no protocol has been selected. B<data> value must not be freed.
64+
65+
=head1 NOTES
66+
67+
The protocol-lists must be in wire-format, which is defined as a vector of
68+
non-empty, 8-bit length-prefixed, byte strings. The length-prefix byte is not
69+
included in the length. Each string is limited to 255 bytes. A byte-string
70+
length of 0 is invalid. A truncated byte-string is invalid. The length of the
71+
vector is not in the vector itself, but in a separate variable.
72+
73+
Example:
74+
75+
unsigned char vector[] = {
76+
6, 's', 'p', 'd', 'y', '/', '1',
77+
8, 'h', 't', 't', 'p', '/', '1', '.', '1'
78+
};
79+
unsigned int length = sizeof(vector);
80+
81+
The ALPN callback is executed after the servername callback; as that servername
82+
callback may update the SSL_CTX, and subsequently, the ALPN callback.
83+
84+
If there is no ALPN proposed in the ClientHello, the ALPN callback is not
85+
invoked.
86+
87+
=head1 RETURN VALUES
88+
89+
SSL_CTX_set_alpn_protos() and SSL_set_alpn_protos() return 0 on success, and
90+
non-0 on failure. WARNING: these functions reverse the return value convention.
91+
92+
SSL_select_next_proto() returns one of the following:
93+
94+
=over 4
95+
96+
=item OPENSSL_NPN_NEGOTIATED
97+
98+
A match was found and is returned in B<out>, B<outlen>.
99+
100+
=item OPENSSL_NPN_NO_OVERLAP
101+
102+
No match was found. The first item in B<client>, B<client_len> is returned in
103+
B<out>, B<outlen>.
104+
105+
=back
106+
107+
The ALPN select callback B<cb>, must return one of the following:
108+
109+
=over 4
110+
111+
=item SSL_TLSEXT_ERR_OK
112+
113+
ALPN protocol selected.
114+
115+
=item SSL_TLSEXT_ERR_NOACK
116+
117+
ALPN protocol not selected.
118+
119+
=back
120+
121+
=head1 SEE ALSO
122+
123+
L<ssl(3)>, L<SSL_CTX_set_tlsext_servername_callback(3)>,
124+
L<SSL_CTX_set_tlsext_servername_arg(3)>
125+
126+
=cut

‎include/openssl/ssl.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -782,9 +782,9 @@ __owur int SSL_select_next_proto(unsigned char **out, unsigned char *outlen,
782782
# define OPENSSL_NPN_NO_OVERLAP 2
783783

784784
__owur int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos,
785-
unsigned protos_len);
785+
unsigned int protos_len);
786786
__owur int SSL_set_alpn_protos(SSL *ssl, const unsigned char *protos,
787-
unsigned protos_len);
787+
unsigned int protos_len);
788788
void SSL_CTX_set_alpn_select_cb(SSL_CTX *ctx,
789789
int (*cb) (SSL *ssl,
790790
const unsigned char **out,
@@ -793,7 +793,7 @@ void SSL_CTX_set_alpn_select_cb(SSL_CTX *ctx,
793793
unsigned int inlen,
794794
void *arg), void *arg);
795795
void SSL_get0_alpn_selected(const SSL *ssl, const unsigned char **data,
796-
unsigned *len);
796+
unsigned int *len);
797797

798798
# ifndef OPENSSL_NO_PSK
799799
/*

‎ssl/s3_lib.c‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3047,6 +3047,7 @@ void ssl3_free(SSL *s)
30473047
OPENSSL_free(s->s3->tmp.peer_sigalgs);
30483048
ssl3_free_digest_list(s);
30493049
OPENSSL_free(s->s3->alpn_selected);
3050+
OPENSSL_free(s->s3->alpn_proposed);
30503051

30513052
#ifndef OPENSSL_NO_SRP
30523053
SSL_SRP_CTX_free(s);
@@ -3060,37 +3061,24 @@ void ssl3_clear(SSL *s)
30603061
ssl3_cleanup_key_block(s);
30613062
sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free);
30623063
OPENSSL_free(s->s3->tmp.ciphers_raw);
3063-
s->s3->tmp.ciphers_raw = NULL;
30643064
OPENSSL_clear_free(s->s3->tmp.pms, s->s3->tmp.pmslen);
3065-
s->s3->tmp.pms = NULL;
30663065
OPENSSL_free(s->s3->tmp.peer_sigalgs);
3067-
s->s3->tmp.peer_sigalgs = NULL;
30683066

3069-
#ifndef OPENSSL_NO_EC
3070-
s->s3->is_probably_safari = 0;
3071-
#endif
30723067
#if !defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH)
30733068
EVP_PKEY_free(s->s3->tmp.pkey);
3074-
s->s3->tmp.pkey = NULL;
30753069
EVP_PKEY_free(s->s3->peer_tmp);
3076-
s->s3->peer_tmp = NULL;
30773070
#endif /* !OPENSSL_NO_EC */
30783071

30793072
ssl3_free_digest_list(s);
30803073

3081-
if (s->s3->alpn_selected) {
3082-
OPENSSL_free(s->s3->alpn_selected);
3083-
s->s3->alpn_selected = NULL;
3084-
}
3074+
OPENSSL_free(s->s3->alpn_selected);
3075+
OPENSSL_free(s->s3->alpn_proposed);
30853076

3077+
/* NULL/zero-out everything in the s3 struct */
30863078
memset(s->s3, 0, sizeof(*s->s3));
30873079

30883080
ssl_free_wbio_buffer(s);
30893081

3090-
s->s3->renegotiate = 0;
3091-
s->s3->total_renegotiations = 0;
3092-
s->s3->num_renegotiations = 0;
3093-
s->s3->in_read_app_data = 0;
30943082
s->version = SSL3_VERSION;
30953083

30963084
#if !defined(OPENSSL_NO_NEXTPROTONEG)

‎ssl/ssl_lib.c‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2220,15 +2220,14 @@ void SSL_CTX_set_next_proto_select_cb(SSL_CTX *ctx,
22202220
* length-prefixed strings). Returns 0 on success.
22212221
*/
22222222
int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos,
2223-
unsigned protos_len)
2223+
unsigned int protos_len)
22242224
{
22252225
OPENSSL_free(ctx->alpn_client_proto_list);
2226-
ctx->alpn_client_proto_list = OPENSSL_malloc(protos_len);
2226+
ctx->alpn_client_proto_list = OPENSSL_memdup(protos, protos_len);
22272227
if (ctx->alpn_client_proto_list == NULL) {
22282228
SSLerr(SSL_F_SSL_CTX_SET_ALPN_PROTOS, ERR_R_MALLOC_FAILURE);
22292229
return 1;
22302230
}
2231-
memcpy(ctx->alpn_client_proto_list, protos, protos_len);
22322231
ctx->alpn_client_proto_list_len = protos_len;
22332232

22342233
return 0;
@@ -2240,15 +2239,14 @@ int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos,
22402239
* length-prefixed strings). Returns 0 on success.
22412240
*/
22422241
int SSL_set_alpn_protos(SSL *ssl, const unsigned char *protos,
2243-
unsigned protos_len)
2242+
unsigned int protos_len)
22442243
{
22452244
OPENSSL_free(ssl->alpn_client_proto_list);
2246-
ssl->alpn_client_proto_list = OPENSSL_malloc(protos_len);
2245+
ssl->alpn_client_proto_list = OPENSSL_memdup(protos, protos_len);
22472246
if (ssl->alpn_client_proto_list == NULL) {
22482247
SSLerr(SSL_F_SSL_SET_ALPN_PROTOS, ERR_R_MALLOC_FAILURE);
22492248
return 1;
22502249
}
2251-
memcpy(ssl->alpn_client_proto_list, protos, protos_len);
22522250
ssl->alpn_client_proto_list_len = protos_len;
22532251

22542252
return 0;
@@ -2278,7 +2276,7 @@ void SSL_CTX_set_alpn_select_cb(SSL_CTX *ctx,
22782276
* respond with a negotiated protocol then |*len| will be zero.
22792277
*/
22802278
void SSL_get0_alpn_selected(const SSL *ssl, const unsigned char **data,
2281-
unsigned *len)
2279+
unsigned int *len)
22822280
{
22832281
*data = NULL;
22842282
if (ssl->s3)

‎ssl/ssl_locl.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1372,7 +1372,12 @@ typedef struct ssl3_state_st {
13721372
* that the server selected once the ServerHello has been processed.
13731373
*/
13741374
unsigned char *alpn_selected;
1375-
unsigned alpn_selected_len;
1375+
size_t alpn_selected_len;
1376+
/* used by the server to know what options were proposed */
1377+
unsigned char *alpn_proposed;
1378+
size_t alpn_proposed_len;
1379+
/* used by the client to know if it actually sent alpn */
1380+
int alpn_sent;
13761381

13771382
# ifndef OPENSSL_NO_EC
13781383
/*

0 commit comments

Comments
 (0)