On 08/01/2016 05:31 AM, Ben Laurie wrote:
The branch master has been updated
via 68e71e9d000b72d964eb8b4106a1d879a0da4908 (commit)
via 3260adf1901ff3a842676ec7fa8c53dbfc66c4bd (commit)
via 620c6ad3125d7631f08c37033d1cb4302aef819a (commit)
from 087d3e89932e00eede95353fbd988e2752bc2468 (commit)
Picking a somewhat-arbitrary chunk to reply to...
diff --git a/test/handshake_helper.c b/test/handshake_helper.c
index eecc6f7..c7023fe 100644
--- a/test/handshake_helper.c
+++ b/test/handshake_helper.c
@@ -315,6 +316,7 @@ static void configure_handshake_ctx(SSL_CTX *server_ctx, SSL_CTX *server2_ctx,
if (test_ctx->session_ticket_expected == SSL_TEST_SESSION_TICKET_BROKEN) {
SSL_CTX_set_tlsext_ticket_key_cb(server_ctx, broken_session_ticket_cb);
}
+#ifndef OPENSSL_NO_NEXTPROTONEG
if (test_ctx->server_npn_protocols != NULL) {
parse_protos(test_ctx->server_npn_protocols,
&server_ctx_data->npn_protocols,
@@ -360,6 +362,7 @@ static void configure_handshake_ctx(SSL_CTX *server_ctx, SSL_CTX *server2_ctx,
alpn_protos_len) == 0);
OPENSSL_free(alpn_protos);
}
+#endif
/*
* Use fixed session ticket keys so that we can decrypt a ticket created with
* one CTX in another CTX. Don't address server2 for the moment.
I think I'm confused as to whether OPENSSL_NO_NEXTPROTONEG is
supposed to also cover ALPN.
If we go back to when ALPN was introduced, commit
6f017a8f9db3a79f3a3406cf8d493ccd346db691, even then things seemed
inconsistent.
We get (in s_client):
@@ -364,6 +364,7 @@ static void sc_usage(void)
BIO_printf(bio_err," -proof_debug - request an audit
proof and print its he
# ifndef OPENSSL_NO_NEXTPROTONEG
BIO_printf(bio_err," -nextprotoneg arg - enable NPN
extension, considering named
+ BIO_printf(bio_err," -alpn arg - enable ALPN
extension, considering name
# endif
#ifndef OPENSSL_NO_TLSEXT
BIO_printf(bio_err," -serverinfo types - send empty
ClientHello extensions (comm
@@ -636,6 +637,7 @@ int MAIN(int argc, char **argv)
{NULL,0};
# ifndef OPENSSL_NO_NEXTPROTONEG
const char *next_proto_neg_in = NULL;
+ const char *alpn_in = NULL;
# endif
# define MAX_SI_TYPES 100
unsigned short serverinfo_types[MAX_SI_TYPES];
but also
@@ -1306,9 +1313,23 @@ bad:
*/
if (socket_type == SOCK_DGRAM) SSL_CTX_set_read_ahead(ctx,
1);
-#if !defined(OPENSSL_NO_TLSEXT) &&
!defined(OPENSSL_NO_NEXTPROTONEG)
+#if !defined(OPENSSL_NO_TLSEXT)
+# if !defined(OPENSSL_NO_NEXTPROTONEG)
if (next_proto.data)
SSL_CTX_set_next_proto_select_cb(ctx, next_proto_cb,
&next_proto);
+# endif
+ if (alpn_in)
+ {
+ unsigned short alpn_len;
+ unsigned char *alpn =
next_protos_parse(&alpn_len, alpn_in);
+
+ if (alpn == NULL)
+ {
+ BIO_printf(bio_err, "Error parsing -alpn
argument\n");
+ goto end;
+ }
+ SSL_CTX_set_alpn_protos(ctx, alpn, alpn_len);
+ }
#endif
#ifndef OPENSSL_NO_TLSEXT
if (serverinfo_types_count)
So some parts are supposed to be compiled out with
OPENSSL_NO_NEXTPROTONEG, but others are supposed to only be
conditional on OPENSSL_NO_TLSEXT. (These particular chunks look
like they would not compile with OPENSSL_NO_NEXTPROTONEG and not
OPENSSL_NO_TLSEXT, since alpn_in is used but not declared.)
Even in current master, if we look at (e.g.) ssl_locl.h, several
next_proto-* fields in struct ssl_ctx_st are ifdef'd out for
OPENSSL_NO_NEXTPROTONEG, with the alpn_* bits unconditional
Configure doesn't help, just listing nextprotoneg as a disablable,
but no comments about what it does.
So ... what is OPENSSL_NO_NEXTPROTONEG supposed to control? Does it
disable NPN everywhere and also disable ALPN for tests [and apps],
but leave ALPN enabled in the library code? That's the best guess I
have from reading the code, but it's a bit unusual of an API
contract, so maybe it should be documented somewhere.
Thanks,
Ben