Re: [openssl-dev] [openssl-commits] OPENSSL_NO_NEXTPROTONEG and ALPN

Skip to first unread message

Benjamin Kaduk

Aug 2, 2016, 10:55:33 AM8/2/16
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);
     if (test_ctx->server_npn_protocols != NULL) {
@@ -360,6 +362,7 @@ static void configure_handshake_ctx(SSL_CTX *server_ctx, SSL_CTX *server2_ctx,
                                                alpn_protos_len) == 0);
      * 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
        BIO_printf(bio_err," -nextprotoneg arg - enable NPN extension, considering named
+       BIO_printf(bio_err," -alpn arg         - enable ALPN extension, considering name
 # endif
        BIO_printf(bio_err," -serverinfo types - send empty ClientHello extensions (comm
@@ -636,6 +637,7 @@ int MAIN(int argc, char **argv)
        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)
        if (
                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);
+               }
                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.


Reply all
Reply to author
0 new messages