Re: Cert validation in pkg_add

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

> Date: Fri, 22 Dec 2023 08:01:07 -0500
> From: Greg Troxel <gdt%lexort.com@localhost>
> 
> Taylor R Campbell <riastradh%NetBSD.org@localhost> writes:
> 
> > 2. when validation is on, pkg_add refuses to let https server
> >    downgrade security by redirect to non-https (http, ftp)
> 
> Nope.  That's not supported in any standard that I have found.

What standard prescribes the behaviour transport authentication in
automated software installation and update?

I don't think the answer here is going to be spelled out in any IETF
standards-track RFC -- this isn't a matter of appeal to authority of a
standards body; this is a matter of making reasonable security (and
usability) decisions for NetBSD and pkgsrc users to rely on in
automation.

>                                                                 If the
> server does that, it's a bug in the server, but that's no different than
> serving malware.  The user has chosen to trust the server to provide
> bits, and there is no reason to second guess that.

The server could serve malware, yes, but it is much easier to
accidentally redirect to http than to accidentally serve malware --
and that accidental misconfiguration can be exploited by an adversary
on the network with no access to the server.

For example, cdn.netbsd.org does this right now:

% curl --head https://cdn.netbsd.org/pub/NetBSD-daily/
HTTP/1.1 301 Document Moved
Connection: keep-alive
Server: bozohttpd/20210227
Content-Type: text/html
Location: http://nycdn.netbsd.org/pub/NetBSD-daily/

And from what I recall, fixing it is not trivial for various
compatibility reasons that might not go away until NetBSD 9 is
desupported.  (But don't quote me on that -- I haven't reviewed the
admins tickets related to this and I'm just going off vague memory of
an IRC discussion some months ago.)

It is a mistake for pkg_add not to authenticate every byte it receives
over the network by default -- it would be a surprising failure of
security expectations for pkg_add to fail to do that.  Unlike an
interactive web browser, pkg_add acts in very exciting ways on the
bytes it receives without giving the user a chance to review the
transport security.

And as I said early on, there is a huge attack surface between the
transport layer and signed packages (not to mention that our system of
signed packages needs to be overhauled with a different approach --
not just as a matter of NIH but as a matter of manageability and
meaningful security properties, to be discussed later).

So even if we deploy a system to authenticate package repositories
from the origin, it is still important to provide a clear security
contract about the transport layer: every byte over the network should
be authenticated.

> > 4. no change if you configure http:// or ftp://
> >    => This implies: when http:// is configured, even if http server
> >       attempts security upgrade by redirecting to https, pkg_add
> >       doesn't bother validation (maybe revisit later)
> 
> I don't think this makes sense.  We are having this entire discussion
> because of a belief that the standards say, or community norms say, that
> https (TLS really) should do pkix validation on the client side of
> server certs.  If someone configures an http URL, that should be used to
> fetch, and if the server sends a redirect to https, then that fetch
> should use the configured validation settings (defaulting to validating,
> unless HTTPS_VALIDATE_CERTS=no, eventually).

We can do that, but I just want to be clear that this means we might
break existing setups that never asked for https in the first place --
that this can change the behaviour of _any_ installation, not just
depending on its local configuration, but depending on the behaviour
of remote servers on the internet.


The attached patch is much smaller, but

(a) it fails to guarantee that it has authenticated every byte over
    the network even if you ask for `pkg_add https://...' without any
    security overrides; and

(b) it is more likely to incur wider fallout because it changes the
    semantics of the library, and the conditions that make it behave
    differently depend not just on the local configuration but also on
    the behaviour of remote servers.

(For this patch, I used the environment variable SSL_NO_VERIFY_PEER to
match what FreeBSD libfetch does, rather than a pkg_install.conf
variable, in order to reduce the number of variations floating around
out there.  Should maybe also support SSL_CA_CERT_FILE/PATH and
SSL_CRL_FILE for compatibility but that can be future work, not a high
priority.)

If that's really what you want, we can commit it instead.

>From ee1977b2a9d6a3e0f9a78f9fea50523df2079a6d Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sat, 9 Dec 2023 03:39:31 +0000
Subject: [PATCH] net/libfetch: update to 2.40

Validate HTTPS by default, unless environment variable
SSL_NO_VERIFY_PEER is set.

NOTE: This changes the semantics of the library in ways that may
break the functionality of existing callers, even callers that don't
ask to fetch HTTPS URLs.
---
 net/libfetch/Makefile       | 3 +--
 net/libfetch/files/common.c | 4 ++++
 net/libfetch/files/fetch.3  | 6 +++++-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/libfetch/Makefile b/net/libfetch/Makefile
index e5cea0f4b830..66d034e66a9c 100644
--- a/net/libfetch/Makefile
+++ b/net/libfetch/Makefile
@@ -1,7 +1,6 @@
 # $NetBSD: Makefile,v 1.64 2023/10/24 22:10:22 wiz Exp $
 
-DISTNAME=	libfetch-2.39
-PKGREVISION=	2
+DISTNAME=	libfetch-2.40
 CATEGORIES=	net
 MASTER_SITES=	# empty
 DISTFILES=	# empty
diff --git a/net/libfetch/files/common.c b/net/libfetch/files/common.c
index c1e158711636..bcb418d29918 100644
--- a/net/libfetch/files/common.c
+++ b/net/libfetch/files/common.c
@@ -451,6 +451,10 @@ fetch_ssl(conn_t *conn, const struct url *URL, int verbose)
 	conn->ssl_meth = SSLv23_client_method();
 	conn->ssl_ctx = SSL_CTX_new(conn->ssl_meth);
 	SSL_CTX_set_mode(conn->ssl_ctx, SSL_MODE_AUTO_RETRY);
+	if (getenv("SSL_NO_VERIFY_PEER") == NULL) {
+		SSL_CTX_set_default_verify_paths(conn->ssl_ctx);
+		SSL_CTX_set_verify(conn->ssl_ctx, SSL_VERIFY_PEER, NULL);
+	}
 
 	conn->ssl = SSL_new(conn->ssl_ctx);
 	if (conn->ssl == NULL){
diff --git a/net/libfetch/files/fetch.3 b/net/libfetch/files/fetch.3
index bb58071dd088..45899eb3ee19 100644
--- a/net/libfetch/files/fetch.3
+++ b/net/libfetch/files/fetch.3
@@ -27,7 +27,7 @@
 .\" $FreeBSD: fetch.3,v 1.64 2007/12/18 11:03:26 des Exp $
 .\" $NetBSD: fetch.3,v 1.17 2016/05/31 18:02:36 abhinav Exp $
 .\"
-.Dd January 22, 2010
+.Dd December 22, 2023
 .Dt FETCH 3
 .Os
 .Sh NAME
@@ -638,6 +638,10 @@ which proxies should not be used.
 Same as
 .Ev NO_PROXY ,
 for compatibility.
+.It Ev SSL_NO_VERIFY_PEER
+If defined,
+.Nm
+will skip validating certificates when fetching HTTPS URLs.
 .El
 .Sh EXAMPLES
 To access a proxy server on