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: NRich Salz <rsalz@openssl.org>
Reviewed-by: NEmilia Käsper <emilia@openssl.org>

CHANGES

@@ -4,6 +4,10 @@
 
  Changes between 1.0.2g and 1.1.0  [xx XXX xxxx]
 
+  *) Modify behavior of ALPN to invoke callback after SNI/servername
+     callback, such that updates to the SSL_CTX affect ALPN.
+     [Todd Short]
+
   *) Changes to the DEFAULT cipherlist:
        - Prefer (EC)DHE handshakes over plain RSA.
        - Prefer AEAD ciphers over legacy ciphers.

apps/apps.c

@@ -1960,7 +1960,7 @@ void policies_print(X509_STORE_CTX *ctx)
  *
  *   returns: a malloced buffer or NULL on failure.
  */
-unsigned char *next_protos_parse(unsigned short *outlen, const char *in)
+unsigned char *next_protos_parse(size_t *outlen, const char *in)
 {
     size_t len;
     unsigned char *out;

apps/apps.h

@@ -565,7 +565,7 @@ int do_X509_CRL_sign(X509_CRL *x, EVP_PKEY *pkey, const EVP_MD *md,
 extern char *psk_key;
 # endif
 
-unsigned char *next_protos_parse(unsigned short *outlen, const char *in);
+unsigned char *next_protos_parse(size_t *outlen, const char *in);
 
 void print_cert_checks(BIO *bio, X509 *x,
                        const char *checkhost,

apps/s_client.c

@@ -445,7 +445,7 @@ static char *srtp_profiles = NULL;
 /* This the context that we pass to next_proto_cb */
 typedef struct tlsextnextprotoctx_st {
     unsigned char *data;
-    unsigned short len;
+    size_t len;
     int status;
 } tlsextnextprotoctx;
 
@@ -1634,7 +1634,7 @@ int s_client_main(int argc, char **argv)
         SSL_CTX_set_next_proto_select_cb(ctx, next_proto_cb, &next_proto);
 #endif
     if (alpn_in) {
-        unsigned short alpn_len;
+        size_t alpn_len;
         unsigned char *alpn = next_protos_parse(&alpn_len, alpn_in);
 
         if (alpn == NULL) {

apps/s_server.c

@@ -743,7 +743,7 @@ static int next_proto_cb(SSL *s, const unsigned char **data,
 /* This the context that we pass to alpn_cb */
 typedef struct tlsextalpnctx_st {
     unsigned char *data;
-    unsigned short len;
+    size_t len;
 } tlsextalpnctx;
 
 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,
 
     if (!s_quiet) {
         /* We can assume that |in| is syntactically valid. */
-        unsigned i;
+        unsigned int i;
         BIO_printf(bio_s_out, "ALPN protocols advertised by the client: ");
         for (i = 0; i < inlen;) {
             if (i)
@@ -1620,7 +1620,7 @@ int s_server_main(int argc, char *argv[])
     }
 #if !defined(OPENSSL_NO_NEXTPROTONEG)
     if (next_proto_neg_in) {
-        unsigned short len;
+        size_t len;
         next_proto.data = next_protos_parse(&len, next_proto_neg_in);
         if (next_proto.data == NULL)
             goto end;
@@ -1631,7 +1631,7 @@ int s_server_main(int argc, char *argv[])
 #endif
     alpn_ctx.data = NULL;
     if (alpn_in) {
-        unsigned short len;
+        size_t len;
         alpn_ctx.data = next_protos_parse(&len, alpn_in);
         if (alpn_ctx.data == NULL)
             goto end;

doc/ssl/SSL_CTX_set_alpn_select_cb.pod

+=pod
+
+=head1 NAME
+
+SSL_CTX_set_alpn_select_cb, SSL_CTX_set_alpn_protos, SSL_set_alpn_protos,
+SSL_get0_alpn_selected, SSL_select_next_proto - handle application layer
+protocol negotiation (ALPN)
+
+=head1 SYNOPSIS
+
+ #include <openssl/ssl.h>
+
+ int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos,
+                             unsigned int protos_len);
+ int SSL_set_alpn_protos(SSL *ssl, const unsigned char *protos,
+                         unsigned int protos_len);
+ void SSL_CTX_set_alpn_select_cb(SSL_CTX *ctx,
+                                 int (*cb) (SSL *ssl,
+                                            const unsigned char **out,
+                                            unsigned char *outlen,
+                                            const unsigned char *in,
+                                            unsigned int inlen,
+                                            void *arg), void *arg);
+ int SSL_select_next_proto(unsigned char **out, unsigned char *outlen,
+                           const unsigned char *server,
+                           unsigned int server_len,
+                           const unsigned char *client,
+                           unsigned int client_len)
+ void SSL_get0_alpn_selected(const SSL *ssl, const unsigned char **data,
+                             unsigned int *len);
+
+=head1 DESCRIPTION
+
+SSL_CTX_set_alpn_protos() and SSL_set_alpn_protos() are used by the client to
+set the list of protocols available to be negotiated. The B<protos> must be in
+protocol-list format, described below. The length of B<protos> is specified in
+B<protos_len>.
+
+SSL_CTX_set_alpn_select_cb() sets the application callback B<cb> used by a
+server to select which protocol to use for the incoming connection. When B<cb>
+is NULL, no ALPN is not used. The B<arg> value is pointer which is passed to
+the application callback.
+
+B<cb> is the application defined callback. The B<in>, B<inlen> parameters are a
+vector in protocol-list format. The value of the B<out>, B<outlen> vector
+should be set to the value of a single protocol contained with in the B<in>,
+B<inlen> vector. The B<arg> parameter is the pointer set via
+SSL_CTX_set_alpn_select_cb().
+
+SSL_select_next_proto() is a helper function used to select protocols. It
+implements the standard protocol selection. It is expected that this function
+is called from the application callback B<cb>. The protocol data in B<server>,
+B<server_len> and B<client>, B<client_len> must be in protocol-list format
+described below. The first item in the B<server>, B<server_len> list that
+matches an item in the B<client>, B<client_len> list is selected, and returned
+in B<out>, B<outlen>. The B<out> value will point into either B<server> or
+B<client>, so it should be copied immediately. If no match is found, the first
+item in B<client>, B<client_len> is returned in B<out>, B<outlen>. This
+function can also be used in the NPN callback.
+
+SSL_get0_alpn_selected() returns a pointer to the selected protocol in B<data>
+with length B<len>. It is not NUL-terminated. B<data> is set to NULL and B<len>
+is set to 0 if no protocol has been selected. B<data> value must not be freed.
+
+=head1 NOTES
+
+The protocol-lists must be in wire-format, which is defined as a vector of
+non-empty, 8-bit length-prefixed, byte strings. The length-prefix byte is not
+included in the length. Each string is limited to 255 bytes. A byte-string
+length of 0 is invalid. A truncated byte-string is invalid. The length of the
+vector is not in the vector itself, but in a separate variable.
+
+Example:
+
+ unsigned char vector[] = {
+     6, 's', 'p', 'd', 'y', '/', '1',
+     8, 'h', 't', 't', 'p', '/', '1', '.', '1'
+ };
+ unsigned int length = sizeof(vector);
+
+The ALPN callback is executed after the servername callback; as that servername
+callback may update the SSL_CTX, and subsequently, the ALPN callback.
+
+If there is no ALPN proposed in the ClientHello, the ALPN callback is not
+invoked.
+
+=head1 RETURN VALUES
+
+SSL_CTX_set_alpn_protos() and SSL_set_alpn_protos() return 0 on success, and
+non-0 on failure. WARNING: these functions reverse the return value convention.
+
+SSL_select_next_proto() returns one of the following:
+
+=over 4
+
+=item OPENSSL_NPN_NEGOTIATED
+
+A match was found and is returned in B<out>, B<outlen>.
+
+=item OPENSSL_NPN_NO_OVERLAP
+
+No match was found. The first item in B<client>, B<client_len> is returned in
+B<out>, B<outlen>.
+
+=back
+
+The ALPN select callback B<cb>, must return one of the following:
+
+=over 4
+
+=item SSL_TLSEXT_ERR_OK
+
+ALPN protocol selected.
+
+=item SSL_TLSEXT_ERR_NOACK
+
+ALPN protocol not selected.
+
+=back
+
+=head1 SEE ALSO
+
+L<ssl(3)>, L<SSL_CTX_set_tlsext_servername_callback(3)>,
+L<SSL_CTX_set_tlsext_servername_arg(3)>
+
+=cut

include/openssl/ssl.h

@@ -782,9 +782,9 @@ __owur int SSL_select_next_proto(unsigned char **out, unsigned char *outlen,
 # define OPENSSL_NPN_NO_OVERLAP  2
 
 __owur int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos,
-                            unsigned protos_len);
+                                   unsigned int protos_len);
 __owur int SSL_set_alpn_protos(SSL *ssl, const unsigned char *protos,
-                        unsigned protos_len);
+                               unsigned int protos_len);
 void SSL_CTX_set_alpn_select_cb(SSL_CTX *ctx,
                                 int (*cb) (SSL *ssl,
                                            const unsigned char **out,
@@ -793,7 +793,7 @@ void SSL_CTX_set_alpn_select_cb(SSL_CTX *ctx,
                                            unsigned int inlen,
                                            void *arg), void *arg);
 void SSL_get0_alpn_selected(const SSL *ssl, const unsigned char **data,
-                            unsigned *len);
+                            unsigned int *len);
 
 # ifndef OPENSSL_NO_PSK
 /*

ssl/s3_lib.c

@@ -3047,6 +3047,7 @@ void ssl3_free(SSL *s)
     OPENSSL_free(s->s3->tmp.peer_sigalgs);
     ssl3_free_digest_list(s);
     OPENSSL_free(s->s3->alpn_selected);
+    OPENSSL_free(s->s3->alpn_proposed);
 
 #ifndef OPENSSL_NO_SRP
     SSL_SRP_CTX_free(s);
@@ -3060,37 +3061,24 @@ void ssl3_clear(SSL *s)
     ssl3_cleanup_key_block(s);
     sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free);
     OPENSSL_free(s->s3->tmp.ciphers_raw);
-    s->s3->tmp.ciphers_raw = NULL;
     OPENSSL_clear_free(s->s3->tmp.pms, s->s3->tmp.pmslen);
-    s->s3->tmp.pms = NULL;
     OPENSSL_free(s->s3->tmp.peer_sigalgs);
-    s->s3->tmp.peer_sigalgs = NULL;
 
-#ifndef OPENSSL_NO_EC
-    s->s3->is_probably_safari = 0;
-#endif
 #if !defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH)
     EVP_PKEY_free(s->s3->tmp.pkey);
-    s->s3->tmp.pkey = NULL;
     EVP_PKEY_free(s->s3->peer_tmp);
-    s->s3->peer_tmp = NULL;
 #endif                         /* !OPENSSL_NO_EC */
 
     ssl3_free_digest_list(s);
 
-    if (s->s3->alpn_selected) {
-        OPENSSL_free(s->s3->alpn_selected);
-        s->s3->alpn_selected = NULL;
-    }
+    OPENSSL_free(s->s3->alpn_selected);
+    OPENSSL_free(s->s3->alpn_proposed);
 
+    /* NULL/zero-out everything in the s3 struct */
     memset(s->s3, 0, sizeof(*s->s3));
 
     ssl_free_wbio_buffer(s);
 
-    s->s3->renegotiate = 0;
-    s->s3->total_renegotiations = 0;
-    s->s3->num_renegotiations = 0;
-    s->s3->in_read_app_data = 0;
     s->version = SSL3_VERSION;
 
 #if !defined(OPENSSL_NO_NEXTPROTONEG)

ssl/ssl_lib.c

@@ -2220,15 +2220,14 @@ void SSL_CTX_set_next_proto_select_cb(SSL_CTX *ctx,
  * length-prefixed strings). Returns 0 on success.
  */
 int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos,
-                            unsigned protos_len)
+                            unsigned int protos_len)
 {
     OPENSSL_free(ctx->alpn_client_proto_list);
-    ctx->alpn_client_proto_list = OPENSSL_malloc(protos_len);
+    ctx->alpn_client_proto_list = OPENSSL_memdup(protos, protos_len);
     if (ctx->alpn_client_proto_list == NULL) {
         SSLerr(SSL_F_SSL_CTX_SET_ALPN_PROTOS, ERR_R_MALLOC_FAILURE);
         return 1;
     }
-    memcpy(ctx->alpn_client_proto_list, protos, protos_len);
     ctx->alpn_client_proto_list_len = protos_len;
 
     return 0;
@@ -2240,15 +2239,14 @@ int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos,
  * length-prefixed strings). Returns 0 on success.
  */
 int SSL_set_alpn_protos(SSL *ssl, const unsigned char *protos,
-                        unsigned protos_len)
+                        unsigned int protos_len)
 {
     OPENSSL_free(ssl->alpn_client_proto_list);
-    ssl->alpn_client_proto_list = OPENSSL_malloc(protos_len);
+    ssl->alpn_client_proto_list = OPENSSL_memdup(protos, protos_len);
     if (ssl->alpn_client_proto_list == NULL) {
         SSLerr(SSL_F_SSL_SET_ALPN_PROTOS, ERR_R_MALLOC_FAILURE);
         return 1;
     }
-    memcpy(ssl->alpn_client_proto_list, protos, protos_len);
     ssl->alpn_client_proto_list_len = protos_len;
 
     return 0;
@@ -2278,7 +2276,7 @@ void SSL_CTX_set_alpn_select_cb(SSL_CTX *ctx,
  * respond with a negotiated protocol then |*len| will be zero.
  */
 void SSL_get0_alpn_selected(const SSL *ssl, const unsigned char **data,
-                            unsigned *len)
+                            unsigned int *len)
 {
     *data = NULL;
     if (ssl->s3)

ssl/ssl_locl.h

@@ -1372,7 +1372,12 @@ typedef struct ssl3_state_st {
      * that the server selected once the ServerHello has been processed.
      */
     unsigned char *alpn_selected;
-    unsigned alpn_selected_len;
+    size_t alpn_selected_len;
+    /* used by the server to know what options were proposed */
+    unsigned char *alpn_proposed;
+    size_t alpn_proposed_len;
+    /* used by the client to know if it actually sent alpn */
+    int alpn_sent;
 
 #   ifndef OPENSSL_NO_EC
     /*

ssl/t1_lib.c

@@ -1413,6 +1413,11 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
     }
 #endif
 
+    /*
+     * finish_md_len is non-zero during a renegotiation, so
+     * this avoids sending ALPN during the renegotiation
+     * (see longer comment below)
+     */
     if (s->alpn_client_proto_list && !s->s3->tmp.finish_md_len) {
         if ((size_t)(limit - ret) < 6 + s->alpn_client_proto_list_len)
             return NULL;
@@ -1421,6 +1426,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
         s2n(s->alpn_client_proto_list_len, ret);
         memcpy(ret, s->alpn_client_proto_list, s->alpn_client_proto_list_len);
         ret += s->alpn_client_proto_list_len;
+        s->s3->alpn_sent = 1;
     }
 #ifndef OPENSSL_NO_SRTP
     if (SSL_IS_DTLS(s) && SSL_get_srtp_profiles(s)) {
@@ -1701,9 +1707,9 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
         s2n(0, ret);
     }
 
-    if (s->s3->alpn_selected) {
+    if (s->s3->alpn_selected != NULL) {
         const unsigned char *selected = s->s3->alpn_selected;
-        unsigned len = s->s3->alpn_selected_len;
+        unsigned int len = s->s3->alpn_selected_len;
 
         if ((long)(limit - ret - 4 - 2 - 1 - len) < 0)
             return NULL;
@@ -1725,16 +1731,13 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
 }
 
 /*
- * Process the ALPN extension in a ClientHello.
+ * Save the ALPN extension in a ClientHello.
  * pkt: the contents of the ALPN extension, not including type and length.
  * al: a pointer to the  alert value to send in the event of a failure.
  * returns: 1 on success, 0 on error.
  */
 static int tls1_alpn_handle_client_hello(SSL *s, PACKET *pkt, int *al)
 {
-    const unsigned char *selected;
-    unsigned char selected_len;
-    int r;
     PACKET protocol_list, save_protocol_list, protocol;
 
     *al = SSL_AD_DECODE_ERROR;
@@ -1753,25 +1756,47 @@ static int tls1_alpn_handle_client_hello(SSL *s, PACKET *pkt, int *al)
         }
     } while (PACKET_remaining(&protocol_list) != 0);
 
-    if (s->ctx->alpn_select_cb == NULL)
-        return 1;
+    if (!PACKET_memdup(&save_protocol_list,
+                       &s->s3->alpn_proposed,
+                       &s->s3->alpn_proposed_len)) {
+        *al = TLS1_AD_INTERNAL_ERROR;
+        return 0;
+    }
+
+    return 1;
+}
+
+/*
+ * Process the ALPN extension in a ClientHello.
+ * ret: a pointer to the TLSEXT return value: SSL_TLSEXT_ERR_*
+ * al: a pointer to the alert value to send in the event of a failure.
+ * returns 1 on success, 0
+ */
+static int tls1_alpn_handle_client_hello_late(SSL *s, int *ret, int *al)
+{
+    const unsigned char *selected = NULL;
+    unsigned char selected_len = 0;
 
-    r = s->ctx->alpn_select_cb(s, &selected, &selected_len,
-                               PACKET_data(&save_protocol_list),
-                               PACKET_remaining(&save_protocol_list),
-                               s->ctx->alpn_select_cb_arg);
-    if (r == SSL_TLSEXT_ERR_OK) {
-        OPENSSL_free(s->s3->alpn_selected);
-        s->s3->alpn_selected = OPENSSL_malloc(selected_len);
-        if (s->s3->alpn_selected == NULL) {
-            *al = SSL_AD_INTERNAL_ERROR;
+    if (s->ctx->alpn_select_cb != NULL && s->s3->alpn_proposed != NULL) {
+        int r = s->ctx->alpn_select_cb(s, &selected, &selected_len,
+                                       s->s3->alpn_proposed,
+                                       s->s3->alpn_proposed_len,
+                                       s->ctx->alpn_select_cb_arg);
+
+        if (r == SSL_TLSEXT_ERR_OK) {
+            OPENSSL_free(s->s3->alpn_selected);
+            s->s3->alpn_selected = OPENSSL_memdup(selected, selected_len);
+            if (s->s3->alpn_selected == NULL) {
+                *al = SSL_AD_INTERNAL_ERROR;
+                *ret = SSL_TLSEXT_ERR_ALERT_FATAL;
+                return 0;
+            }
+            s->s3->alpn_selected_len = selected_len;
+        } else {
+            *al = SSL_AD_NO_APPLICATION_PROTOCOL;
+            *ret = SSL_TLSEXT_ERR_ALERT_FATAL;
             return 0;
         }
-        memcpy(s->s3->alpn_selected, selected, selected_len);
-        s->s3->alpn_selected_len = selected_len;
-    } else {
-        *al = SSL_AD_NO_APPLICATION_PROTOCOL;
-        return 0;
     }
 
     return 1;
@@ -2484,7 +2509,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al)
         else if (type == TLSEXT_TYPE_application_layer_protocol_negotiation) {
             unsigned len;
             /* We must have requested it. */
-            if (s->alpn_client_proto_list == NULL) {
+            if (!s->s3->alpn_sent) {
                 *al = TLS1_AD_UNSUPPORTED_EXTENSION;
                 return 0;
             }
@@ -2617,7 +2642,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al)
 
 int ssl_prepare_clienthello_tlsext(SSL *s)
 {
-
+    s->s3->alpn_sent = 0;
     return 1;
 }
 
@@ -2776,6 +2801,10 @@ int ssl_check_clienthello_tlsext_late(SSL *s)
     } else
         s->tlsext_status_expected = 0;
 
+    if (!tls1_alpn_handle_client_hello_late(s, &ret, &al)) {
+        goto err;
+    }
+
  err:
     switch (ret) {
     case SSL_TLSEXT_ERR_ALERT_FATAL:

test/recipes/80-test_ssl.t

@@ -64,7 +64,7 @@ my $P2intermediate="tmp_intP2.ss";
 plan tests =>
     1				# For testss
     + 1				# For ssltest -test_cipherlist
-    + 11			# For the first testssl
+    + 13			# For the first testssl
     + 16			# For the first testsslproxy
     + 16			# For the second testsslproxy
     ;
@@ -603,10 +603,29 @@ sub testssl {
 	}
     };
 
+    subtest 'SNI tests' => sub {
+
+	plan tests => 7;
+
+      SKIP: {
+	  skip "TLSv1.x is not supported by this OpenSSL build", 7
+	      if $no_tls1 && $no_tls1_1 && $no_tls1_2;
+
+	  ok(run(test([@ssltest, "-bio_pair", "-sn_client", "foo"])));
+	  ok(run(test([@ssltest, "-bio_pair", "-sn_server1", "foo"])));
+	  ok(run(test([@ssltest, "-bio_pair", "-sn_client", "foo", "-sn_server1", "foo", "-sn_expect1"])));
+	  ok(run(test([@ssltest, "-bio_pair", "-sn_client", "foo", "-sn_server1", "bar", "-sn_expect1"])));
+	  ok(run(test([@ssltest, "-bio_pair", "-sn_client", "foo", "-sn_server1", "foo", "-sn_server2", "bar", "-sn_expect1"])));
+	  ok(run(test([@ssltest, "-bio_pair", "-sn_client", "bar", "-sn_server1", "foo", "-sn_server2", "bar", "-sn_expect2"])));
+	  # Negative test - make sure it doesn't crash, and doesn't switch contexts
+	  ok(run(test([@ssltest, "-bio_pair", "-sn_client", "foobar", "-sn_server1", "foo", "-sn_server2", "bar", "-sn_expect1"])));
+	}
+    };
+
     subtest 'ALPN tests' => sub {
 	######################################################################
 
-	plan tests => 14;
+	plan tests => 12;
 
       SKIP: {
 	  skip "TLSv1.0 is not supported by this OpenSSL build", 12
@@ -626,22 +645,39 @@ sub testssl {
 	  is(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "baz", "-alpn_server", "bar,foo"])), 0,
              "Testing ALPN with protocol mismatch, expecting failure");
 
-	SKIP: {
-	    skip "skipping SRP tests", 4
-		if $no_srp;
+	  # ALPN + SNI
+	  ok(run(test([@ssltest, "-bio_pair",
+		       "-alpn_client", "foo,bar", "-sn_client", "alice",
+		       "-alpn_server1", "foo,123", "-sn_server1", "alice",
+		       "-alpn_server2", "bar,456", "-sn_server2", "bob",
+		       "-alpn_expected", "foo"])));
+	  ok(run(test([@ssltest, "-bio_pair",
+		       "-alpn_client", "foo,bar", "-sn_client", "bob",
+		       "-alpn_server1", "foo,123", "-sn_server1", "alice",
+		       "-alpn_server2", "bar,456", "-sn_server2", "bob",
+		       "-alpn_expected", "bar"])));
+	}
+    };
 
-	    ok(run(test([@ssltest, "-tls1", "-cipher", "SRP", "-srpuser", "test", "-srppass", "abc123"])),
-	       'test tls1 with SRP');
+    subtest 'SRP tests' => sub {
 
-	    ok(run(test([@ssltest, "-bio_pair", "-tls1", "-cipher", "SRP", "-srpuser", "test", "-srppass", "abc123"])),
-	       'test tls1 with SRP via BIO pair');
+	plan tests => 4;
 
-	    ok(run(test([@ssltest, "-tls1", "-cipher", "aSRP", "-srpuser", "test", "-srppass", "abc123"])),
-		 'test tls1 with SRP auth');
+      SKIP: {
+	  skip "skipping SRP tests", 4
+	      if $no_srp;
 
-	    ok(run(test([@ssltest, "-bio_pair", "-tls1", "-cipher", "aSRP", "-srpuser", "test", "-srppass", "abc123"])),
-	       'test tls1 with SRP auth via BIO pair');
-	  }
+	  ok(run(test([@ssltest, "-tls1", "-cipher", "SRP", "-srpuser", "test", "-srppass", "abc123"])),
+	     'test tls1 with SRP');
+
+	  ok(run(test([@ssltest, "-bio_pair", "-tls1", "-cipher", "SRP", "-srpuser", "test", "-srppass", "abc123"])),
+	     'test tls1 with SRP via BIO pair');
+
+	  ok(run(test([@ssltest, "-tls1", "-cipher", "aSRP", "-srpuser", "test", "-srppass", "abc123"])),
+	     'test tls1 with SRP auth');
+
+	  ok(run(test([@ssltest, "-bio_pair", "-tls1", "-cipher", "aSRP", "-srpuser", "test", "-srppass", "abc123"])),
+	     'test tls1 with SRP auth via BIO pair');
 	}
     };