HTTP client: Correct the use of optional proxy URL and its documentation

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15104)

4 files changed, 48 insertions, 50 deletions

diff --git a/crypto/http/http_client.c b/crypto/http/http_client.c
index 9c2b593a2d..bf2e3b54c7 100644
--- a/crypto/http/http_client.c
+++ b/crypto/http/http_client.c
@@ -693,10 +693,11 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx)
 /* set up a new connection BIO, to HTTP server or to HTTP(S) proxy if given */
 static BIO *HTTP_new_bio(const char *server /* optionally includes ":port" */,
                          const char *server_port /* explicit server port */,
-                         const char *proxy /* optionally includes ":port" */)
+                         int use_ssl,
+                         const char *proxy /* optionally includes ":port" */,
+                         const char *proxy_port /* explicit proxy port */)
 {
-    const char *host = server, *host_end;
-    char host_name[100];
+    const char *host = server;
     const char *port = server_port;
     BIO *cbio;
 
@@ -705,20 +706,11 @@ static BIO *HTTP_new_bio(const char *server /* optionally includes ":port" */,
 
     if (proxy != NULL) {
         host = proxy;
-        port = NULL;
+        port = proxy_port;
     }
 
-    host_end = strchr(host, '/');
-    if (host_end != NULL) {
-        size_t host_len = host_end - host;
-
-        if (host_len < sizeof(host_name)) {
-            /* chop trailing string starting with '/' */
-            strncpy(host_name, host, host_len);
-            host_name[host_len] = '\0';
-            host = host_name;
-        }
-    }
+    if (port == NULL && strchr(host, ':') == NULL)
+        port = use_ssl ? OSSL_HTTPS_PORT : OSSL_HTTP_PORT;
 
     cbio = BIO_new_connect(host /* optionally includes ":port" */);
     if (cbio == NULL)
@@ -854,6 +846,8 @@ BIO *OSSL_HTTP_transfer(const char *server, const char *port, const char *path,
         cbio = bio;
     } else {
 #ifndef OPENSSL_NO_SOCK
+        char *proxy_host = NULL, *proxy_port = NULL;
+
         if (server == NULL) {
             ERR_raise(ERR_LIB_HTTP, ERR_R_PASSED_NULL_PARAMETER);
             return NULL;
@@ -863,7 +857,15 @@ BIO *OSSL_HTTP_transfer(const char *server, const char *port, const char *path,
         if (port == NULL && strchr(server, ':') == NULL)
             port = use_ssl ? OSSL_HTTPS_PORT : OSSL_HTTP_PORT;
         proxy = ossl_http_adapt_proxy(proxy, no_proxy, server, use_ssl);
-        if ((cbio = HTTP_new_bio(server, port, proxy)) == NULL)
+        if (proxy != NULL
+            && !OSSL_HTTP_parse_url(proxy, NULL /* use_ssl */, NULL /* user */,
+                                    &proxy_host, &proxy_port, NULL /* num */,
+                                    NULL /* path */, NULL, NULL))
+            return NULL;
+        cbio = HTTP_new_bio(server, port, use_ssl, proxy_host, proxy_port);
+        OPENSSL_free(proxy_host);
+        OPENSSL_free(proxy_port);
+        if (cbio == NULL)
             return NULL;
 #else
         ERR_raise(ERR_LIB_HTTP, HTTP_R_SOCK_NOT_SUPPORTED);
diff --git a/crypto/http/http_lib.c b/crypto/http/http_lib.c
index a8697cca33..2aa0736ac5 100644
--- a/crypto/http/http_lib.c
+++ b/crypto/http/http_lib.c
@@ -113,7 +113,7 @@ int OSSL_parse_url(const char *url, char **pscheme, char **puser, char **phost,
     /* remaining port spec handling is also done for the default values */
     /* make sure a decimal port number is given */
     if (!sscanf(port, "%u", &portnum) || portnum > 65535) {
-        ERR_raise(ERR_LIB_HTTP, HTTP_R_INVALID_PORT_NUMBER);
+        ERR_raise_data(ERR_LIB_HTTP, HTTP_R_INVALID_PORT_NUMBER, "%s", port);
         goto err;
     }
     for (port_end = port; '0' <= *port_end && *port_end <= '9'; port_end++)
@@ -240,6 +240,7 @@ int OSSL_HTTP_parse_url(const char *url, int *pssl, char **puser, char **phost,
     return 0;
 }
 
+/* Respect no_proxy, taking default value from environment variable(s) */
 int ossl_http_use_proxy(const char *no_proxy, const char *server)
 {
     size_t sl;
@@ -257,6 +258,7 @@ int ossl_http_use_proxy(const char *no_proxy, const char *server)
         no_proxy = getenv("no_proxy");
     if (no_proxy == NULL)
         no_proxy = getenv(OPENSSL_NO_PROXY);
+
     if (no_proxy != NULL)
         found = strstr(no_proxy, server);
     while (found != NULL
@@ -266,12 +268,10 @@ int ossl_http_use_proxy(const char *no_proxy, const char *server)
     return found == NULL;
 }
 
+/* Take default value from environment variable(s), respect no_proxy */
 const char *ossl_http_adapt_proxy(const char *proxy, const char *no_proxy,
                                   const char *server, int use_ssl)
 {
-    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
@@ -281,16 +281,9 @@ const char *ossl_http_adapt_proxy(const char *proxy, const char *no_proxy,
     if (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' || !ossl_http_use_proxy(no_proxy, server))
+    if (proxy == NULL || *proxy == '\0'
+            || !ossl_http_use_proxy(no_proxy, server))
         return NULL;
     return proxy;
 }
diff --git a/doc/man1/openssl-cmp.pod.in b/doc/man1/openssl-cmp.pod.in
index 8700d6bdcf..f27443ca9c 100644
--- a/doc/man1/openssl-cmp.pod.in
+++ b/doc/man1/openssl-cmp.pod.in
@@ -260,7 +260,7 @@ if any, or else the current client key, if given.
 Pass phrase source for the key given with the B<-newkey> option.
 If not given here, the password will be prompted for if needed.
 
-For more information about the format of B<arg> see
+For more information about the format of I<arg> see
 L<openssl-passphrase-options(1)>.
 
 =item B<-subject> I<name>
@@ -441,9 +441,10 @@ Reason numbers defined in RFC 5280 are:
 
 =item B<-server> I<[http[s]://][userinfo@]host[:port][/path][?query][#fragment]>
 
-The IP address or DNS hostname and optionally port (defaulting to 80 or 443)
+The IP address or DNS hostname and optionally port
 of the CMP server to connect to using HTTP(S) transport.
-The scheme I<https> may be given only if the B<tls_used> option is used.
+The scheme C<https> may be given only if the B<-tls_used> option is used.
+In this case the default port is 443, else 80.
 The optional userinfo and fragment components are ignored.
 Any given query component is handled as part of the path component.
 If a path is included it provides the default value for the B<-path> option.
@@ -453,12 +454,13 @@ If a path is included it provides the default value for the B<-path> option.
 HTTP path at the CMP server (aka CMP alias) to use for POST requests.
 Defaults to any path given with B<-server>, else C<"/">.
 
-=item B<-proxy> I<[http[s]://][userinfo@]host[:port] [/path][?query][#fragment]>
+=item B<-proxy> I<[http[s]://][userinfo@]host[:port][/path][?query][#fragment]>
 
-The HTTP(S) proxy server to use for reaching the CMP server unless B<no_proxy>
+The HTTP(S) proxy server to use for reaching the CMP server unless B<-no_proxy>
 applies, see below.
-The optional I<http://> or I<https://> prefix is ignored (note that TLS may be
-selected by B<tls_used>), as well as any path, userinfo, and query, and fragment
+The proxy port defaults to 80 or 443 if the scheme is C<https>; apart from that
+the optional C<http://> or C<https://> prefix is ignored (note that TLS may be
+selected by B<-tls_used>), as well as any path, userinfo, and query, and fragment
 components.
 Defaults to the environment variable C<http_proxy> if set, else C<HTTP_PROXY>
 in case no TLS is used, otherwise C<https_proxy> if set, else C<HTTPS_PROXY>.
@@ -635,7 +637,7 @@ and (as far as needed) for validating PBM-based protection of incoming messages.
 PBM stands for Password-Based Message Authentication Code.
 This takes precedence over the B<-cert> and B<-key> options.
 
-For more information about the format of B<arg> see
+For more information about the format of I<arg> see
 L<openssl-passphrase-options(1)>.
 
 =item B<-cert> I<filename>|I<uri>
@@ -684,7 +686,7 @@ Pass phrase source for the private key given with the B<-key> option.
 Also used for B<-cert> and B<-oldcert> in case it is an encrypted PKCS#12 file.
 If not given here, the password will be prompted for if needed.
 
-For more information about the format of B<arg> see
+For more information about the format of I<arg> see
 L<openssl-passphrase-options(1)>.
 
 =item B<-digest> I<name>
@@ -693,13 +695,13 @@ Specifies name of supported digest to use in RFC 4210's MSG_SIG_ALG
 and as the one-way function (OWF) in MSG_MAC_ALG.
 If applicable, this is used for message protection and
 Proof-of-Possession (POPO) signatures.
-To see the list of supported digests, use B<openssl list -digest-commands>.
+To see the list of supported digests, use C<openssl list -digest-commands>.
 Defaults to C<sha256>.
 
 =item B<-mac> I<name>
 
 Specifies the name of the MAC algorithm in MSG_MAC_ALG.
-To get the names of supported MAC algorithms use B<openssl list -mac-algorithms>
+To get the names of supported MAC algorithms use C<openssl list -mac-algorithms>
 and possibly combine such a name with the name of a supported digest algorithm,
 e.g., hmacWithSHA256.
 Defaults to C<hmac-sha1> as per RFC 4210.
@@ -742,7 +744,7 @@ B<-srv_trusted>, B<-srv_untrusted>, B<-rsp_extracerts>, B<-rsp_capubs>,
 B<-tls_extra>, and B<-tls_trusted> options.
 If not given here, the password will be prompted for if needed.
 
-For more information about the format of B<arg> see
+For more information about the format of I<arg> see
 L<openssl-passphrase-options(1)>.
 
 {- $OpenSSL::safe::opt_engine_item -}
@@ -800,11 +802,11 @@ Private key for the client's TLS certificate.
 
 =item B<-tls_keypass> I<arg>
 
-Pass phrase source for client's private TLS key B<tls_key>.
+Pass phrase source for client's private TLS key B<-tls_key>.
 Also used for B<-tls_cert> in case it is an encrypted PKCS#12 file.
 If not given here, the password will be prompted for if needed.
 
-For more information about the format of B<arg> see
+For more information about the format of I<arg> see
 L<openssl-passphrase-options(1)>.
 
 =item B<-tls_extra> I<filenames>|I<uris>
@@ -1061,7 +1063,7 @@ It can be viewed using, e.g.,
   openssl x509 -noout -text -in insta.cert.pem
 
 In case the network setup requires using an HTTP proxy it may be given as usual
-via the environment variable B<http_proxy> or via the B<proxy> option in the
+via the environment variable B<http_proxy> or via the B<-proxy> option in the
 configuration file or the CMP command-line argument B<-proxy>, for example
 
   -proxy http://192.168.1.1:8080
@@ -1108,7 +1110,7 @@ Many more options can be given in the configuration file
 and/or on the command line.
 For instance, the B<-reqexts> CLI option may refer to a section in the
 configuration file defining X.509 extensions to use in certificate requests,
-such as B<v3_req> in F<openssl/apps/openssl.cnf>:
+such as C<v3_req> in F<openssl/apps/openssl.cnf>:
 
   openssl cmp -section insta,cr -reqexts v3_req
 
@@ -1165,7 +1167,7 @@ For CMP client invocations, in particular for certificate enrollment,
 usually many parameters need to be set, which is tedious and error-prone to do
 on the command line.
 Therefore, the client offers the possibility to read
-options from sections of the OpenSSL config file, usually called B<openssl.cnf>.
+options from sections of the OpenSSL config file, usually called F<openssl.cnf>.
 The values found there can still be extended and even overridden by any
 subsequently loaded sections and on the command line.
 
diff --git a/doc/man3/OSSL_HTTP_transfer.pod b/doc/man3/OSSL_HTTP_transfer.pod
index 7de213670d..d2ff8eeebc 100644
--- a/doc/man3/OSSL_HTTP_transfer.pod
+++ b/doc/man3/OSSL_HTTP_transfer.pod
@@ -104,14 +104,15 @@ I<bio> is used for writing the request, and I<rbio> for reading the response.
 As soon as the client has flushed I<bio> the server must be ready to provide
 a response or indicate a waiting condition via I<rbio>.
 
-The optional I<proxy> parameter can be used to set the address of the an
+If I<bio> is NULL the optional I<proxy> parameter can be used to set an
 HTTP(S) proxy to use (unless overridden by "no_proxy" settings).
 If TLS is not used this defaults to the environment variable C<http_proxy>
 if set, else C<HTTP_PROXY>.
 If I<use_ssl> != 0 it defaults to C<https_proxy> if set, else C<HTTPS_PROXY>.
-An empty proxy string specifies not to use a proxy.
-Else the format is C<[http[s]://]address[:port][/path]>,
-where any path given is ignored.
+An empty proxy string C<""> forbids using a proxy.
+Else the format is
+C<[http[s]://][userinfo@]host[:port][/path][?query][#fragment]>,
+where any userinfo, path, query, and fragment given is ignored.
 The default proxy port number is 80, or 443 in case "https:" is given.
 The HTTP client functions connect via the given proxy unless the I<server>
 is found in the optional list I<no_proxy> of proxy hostnames (if not NULL;