Test various empty string cases with NPN callbacks

NPN is a little odd, owing to being a three-step process. The client
offers NPN, then the server accepts NPN and offers a list of protocols,
then the client picks a protocol.

The server is permitted to accept NPN but then offer zero supported
protocols. This worked, but was not tested or clearly documented.

In the last step, the client *must* pick a protocol, but it is permitted
to pick the empty string. The semantics of this are not explicitly
stated in the draft, but one can infer it means we aren't picking a
protocol. This also worked but was not tested or clearly documented.

Change-Id: I26d7089f4902834ff68a97467fc826e957d5fdf8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69027
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>

1 files changed, 38 insertions, 19 deletions

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 04c191f..6a92a28 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3265,30 +3265,49 @@ OPENSSL_EXPORT int SSL_CTX_add_cert_compression_alg(
 // and deprecated in favor of it.
 
 // SSL_CTX_set_next_protos_advertised_cb sets a callback that is called when a
-// TLS server needs a list of supported protocols for Next Protocol
-// Negotiation. The returned list must be in wire format. The list is returned
-// by setting |*out| to point to it and |*out_len| to its length. This memory
-// will not be modified, but one should assume that |ssl| keeps a reference to
-// it.
-//
-// The callback should return |SSL_TLSEXT_ERR_OK| if it wishes to advertise.
-// Otherwise, no such extension will be included in the ServerHello.
+// TLS server needs a list of supported protocols for Next Protocol Negotiation.
+//
+// If the callback wishes to advertise NPN to the client, it should return
+// |SSL_TLSEXT_ERR_OK| and then set |*out| and |*out_len| to describe to a
+// buffer containing a (possibly empty) list of supported protocols in wire
+// format. That is, each protocol is prefixed with a 1-byte length, then
+// concatenated. From there, the client will select a protocol, possibly one not
+// on the server's list. The caller can use |SSL_get0_next_proto_negotiated|
+// after the handshake completes to query the final protocol.
+//
+// The returned buffer must remain valid and unmodified for at least the
+// duration of the |SSL| operation (e.g. |SSL_do_handshake|) that triggered the
+// callback.
+//
+// If the caller wishes not to advertise NPN, it should return
+// |SSL_TLSEXT_ERR_NOACK|. No NPN extension will be included in the ServerHello,
+// and the TLS server will behave as if it does not implement NPN.
 OPENSSL_EXPORT void SSL_CTX_set_next_protos_advertised_cb(
     SSL_CTX *ctx,
     int (*cb)(SSL *ssl, const uint8_t **out, unsigned *out_len, void *arg),
     void *arg);
 
 // SSL_CTX_set_next_proto_select_cb sets a callback that is called when a client
-// needs to select a protocol from the server's provided list. |*out| must be
-// set to point to the selected protocol (which may be within |in|). The length
-// of the protocol name must be written into |*out_len|. The server's advertised
-// protocols are provided in |in| and |in_len|. The callback can assume that
-// |in| is syntactically valid.
-//
-// The client must select a protocol. It is fatal to the connection if this
-// callback returns a value other than |SSL_TLSEXT_ERR_OK|.
-//
-// Configuring this callback enables NPN on a client.
+// needs to select a protocol from the server's provided list, passed in wire
+// format in |in_len| bytes from |in|. The callback can assume that |in| is
+// syntactically valid.
+//
+// On success, the callback should return |SSL_TLSEXT_ERR_OK| and set |*out| and
+// |*out_len| to describe a buffer containing the selected protocol, or an
+// empty buffer to select no protocol. The returned buffer may point within
+// |in|, or it may point to some other buffer that remains valid and unmodified
+// for at least the duration of the |SSL| operation (e.g. |SSL_do_handshake|)
+// that triggered the callback.
+//
+// Returning any other value indicates a fatal error and will terminate the TLS
+// connection. To proceed without selecting a protocol, the callback must return
+// |SSL_TLSEXT_ERR_OK| and set |*out| and |*out_len| to an empty buffer. (E.g.
+// NULL and zero, respectively.)
+//
+// Configuring this callback enables NPN on a client. Although the callback can
+// then decline to negotiate a protocol, merely configuring the callback causes
+// the client to offer NPN in the ClientHello. Callers thus should not configure
+// this callback in TLS client contexts that are not intended to use NPN.
 OPENSSL_EXPORT void SSL_CTX_set_next_proto_select_cb(
     SSL_CTX *ctx, int (*cb)(SSL *ssl, uint8_t **out, uint8_t *out_len,
                             const uint8_t *in, unsigned in_len, void *arg),
@@ -3296,7 +3315,7 @@ OPENSSL_EXPORT void SSL_CTX_set_next_proto_select_cb(
 
 // SSL_get0_next_proto_negotiated sets |*out_data| and |*out_len| to point to
 // the client's requested protocol for this connection. If the client didn't
-// request any protocol, then |*out_data| is set to NULL.
+// request any protocol, then |*out_len| is set to zero.
 //
 // Note that the client can request any protocol it chooses. The value returned
 // from this function need not be a member of the list of supported protocols