Re: ftp(1) prints TLS and short read errors to stdout rather than stderr

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

Pontus Stenetorp <[email protected]> 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=3Dlocalhost' -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 w=
ell into 2027, which caused syspatch to run in an interactive shell but fe=
tch nothing and print nothing due to a perceived expired certificate.
> =

> From what I can tell, the relevant code goes back to the original SSL su=
pport implementation back in 2006:
> =

> 	https://cvsweb.openbsd.org/diff/src/usr.bin/ftp/fetch.c?rev=3D1.61&prev=
=3D1.60
> =

> The diff mirrors how errors are printed in the surrounding code. Feedbac=
k 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 =3D tls_client()) =3D=3D 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) !=3D 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) !=3D 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 =3D tls_handshake(tls);
>  		} while (ret =3D=3D TLS_WANT_POLLIN || ret =3D=3D TLS_WANT_POLLOUT);
>  		if (ret !=3D 0) {
> -			fprintf(ttyout, "TLS handshake failure: %s\n", tls_error(tls));
> +			warnx("TLS handshake failure: %s", tls_error(tls));
>  			goto cleanup_url_get;
>  		}
>  		fin =3D funopen(tls, stdio_tls_read_wrapper,
> @@ -1101,7 +1100,7 @@ noslash:
>  #endif /* !SMALL */
>  		filesize !=3D -1 && len =3D=3D 0 && bytes !=3D filesize) {
>  		if (verbose)
> -			fputs("Read short file.\n", ttyout);
> +			warnx("Read short file.");
>  		goto cleanup_url_get;
>  	}
> =