HTTP client: make server/proxy and port params more consistent; minor other improvements

Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from openssl#11404)

Author: DDvO

crypto/cmp/cmp_ctx.c

@@ -95,7 +95,6 @@ OSSL_CMP_CTX *OSSL_CMP_CTX_new(void)
     ctx->status = -1;
     ctx->failInfoCode = -1;
 
-    ctx->serverPort = OSSL_CMP_DEFAULT_PORT;
     ctx->msg_timeout = 2 * 60;
 
     if ((ctx->untrusted_certs = sk_X509_new_null()) == NULL)
@@ -146,7 +145,7 @@ void OSSL_CMP_CTX_free(OSSL_CMP_CTX *ctx)
         return;
 
     OPENSSL_free(ctx->serverPath);
-    OPENSSL_free(ctx->serverName);
+    OPENSSL_free(ctx->server);
     OPENSSL_free(ctx->proxy);
     OPENSSL_free(ctx->no_proxy);
 
@@ -775,7 +774,7 @@ int OSSL_CMP_CTX_set1_senderNonce(OSSL_CMP_CTX *ctx,
 DEFINE_OSSL_CMP_CTX_set1(proxy, char)
 
 /* Set the (HTTP) host name of the CMP server */
-DEFINE_OSSL_CMP_CTX_set1(serverName, char)
+DEFINE_OSSL_CMP_CTX_set1(server, char)
 
 /* Set the server exclusion list of the HTTP proxy server */
 DEFINE_OSSL_CMP_CTX_set1(no_proxy, char)

crypto/cmp/cmp_http.c

@@ -35,24 +35,24 @@
 OSSL_CMP_MSG *OSSL_CMP_MSG_http_perform(OSSL_CMP_CTX *ctx,
                                         const OSSL_CMP_MSG *req)
 {
-    char server_port[32];
+    char server_port[32] = { '\0' };
     STACK_OF(CONF_VALUE) *headers = NULL;
-    OSSL_CMP_MSG *res = NULL;
     const char *const content_type_pkix = "application/pkixcmp";
+    OSSL_CMP_MSG *res;
 
-    if (ctx == NULL || req == NULL
-            || ctx->serverName == NULL || ctx->serverPort == 0) {
+    if (ctx == NULL || req == NULL) {
         CMPerr(0, CMP_R_NULL_ARGUMENT);
-        return 0;
+        return NULL;
     }
 
     if (!X509V3_add_value("Pragma", "no-cache", &headers))
         return NULL;
 
-    BIO_snprintf(server_port, sizeof(server_port), "%d", ctx->serverPort);
+    if (ctx->serverPort != 0)
+        BIO_snprintf(server_port, sizeof(server_port), "%d", ctx->serverPort);
 
     res = (OSSL_CMP_MSG *)
-        OSSL_HTTP_post_asn1(ctx->serverName, server_port, ctx->serverPath,
+        OSSL_HTTP_post_asn1(ctx->server, server_port, ctx->serverPath,
                             OSSL_CMP_CTX_get_http_cb_arg(ctx) != NULL,
                             ctx->proxy, ctx->no_proxy, NULL, NULL,
                             ctx->http_cb, OSSL_CMP_CTX_get_http_cb_arg(ctx),

crypto/cmp/cmp_local.h

@@ -36,7 +36,7 @@ struct ossl_cmp_ctx_st {
     void *transfer_cb_arg; /* allows to store optional argument to cb */
     /* HTTP-based transfer */
     char *serverPath;
-    char *serverName;
+    char *server;
     int serverPort;
     char *proxy;
     char *no_proxy;

crypto/err/openssl.txt

@@ -2606,6 +2606,7 @@ HTTP_R_REDIRECTION_FROM_HTTPS_TO_HTTP:112:redirection from https to http
 HTTP_R_REDIRECTION_NOT_ENABLED:116:redirection not enabled
 HTTP_R_RESPONSE_LINE_TOO_LONG:113:response line too long
 HTTP_R_RESPONSE_PARSE_ERROR:104:response parse error
+HTTP_R_SOCK_NOT_SUPPORTED:122:sock not supported
 HTTP_R_STATUS_CODE_UNSUPPORTED:114:status code unsupported
 HTTP_R_TLS_NOT_ENABLED:107:tls not enabled
 HTTP_R_TOO_MANY_REDIRECTIONS:115:too many redirections

crypto/http/http_client.c

@@ -21,7 +21,7 @@
 #include <openssl/buffer.h>
 #include <openssl/http.h>
 #include "internal/sockets.h"
-#include "internal/cryptlib.h"
+#include "internal/cryptlib.h" /* for ossl_assert() */
 
 #include "http_local.h"
 
@@ -157,7 +157,7 @@ int OSSL_HTTP_REQ_CTX_header(OSSL_HTTP_REQ_CTX *rctx, const char *server,
          * Section 5.1.2 of RFC 1945 states that the absoluteURI form is only
          * allowed when using a proxy
          */
-        if (BIO_printf(rctx->mem, "http://%s", server) <= 0)
+        if (BIO_printf(rctx->mem, OSSL_HTTP_PREFIX"%s", server) <= 0)
             return 0;
         if (port != NULL && BIO_printf(rctx->mem, ":%s", port) <= 0)
             return 0;
@@ -701,10 +701,8 @@ static BIO *HTTP_new_bio(const char *server /* optionally includes ":port" */,
     const char *port = server_port;
     BIO *cbio;
 
-    if (server == NULL) {
-        HTTPerr(0, ERR_R_PASSED_NULL_PARAMETER);
+    if (!ossl_assert(server != NULL))
         return NULL;
-    }
 
     if (proxy != NULL) {
         host = proxy;
@@ -714,7 +712,7 @@ static BIO *HTTP_new_bio(const char *server /* optionally includes ":port" */,
     host_end = strchr(host, '/');
     if (host_end != NULL && (size_t)(host_end - host) < sizeof(host_name)) {
         /* chop trailing string starting with '/' */
-        strncpy(host_name, host, host_end - host);
+        strncpy(host_name, host, host_end - host + 1);
         host = host_name;
     }
 
@@ -849,18 +847,28 @@ BIO *OSSL_HTTP_transfer(const char *server, const char *port, const char *path,
         HTTPerr(0, ERR_R_PASSED_INVALID_ARGUMENT);
         return NULL;
     }
-    /* remaining parameters are checked indirectly by the functions called */
 
-    proxy = http_adapt_proxy(proxy, no_proxy, server, use_ssl);
-    if (bio != NULL)
+    if (bio != NULL) {
         cbio = bio;
-    else
+    } else {
 #ifndef OPENSSL_NO_SOCK
+        if (server == NULL) {
+            HTTPerr(0, ERR_R_PASSED_NULL_PARAMETER);
+            return NULL;
+        }
+        if (*port == '\0')
+            port = NULL;
+        if (port == NULL && strchr(server, ':') == NULL)
+            port = use_ssl ? OSSL_HTTPS_PORT : OSSL_HTTP_PORT;
+        proxy = http_adapt_proxy(proxy, no_proxy, server, use_ssl);
         if ((cbio = HTTP_new_bio(server, port, proxy)) == NULL)
             return NULL;
 #else
+        HTTPerr(0, HTTP_R_SOCK_NOT_SUPPORTED);
         return NULL;
 #endif
+    }
+    /* remaining parameters are checked indirectly by the functions called */
 
     (void)ERR_set_mark(); /* prepare removing any spurious libssl errors */
     if (rbio == NULL && BIO_connect_retry(cbio, timeout) <= 0)
@@ -902,10 +910,10 @@ BIO *OSSL_HTTP_transfer(const char *server, const char *port, const char *path,
             if (lib == ERR_LIB_SSL || lib == ERR_LIB_HTTP
                     || (lib == ERR_LIB_BIO && reason == BIO_R_CONNECT_TIMEOUT)
                     || (lib == ERR_LIB_BIO && reason == BIO_R_CONNECT_ERROR)
-# ifndef OPENSSL_NO_CMP
+#ifndef OPENSSL_NO_CMP
                     || (lib == ERR_LIB_CMP
                         && reason == CMP_R_POTENTIALLY_INVALID_CERTIFICATE)
-# endif
+#endif
                 ) {
                 BIO_snprintf(buf, 200, "server=%s:%s", server, port);
                 ERR_add_error_data(1, buf);
@@ -949,17 +957,16 @@ BIO *OSSL_HTTP_transfer(const char *server, const char *port, const char *path,
 
 static int redirection_ok(int n_redir, const char *old_url, const char *new_url)
 {
-    static const char https[] = "https:";
-    int https_len = 6; /* strlen(https) */
+    size_t https_len = strlen(OSSL_HTTPS_NAME":");
 
     if (n_redir >= HTTP_VERSION_MAX_REDIRECTIONS) {
         HTTPerr(0, HTTP_R_TOO_MANY_REDIRECTIONS);
         return 0;
     }
     if (*new_url == '/') /* redirection to same server => same protocol */
         return 1;
-    if (strncmp(old_url, https, https_len) == 0 &&
-        strncmp(new_url, https, https_len) != 0) {
+    if (strncmp(old_url, OSSL_HTTPS_NAME":", https_len) == 0 &&
+        strncmp(new_url, OSSL_HTTPS_NAME":", https_len) != 0) {
         HTTPerr(0, HTTP_R_REDIRECTION_FROM_HTTPS_TO_HTTP);
         return 0;
     }
@@ -1122,8 +1129,8 @@ int OSSL_HTTP_proxy_connect(BIO *bio, const char *server, const char *port,
                             const char *proxyuser, const char *proxypass,
                             int timeout, BIO *bio_err, const char *prog)
 {
-# undef BUF_SIZE
-# define BUF_SIZE (8 * 1024)
+#undef BUF_SIZE
+#define BUF_SIZE (8 * 1024)
     char *mbuf = OPENSSL_malloc(BUF_SIZE);
     char *mbufp;
     int read_len = 0;
@@ -1132,11 +1139,13 @@ int OSSL_HTTP_proxy_connect(BIO *bio, const char *server, const char *port,
     int rv;
     time_t max_time = timeout > 0 ? time(NULL) + timeout : 0;
 
-    if (bio == NULL || server == NULL || port == NULL
+    if (bio == NULL || server == NULL
             || (bio_err != NULL && prog == NULL)) {
         HTTPerr(0, ERR_R_PASSED_NULL_PARAMETER);
         goto end;
     }
+    if (port == NULL || *port == '\0')
+        port = OSSL_HTTPS_PORT;
 
     if (mbuf == NULL || fbio == NULL) {
         BIO_printf(bio_err /* may be NULL */, "%s: out of memory", prog);
@@ -1256,6 +1265,5 @@ int OSSL_HTTP_proxy_connect(BIO *bio, const char *server, const char *port,
     }
     OPENSSL_free(mbuf);
     return ret;
-# undef BUF_SIZE
+#undef BUF_SIZE
 }
-

crypto/http/http_err.c

@@ -45,6 +45,8 @@ static const ERR_STRING_DATA HTTP_str_reasons[] = {
     "response line too long"},
     {ERR_PACK(ERR_LIB_HTTP, 0, HTTP_R_RESPONSE_PARSE_ERROR),
     "response parse error"},
+    {ERR_PACK(ERR_LIB_HTTP, 0, HTTP_R_SOCK_NOT_SUPPORTED),
+    "sock not supported"},
     {ERR_PACK(ERR_LIB_HTTP, 0, HTTP_R_STATUS_CODE_UNSUPPORTED),
     "status code unsupported"},
     {ERR_PACK(ERR_LIB_HTTP, 0, HTTP_R_TLS_NOT_ENABLED), "tls not enabled"},

crypto/http/http_lib.c

@@ -11,6 +11,7 @@
 #include <openssl/httperr.h>
 #include <openssl/err.h>
 #include <string.h>
+#include "internal/cryptlib.h" /* for ossl_assert() */
 
 #include "http_local.h"
 
@@ -24,8 +25,11 @@ int OSSL_HTTP_parse_url(const char *url, char **phost, char **pport,
 {
     char *p, *buf;
     char *host;
-    char *port = "80";
+    const char *port = OSSL_HTTP_PORT;
+    size_t https_len = strlen(OSSL_HTTPS_NAME);
 
+    if (!ossl_assert(https_len >= strlen(OSSL_HTTP_NAME)))
+        return 0;
     if (url == NULL) {
         HTTPerr(0, ERR_R_PASSED_NULL_PARAMETER);
         return 0;
@@ -46,16 +50,16 @@ int OSSL_HTTP_parse_url(const char *url, char **phost, char **pport,
 
     /* Check for initial colon */
     p = strchr(buf, ':');
-    if (p == NULL || p - buf > 5 /* strlen("https") */) {
+    if (p == NULL || (size_t)(p - buf) > https_len) {
         p = buf;
     } else {
         *(p++) = '\0';
 
-        if (strcmp(buf, "https") == 0) {
+        if (strcmp(buf, OSSL_HTTPS_NAME) == 0) {
             if (pssl != NULL)
                 *pssl = 1;
-            port = "443";
-        } else if (strcmp(buf, "http") != 0) {
+            port = OSSL_HTTPS_PORT;
+        } else if (strcmp(buf, OSSL_HTTP_NAME) != 0) {
             goto parse_err;
         }
 
@@ -119,13 +123,21 @@ int OSSL_HTTP_parse_url(const char *url, char **phost, char **pport,
 
 int http_use_proxy(const char *no_proxy, const char *server)
 {
-    size_t sl = strlen(server);
+    size_t sl;
     const char *found = NULL;
 
+    if (!ossl_assert(server != NULL))
+        return 0;
+    sl = strlen(server);
+
+    /*
+     * using environment variable names, both lowercase and uppercase variants,
+     * compatible with other HTTP client implementations like wget, curl and git
+     */
     if (no_proxy == NULL)
         no_proxy = getenv("no_proxy");
     if (no_proxy == NULL)
-        no_proxy = getenv("NO_PROXY");
+        no_proxy = getenv(OPENSSL_NO_PROXY);
     if (no_proxy != NULL)
         found = strstr(no_proxy, server);
     while (found != NULL
@@ -138,17 +150,28 @@ int http_use_proxy(const char *no_proxy, const char *server)
 const char *http_adapt_proxy(const char *proxy, const char *no_proxy,
                              const char *server, int use_ssl)
 {
-    int prefix_len = strlen(HTTP_URL_PREFIX);
+    const int http_len = strlen(OSSL_HTTP_PREFIX);
+    const int https_len = strlen(OSSL_HTTPS_PREFIX);
 
+    /*
+     * using environment variable names, both lowercase and uppercase variants,
+     * compatible with other HTTP client implementations like wget, curl and git
+     */
     if (proxy == NULL)
         proxy = getenv(use_ssl ? "https_proxy" : "http_proxy");
     if (proxy == NULL)
-        proxy = getenv(use_ssl ? "HTTPS_PROXY" : "HTTP_PROXY");
-    if (proxy != NULL && strncmp(proxy, HTTP_URL_PREFIX, prefix_len) == 0)
-        proxy += prefix_len; /* skip any leading "http://" */
-    if (proxy != NULL && *proxy == '\0')
-        proxy = NULL;
-    if (proxy != NULL && !http_use_proxy(no_proxy, server))
-        proxy = NULL;
+        proxy = getenv(use_ssl ? OPENSSL_HTTP_PROXY :
+                       OPENSSL_HTTPS_PROXY);
+    if (proxy == NULL)
+        return NULL;
+
+    /* skip any leading "http://" or "https://" */
+    if (strncmp(proxy, OSSL_HTTP_PREFIX, http_len) == 0)
+        proxy += http_len;
+    else if (strncmp(proxy, OSSL_HTTPS_PREFIX, https_len) == 0)
+        proxy += https_len;
+
+    if (*proxy == '\0' || !http_use_proxy(no_proxy, server))
+        return NULL;
     return proxy;
 }

crypto/http/http_local.h

@@ -27,8 +27,6 @@ typedef OCSP_REQ_CTX OSSL_HTTP_REQ_CTX;
 # define OSSL_HTTP_REQ_CTX_get0_mem_bio OCSP_REQ_CTX_get0_mem_bio /* undoc'd */
 # define OSSL_HTTP_REQ_CTX_set_max_response_length OCSP_set_max_response_length
 
-# define HTTP_URL_PREFIX "http://"
-
 BIO *HTTP_asn1_item2bio(const ASN1_ITEM *it, ASN1_VALUE *val);
 OSSL_HTTP_REQ_CTX *HTTP_REQ_CTX_new(BIO *wbio, BIO *rbio, int use_http_proxy,
                                     const char *server, const char *port,