Re: Cert validation in pkgin

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

> Date: Sat, 09 Dec 2023 07:39:55 -0500
> From: Greg Troxel <gdt%lexort.com@localhost>
> 
> > +.It Fl i
> > +Allow insecure transports (HTTP, FTP), not just HTTPS
> 
> That fails to explain that TLS validation is forced on for https without
> -i, and disabled with -i.   Easy to fix - the real issue is breaking
> something that the user configured.

Updated patch (along the lines of the updated pkg_add patch):

- If you specified http:// or ftp:// in repositories.conf, no change.

- If you specified https:// in repositories.conf, server must have
  valid cert and must not redirect to http/ftp.  Option `-i' allows
  insecure downloads, bypassing these rules, even if you used https://
  in repositories.conf.

New text of man page:

+.It Fl i
+Allow insecure downloads: bypass HTTPS certificate validation, allow
+HTTPS to redirect to HTTP/FTP

Better?

From e48b69ccfa53406ad01b3346ef64eb61490ec05c Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sat, 9 Dec 2023 03:30:45 +0000
Subject: [PATCH] Require valid certs for https:// repositories.

New `-i' option to allow insecure transport.

Requires libfetch>=2.40.
---
 README.md  |  6 +++++-
 download.c | 26 +++++++++++++++++++++++---
 main.c     | 13 +++++++++++--
 pkgin.1.in |  7 +++++--
 pkgin.h    |  2 ++
 5 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/README.md b/README.md
index 3a4c405..d3b8c0f 100644
--- a/README.md
+++ b/README.md
@@ -3,7 +3,7 @@ pkgin(1) -- A tool to manage pkgsrc binary packages.
 
 ## SYNOPSIS
 
-`pkgin` [`-dfFhpPvVyn`] [`-l` _limit_chars_] [`-c` _chroot_path_] [`-t` _log_file_] _command_ [package ...]
+`pkgin` [`-dfFhipPvVyn`] [`-l` _limit_chars_] [`-c` _chroot_path_] [`-t` _log_file_] _command_ [package ...]
 
 ## DESCRIPTION
 
@@ -28,6 +28,10 @@ The following command line arguments are supported:
   * `-h`:
     Displays help for the command
 
+  * `-i`:
+    Allow insecure downloads: bypass HTTPS certificate validation,
+    allow HTTPS to redirect to HTTP/FTP
+
   * `-l` _limit_chars_:
     Only include the packages with the specified [STATUS FLAGS][]
 
diff --git a/download.c b/download.c
index b212bb6..5c48c13 100644
--- a/download.c
+++ b/download.c
@@ -30,7 +30,26 @@
 #include "pkgin.h"
 #include "external/progressmeter.h"
 
-extern char fetchflags[3];
+static const char *
+urlfetchflags(const struct url *url)
+{
+
+	/*
+	 * For a package repository at http://... or ftp://..., enable
+	 * insecure transport to download it -- this way we don't break
+	 * existing setups that never expected secure transport in the
+	 * first place.
+	 *
+	 * This function is used both for the summary file and for the
+	 * package URLs, which are all constructed relative to a
+	 * repository URL.
+	 */
+	if (strcasecmp(url->scheme, SCHEME_HTTP) == 0 ||
+	    strcasecmp(url->scheme, SCHEME_FTP) == 0)
+		return insecurefetchflags;
+
+	return fetchflags;
+}
 
 /*
  * Open a pkg_summary and if newer than local return an open libfetch
@@ -46,7 +65,8 @@ sum_open(char *str_url, time_t *db_mtime)
 
 	url = fetchParseURL(str_url);
 
-	if (url == NULL || (f = fetchXGet(url, &st, fetchflags)) == NULL)
+	if (url == NULL ||
+	    (f = fetchXGet(url, &st, urlfetchflags(url))) == NULL)
 		goto nofetch;
 
 	if (st.size == -1) { /* could not obtain file size */
@@ -173,7 +193,7 @@ download_pkg(char *pkg_url, FILE *fp, int cur, int total)
 	if ((url = fetchParseURL(pkg_url)) == NULL)
 		errx(EXIT_FAILURE, "%s: parse failure", pkg_url);
 
-	if ((f = fetchXGet(url, &st, fetchflags)) == NULL) {
+	if ((f = fetchXGet(url, &st, urlfetchflags(url))) == NULL) {
 		fprintf(stderr, "download error: %s %s\n", pkg_url,
 		    fetchLastErrString);
 		fetchFreeURL(url);
diff --git a/main.c b/main.c
index a693bca..ddee207 100644
--- a/main.c
+++ b/main.c
@@ -39,8 +39,10 @@ static void	ginto(void);
 
 uint8_t		yesflag = 0, noflag = 0;
 uint8_t		verbosity = 0, package_version = 0, parsable = 0, pflag = 0;
+uint8_t		insecure_transport = 0;
 char		lslimit = '\0';
-char		fetchflags[4] = { 0, 0, 0, 0 };
+char		insecurefetchflags[5] = { 0, 0, 0, 0, 0 };
+char		fetchflags[6] = { 0, 0, 0, 0, 0, 0 };
 FILE  		*tracefp = NULL;
 
 int
@@ -59,7 +61,7 @@ main(int argc, char *argv[])
 	/* Default to not doing \r printouts if we don't send to a tty */
 	parsable = !isatty(fileno(stdout));
 
-	while ((ch = getopt(argc, argv, "46dhyfFPvVl:nc:t:p")) != -1) {
+	while ((ch = getopt(argc, argv, "46dhiyfFPvVl:nc:t:p")) != -1) {
 		switch (ch) {
 		case '4':
 			v4flag = 1;
@@ -67,6 +69,9 @@ main(int argc, char *argv[])
 		case '6':
 			v6flag = 1;
 			break;
+		case 'i':
+			insecure_transport = 1;
+			break;
 		case 'f':
 			force_update = 1;
 			break;
@@ -147,6 +152,10 @@ main(int argc, char *argv[])
 	if (verbosity) {
 		fetchflags[ffidx++] = 'v';
 	}
+	strlcpy(insecurefetchflags, fetchflags, sizeof(insecurefetchflags));
+	if (!insecure_transport) {
+		fetchflags[ffidx++] = 'V';
+	}
 
 	/* Configure pkg_install */
 	setup_pkg_install();
diff --git a/pkgin.1.in b/pkgin.1.in
index 1f38c79..2d98a1c 100644
--- a/pkgin.1.in
+++ b/pkgin.1.in
@@ -1,4 +1,4 @@
-.Dd July 1, 2020
+.Dd December 8, 2023
 .Dt PKGIN 1
 .Os
 .Sh NAME
@@ -6,7 +6,7 @@
 .Nd pkgsrc binary package manager
 .Sh SYNOPSIS
 .Nm
-.Op Fl 46dfhnPpVvy
+.Op Fl 46dfhinPpVvy
 .Op Fl c Ar chroot_path
 .Op Fl l Ar limit_chars
 .Op Fl t Ar log_file
@@ -42,6 +42,9 @@ Download only
 Force database update
 .It Fl h
 Displays help for the command
+.It Fl i
+Allow insecure downloads: bypass HTTPS certificate validation, allow
+HTTPS to redirect to HTTP/FTP
 .It Fl l Ar limit_chars
 Only include the packages with the specified
 .Dv STATUS FLAGS
diff --git a/pkgin.h b/pkgin.h
index 3730c93..b066715 100644
--- a/pkgin.h
+++ b/pkgin.h
@@ -353,6 +353,8 @@ extern int		r_plistcounter;
 extern Plisthead	l_plisthead[LOCAL_PKG_HASH_SIZE];
 extern Plisthead	r_plisthead[REMOTE_PKG_HASH_SIZE];
 extern FILE		*tracefp;
+extern char		fetchflags[];
+extern char		insecurefetchflags[];
 
 /* download.c*/
 Sumfile		*sum_open(char *, time_t *);