The problem of Busybox wget not supporting TLS SNI has come up a couple
times on the Tomato firmware board on linksysinfo.org. This impacts
sites like CloudFlare who are very strict about what SSL and TLS
parameters they require.
Below is a patch against master that rectifies this. It should be easy
to backport to 1_{23,24,25}_stable.
I should note that my patch trumps the one sent on 2015/10/23 here:
http://lists.busybox.net/pipermail/busybox/2015-October/083510.html
That patch blindly violates RFC 6066 by blindly passing on whatever the
"host" argument is into -servername. The "host" argument can (will)
includes such values as ip, ip:port, and hostname:port. RFC 6066 is
very clear that the only allowed servername value permitted is a
string/hostname (i.e. only an FQDN).
And regarding the additional patch from the same individual:
http://lists.busybox.net/pipermail/busybox/2015-October/083509.html
That patch assumes the OpenSSL library on the client machine has a
properly configured openssl.cnf as well as a full CA root list (many
embedded devices do not). This is a precarious situation and not always
warranted. If this is to be done, then a --no-check-certificates flag
must be added so that it can be disabled.
--
| Jeremy Chadwick [email protected] |
| UNIX Systems Administrator http://jdc.koitsu.org/ |
| Making life hard for others since 1977. PGP 4BD6C0CB |
=== SNIP ===
diff --git a/networking/wget.c b/networking/wget.c
index 37950ed..b7fcc5c 100644
--- a/networking/wget.c
+++ b/networking/wget.c
@@ -62,15 +62,22 @@
//config: a helper program to talk over HTTPS.
//config:
//config: OpenSSL has a simple SSL client for debug purposes.
-//config: If you select "openssl" helper, wget will effectively call
-//config: "openssl s_client -quiet -connect IP:443 2>/dev/null"
-//config: and pipe its data through it.
+//config: If you select "openssl" helper, wget will effectively run:
+//config: "openssl s_client -quiet -connect hostname:443
+//config: -servername hostname 2>/dev/null" and pipe its data
+//config: through it. (-servername should only be used with FQDNs,
+//config: not IP addresses.)
//config: Note inconvenient API: host resolution is done twice,
//config: and there is no guarantee openssl's idea of IPv6 address
//config: format is the same as ours.
//config: Another problem is that s_client prints debug information
//config: to stderr, and it needs to be suppressed. This means
//config: all error messages get suppressed too.
+//config: Also, if the s_client subcommand is not compiled in to
+//config: the system's openssl binary (i.e. subcommand fails), or
+//config: certain subcommand flags aren't available/supported,
+//config: openssl returns exit code 0, making it difficult to
+//config: detect a true error/failure.
//config: openssl is also a big binary, often dynamically linked
//config: against ~15 libraries.
//config:
@@ -349,6 +356,25 @@ static void set_alarm(void)
# define clear_alarm() ((void)0)
#endif
+/*
+ * is_ip_address() attempts to verify whether or not a string
+ * contains an IPv4 or IPv6 address (vs. an FQDN). The result
+ * of inet_pton() can be used to determine this.
+ *
+ * TODO add proper error checking when inet_pton() returns -1
+ * (some form of system error has occurred, and errno is set)
+ */
+static int is_ip_address(const char *string)
+{
+ struct sockaddr_in sa;
+ int result = inet_pton(AF_INET, string, &(sa.sin_addr));
+
+ if (ENABLE_FEATURE_IPV6 && result != 0) {
+ result = inet_pton(AF_INET6, string, &(sa.sin_addr));
+ }
+ return (result != 0);
+}
+
static FILE *open_socket(len_and_sockaddr *lsa)
{
int fd;
@@ -629,6 +655,8 @@ static FILE* prepare_ftp_session(FILE **dfpp, struct
host_info *target, len_and_
static int spawn_https_helper_openssl(const char *host, unsigned port)
{
char *allocated = NULL;
+ char *servername;
+ char *colon_ptr;
int sp[2];
int pid;
IF_FEATURE_WGET_SSL_HELPER(volatile int child_failed = 0;)
@@ -637,15 +665,39 @@ static int spawn_https_helper_openssl(const char *host,
unsigned port)
/* Kernel can have AF_UNIX support disabled */
bb_perror_msg_and_die("socketpair");
+ /*
+ * Per RFC 6066 Section 3, the only permitted values in the
+ * TLS server_name (SNI) field are FQDNs (DNS hostnames).
+ * IPv4 nor IPv6 addresses are supported, nor is use of an
+ * alternate port number.
+ *
+ * The host argument to spawn_https_helper_openssl() can
+ * contain things like: test.com, test.com:8080, 1.2.3.4, or
+ * 1.2.3.4:8888. The strchr() check determines if the user
+ * passed something like test.com:8080 or 1.2.3.4:8888.
+ *
+ * The rest of the code ensures that servername only gets
+ * set to the hostname portion, and that the -servername
+ * argument to openssl s_client is only used if the hostname
+ * itself is an actual FQDN, not an IPv4/IPv6 address.
+ */
if (!strchr(host, ':'))
host = allocated = xasprintf("%s:%u", host, port);
+ servername = xstrdup(host);
+ colon_ptr = strrchr(servername, ':');
+ *colon_ptr = '\0';
+
+ if (is_ip_address(servername))
+ servername = NULL;
+
fflush_all();
pid = xvfork();
if (pid == 0) {
/* Child */
- char *argv[6];
+ char *argv[8];
+ memset(&argv, 0, sizeof(argv));
close(sp[0]);
xmove_fd(sp[1], 0);
xdup2(0, 1);
@@ -661,7 +713,12 @@ static int spawn_https_helper_openssl(const char *host,
unsigned port)
argv[2] = (char*)"-quiet";
argv[3] = (char*)"-connect";
argv[4] = (char*)host;
- argv[5] = NULL;
+
+ if (servername) {
+ argv[5] = (char*)"-servername";
+ argv[6] = (char*)servername;
+ }
+
BB_EXECVP(argv[0], argv);
xmove_fd(3, 2);
# if ENABLE_FEATURE_WGET_SSL_HELPER
@@ -674,6 +731,7 @@ static int spawn_https_helper_openssl(const char *host,
unsigned port)
}
/* Parent */
+ free(servername);
free(allocated);
close(sp[1]);
# if ENABLE_FEATURE_WGET_SSL_HELPER
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
