Index | Thread | Search

From:
"Theo de Raadt" <deraadt@openbsd.org>
Subject:
Re: ftp(1) prints TLS and short read errors to stdout rather than stderr
To:
Pontus Stenetorp <pontus@stenetorp.se>
Cc:
tech@openbsd.org
Date:
Tue, 28 Jul 2026 09:27:38 -0600

Download raw body.

Thread
Those fixes look good to me.  Anyone else want to comment?

Pontus Stenetorp <pontus@stenetorp.se> wrote:

> ftp(1) prints some of its errors to stdout, which may lead to confusion when used in scripts like syspatch(8).
> 
> 	$ ftp -Mo /dev/null https://expired.badssl.com; echo $?
> 	Trying 104.154.89.105...
> 	TLS handshake failure: certificate verification failed: certificate has expired
> 	1
> 
> 	$ ftp -Mo /dev/null https://expired.badssl.com > /dev/null; echo $?
> 	1
> 
> 
> 	$ openssl req -keyout /tmp/localhost.key -newkey rsa -nodes -out \
> 		/tmp/localhost.crt -subj '/CN=localhost' -x509
> 	$ printf 'HTTP/1.1 200 OK\nContent-Length:4711\n\n17' | nc -clN -C \
> 		/tmp/localhost.crt -K /tmp/localhost.key localhost 4443
> 
> 	$ ftp -Mo /dev/null -S dont https://localhost:4443; echo $?
> 	Trying 127.0.0.1...
> 	Requesting https://localhost:4443
> 	Read short file.
> 	1
> 
> 	$ ftp -Mo /dev/null -S dont https://localhost:4443 > /dev/null; echo $?
> 	1
> 
> Discovered by trying to run syspatch(8) on a machine with a Bios clock well into 2027, which caused syspatch to run in an interactive shell but fetch nothing and print nothing due to a perceived expired certificate.
> 
> From what I can tell, the relevant code goes back to the original SSL support implementation back in 2006:
> 
> 	https://cvsweb.openbsd.org/diff/src/usr.bin/ftp/fetch.c?rev=1.61&prev=1.60
> 
> The diff mirrors how errors are printed in the surrounding code. Feedback is welcome though as I am still rather new to OpenBSD hacking.
> 
> diff /usr/src
> path + /usr/src
> commit - 95a78e283c1363d0a2f44ed4baea4243f1abf975
> blob - 453f6fd269851e8f4023e726128821ff2b4a15c3
> file + usr.bin/ftp/fetch.c
> --- usr.bin/ftp/fetch.c
> +++ usr.bin/ftp/fetch.c
> @@ -644,23 +644,22 @@ noslash:
>  				errx(1, "Can't allocate memory for https host.");
>  		}
>  		if ((tls = tls_client()) == NULL) {
> -			fprintf(ttyout, "failed to create SSL client\n");
> +			warnx("failed to create SSL client");
>  			goto cleanup_url_get;
>  		}
>  		if (tls_configure(tls, tls_config) != 0) {
> -			fprintf(ttyout, "TLS configuration failure: %s\n",
> -			    tls_error(tls));
> +			warnx("TLS configuration failure: %s", tls_error(tls));
>  			goto cleanup_url_get;
>  		}
>  		if (tls_connect_socket(tls, fd, sslhost) != 0) {
> -			fprintf(ttyout, "TLS connect failure: %s\n", tls_error(tls));
> +			warnx("TLS connect failure: %s", tls_error(tls));
>  			goto cleanup_url_get;
>  		}
>  		do {
>  			ret = tls_handshake(tls);
>  		} while (ret == TLS_WANT_POLLIN || ret == TLS_WANT_POLLOUT);
>  		if (ret != 0) {
> -			fprintf(ttyout, "TLS handshake failure: %s\n", tls_error(tls));
> +			warnx("TLS handshake failure: %s", tls_error(tls));
>  			goto cleanup_url_get;
>  		}
>  		fin = funopen(tls, stdio_tls_read_wrapper,
> @@ -1101,7 +1100,7 @@ noslash:
>  #endif /* !SMALL */
>  		filesize != -1 && len == 0 && bytes != filesize) {
>  		if (verbose)
> -			fputs("Read short file.\n", ttyout);
> +			warnx("Read short file.");
>  		goto cleanup_url_get;
>  	}
>