[PATCH v2 1/4] Build system: Replace git:// and http:// with https://

Demi Marie Obenour posted 4 patches 2 years, 12 months ago
There is a newer version of this series
[PATCH v2 1/4] Build system: Replace git:// and http:// with https://
Posted by Demi Marie Obenour 2 years, 12 months ago
Obtaining code over an insecure transport is a terrible idea for
blatently obvious reasons.  Even for non-executable data, insecure
transports are considered deprecated.

This patch enforces the use of secure transports in the build system.

Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
---
 Config.mk                         | 20 ++++++--------------
 docs/README.remus                 |  2 +-
 docs/conf.py                      |  2 +-
 scripts/get_maintainer.pl         |  2 +-
 stubdom/configure                 | 18 +++++++++---------
 stubdom/configure.ac              | 24 +++++++++++++++---------
 tools/firmware/etherboot/Makefile |  6 +-----
 7 files changed, 34 insertions(+), 40 deletions(-)

diff --git a/Config.mk b/Config.mk
index 10eb443b17d85381b2d1e2282f8965c3e99767e0..b2bef45b059976d5a6320eabada6073004eb22ee 100644
--- a/Config.mk
+++ b/Config.mk
@@ -191,7 +191,7 @@ APPEND_CFLAGS += $(foreach i, $(APPEND_INCLUDES), -I$(i))
 EMBEDDED_EXTRA_CFLAGS := -fno-pie -fno-stack-protector -fno-stack-protector-all
 EMBEDDED_EXTRA_CFLAGS += -fno-exceptions -fno-asynchronous-unwind-tables
 
-XEN_EXTFILES_URL ?= http://xenbits.xen.org/xen-extfiles
+XEN_EXTFILES_URL ?= https://xenbits.xen.org/xen-extfiles
 # All the files at that location were downloaded from elsewhere on
 # the internet.  The original download URL is preserved as a comment
 # near the place in the Xen Makefiles where the file is used.
@@ -215,19 +215,11 @@ ifneq (,$(QEMU_TAG))
 QEMU_TRADITIONAL_REVISION ?= $(QEMU_TAG)
 endif
 
-ifeq ($(GIT_HTTP),y)
-OVMF_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/ovmf.git
-QEMU_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/qemu-xen.git
-QEMU_TRADITIONAL_URL ?= http://xenbits.xen.org/git-http/qemu-xen-traditional.git
-SEABIOS_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/seabios.git
-MINIOS_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/mini-os.git
-else
-OVMF_UPSTREAM_URL ?= git://xenbits.xen.org/ovmf.git
-QEMU_UPSTREAM_URL ?= git://xenbits.xen.org/qemu-xen.git
-QEMU_TRADITIONAL_URL ?= git://xenbits.xen.org/qemu-xen-traditional.git
-SEABIOS_UPSTREAM_URL ?= git://xenbits.xen.org/seabios.git
-MINIOS_UPSTREAM_URL ?= git://xenbits.xen.org/mini-os.git
-endif
+OVMF_UPSTREAM_URL ?= https://xenbits.xen.org/git-http/ovmf.git
+QEMU_UPSTREAM_URL ?= https://xenbits.xen.org/git-http/qemu-xen.git
+QEMU_TRADITIONAL_URL ?= https://xenbits.xen.org/git-http/qemu-xen-traditional.git
+SEABIOS_UPSTREAM_URL ?= https://xenbits.xen.org/git-http/seabios.git
+MINIOS_UPSTREAM_URL ?= https://xenbits.xen.org/git-http/mini-os.git
 OVMF_UPSTREAM_REVISION ?= 7b4a99be8a39c12d3a7fc4b8db9f0eab4ac688d5
 QEMU_UPSTREAM_REVISION ?= master
 MINIOS_UPSTREAM_REVISION ?= 5bcb28aaeba1c2506a82fab0cdad0201cd9b54b3
diff --git a/docs/README.remus b/docs/README.remus
index e41e045a109466213b39bf5099ee16652b229ccc..10929e06d049755c4e8a9c900af7e10048c3effb 100644
--- a/docs/README.remus
+++ b/docs/README.remus
@@ -7,7 +7,7 @@ Using Remus with libxl on Xen 4.5 and higher:
  To enable network buffering, you need libnl 3.2.8
  or higher along with the development headers and command line utilities.
  If your distro does not have the appropriate libnl3 version, you can find
- the latest source tarball of libnl3 at http://www.carisma.slowglass.com/~tgr/libnl/
+ the latest source tarball of libnl3 at https://www.infradead.org/~tgr/libnl/
 
 Disk replication:
  VMs protected by Remus need to use DRBD based disk backends. Specifically, you
diff --git a/docs/conf.py b/docs/conf.py
index 50e41501db8f95bd186818c49a8e6538d733012b..7f4adce29e57e4ab8be9a09fc105bb133c51dbb0 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -5,7 +5,7 @@
 #
 # This file does only contain a selection of the most common options. For a
 # full list see the documentation:
-# http://www.sphinx-doc.org/en/master/config
+# https://www.sphinx-doc.org/en/master/config
 
 # -- Path setup --------------------------------------------------------------
 
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 48e07370e8d462ced70a1de13ec8134b4eed65ba..cf629cdf3c44e4abe67214378c49a3a9d858d9b5 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -1457,7 +1457,7 @@ sub vcs_exists {
 	warn("$P: No supported VCS found.  Add --nogit to options?\n");
 	warn("Using a git repository produces better results.\n");
 	warn("Try latest git repository using:\n");
-	warn("git clone git://xenbits.xen.org/xen.git\n");
+	warn("git clone https://xenbits.xen.org/git-http/xen.git\n");
 	$printed_novcs = 1;
     }
     return 0;
diff --git a/stubdom/configure b/stubdom/configure
index b8bffceafdd46181e26a79b85405aefb8bc3ff7d..8b409d294d6ad5e363d6942078e66de95fa0503d 100755
--- a/stubdom/configure
+++ b/stubdom/configure
@@ -3535,7 +3535,7 @@ if test "x$ZLIB_URL" = "x"; then :
 	if test "x$extfiles" = "xy"; then :
   ZLIB_URL=\$\(XEN_EXTFILES_URL\)
 else
-  ZLIB_URL="http://www.zlib.net"
+  ZLIB_URL="https://www.zlib.net"
 fi
 
 fi
@@ -3550,7 +3550,7 @@ if test "x$LIBPCI_URL" = "x"; then :
 	if test "x$extfiles" = "xy"; then :
   LIBPCI_URL=\$\(XEN_EXTFILES_URL\)
 else
-  LIBPCI_URL="http://www.kernel.org/pub/software/utils/pciutils"
+  LIBPCI_URL="https://www.kernel.org/pub/software/utils/pciutils"
 fi
 
 fi
@@ -3565,7 +3565,7 @@ if test "x$NEWLIB_URL" = "x"; then :
 	if test "x$extfiles" = "xy"; then :
   NEWLIB_URL=\$\(XEN_EXTFILES_URL\)
 else
-  NEWLIB_URL="ftp://sources.redhat.com/pub/newlib"
+  NEWLIB_URL="https://sources.redhat.com/pub/newlib"
 fi
 
 fi
@@ -3580,7 +3580,7 @@ if test "x$LWIP_URL" = "x"; then :
 	if test "x$extfiles" = "xy"; then :
   LWIP_URL=\$\(XEN_EXTFILES_URL\)
 else
-  LWIP_URL="http://download.savannah.gnu.org/releases/lwip"
+  LWIP_URL="https://download.savannah.gnu.org/releases/lwip"
 fi
 
 fi
@@ -3595,7 +3595,7 @@ if test "x$GRUB_URL" = "x"; then :
 	if test "x$extfiles" = "xy"; then :
   GRUB_URL=\$\(XEN_EXTFILES_URL\)
 else
-  GRUB_URL="http://alpha.gnu.org/gnu/grub"
+  GRUB_URL="https://alpha.gnu.org/gnu/grub"
 fi
 
 fi
@@ -3607,7 +3607,7 @@ GRUB_VERSION="0.97"
 
 if test "x$OCAML_URL" = "x"; then :
 
-	OCAML_URL="http://caml.inria.fr/pub/distrib/ocaml-4.02"
+	OCAML_URL="https://caml.inria.fr/pub/distrib/ocaml-4.02"
 
 fi
 OCAML_VERSION="4.02.0"
@@ -3621,7 +3621,7 @@ if test "x$GMP_URL" = "x"; then :
 	if test "x$extfiles" = "xy"; then :
   GMP_URL=\$\(XEN_EXTFILES_URL\)
 else
-  GMP_URL="ftp://ftp.gmplib.org/pub/gmp-4.3.2"
+  GMP_URL="https://gmplib.org/download/gmp"
 fi
 
 fi
@@ -3636,7 +3636,7 @@ if test "x$POLARSSL_URL" = "x"; then :
 	if test "x$extfiles" = "xy"; then :
   POLARSSL_URL=\$\(XEN_EXTFILES_URL\)
 else
-  POLARSSL_URL="http://polarssl.org/code/releases"
+  POLARSSL_URL="https://polarssl.org/code/releases"
 fi
 
 fi
@@ -3651,7 +3651,7 @@ if test "x$TPMEMU_URL" = "x"; then :
 	if test "x$extfiles" = "xy"; then :
   TPMEMU_URL=\$\(XEN_EXTFILES_URL\)
 else
-  TPMEMU_URL="http://download.berlios.de/tpm-emulator"
+  TPMEMU_URL="https://download.berlios.de/tpm-emulator"
 fi
 
 fi
diff --git a/stubdom/configure.ac b/stubdom/configure.ac
index e20d99edac0da88098f4806333edde9f31dbc1a7..e43853d45a5f652c05fe36f9171fba4c1b5863c0 100644
--- a/stubdom/configure.ac
+++ b/stubdom/configure.ac
@@ -55,19 +55,25 @@ AC_PROG_INSTALL
 AX_DEPENDS_PATH_PROG([vtpm], [CMAKE], [cmake])
 
 # Stubdom libraries version and url setup
-AX_STUBDOM_LIB([ZLIB], [zlib], [1.2.3], [http://www.zlib.net])
-AX_STUBDOM_LIB([LIBPCI], [libpci], [2.2.9], [http://www.kernel.org/pub/software/utils/pciutils])
-AX_STUBDOM_LIB([NEWLIB], [newlib], [1.16.0], [ftp://sources.redhat.com/pub/newlib])
-AX_STUBDOM_LIB([LWIP], [lwip], [1.3.0], [http://download.savannah.gnu.org/releases/lwip])
-AX_STUBDOM_LIB([GRUB], [grub], [0.97], [http://alpha.gnu.org/gnu/grub])
-AX_STUBDOM_LIB_NOEXT([OCAML], [ocaml], [4.02.0], [http://caml.inria.fr/pub/distrib/ocaml-4.02])
-AX_STUBDOM_LIB([GMP], [libgmp], [4.3.2], [ftp://ftp.gmplib.org/pub/gmp-4.3.2])
-AX_STUBDOM_LIB([POLARSSL], [polarssl], [1.1.4], [http://polarssl.org/code/releases])
-AX_STUBDOM_LIB([TPMEMU], [berlios tpm emulator], [0.7.4], [http://download.berlios.de/tpm-emulator])
+AX_STUBDOM_LIB([ZLIB], [zlib], [1.2.3], [https://www.zlib.net])
+AX_STUBDOM_LIB([LIBPCI], [libpci], [2.2.9], [https://www.kernel.org/pub/software/utils/pciutils])
+AX_STUBDOM_LIB([NEWLIB], [newlib], [1.16.0], [https://sourceware.org/ftp/newlib])
+AX_STUBDOM_LIB([LWIP], [lwip], [1.3.0], [https://download.savannah.gnu.org/releases/lwip])
+AX_STUBDOM_LIB([GRUB], [grub], [0.97], [https://alpha.gnu.org/gnu/grub])
+AX_STUBDOM_LIB_NOEXT([OCAML], [ocaml], [4.02.0], [https://caml.inria.fr/pub/distrib/ocaml-4.02])
+AX_STUBDOM_LIB([GMP], [libgmp], [4.3.2], [https://gmplib.org/download/gmp])
+AX_STUBDOM_LIB([POLARSSL], [polarssl], [1.1.4], [https://polarssl.org/code/releases])
+AX_STUBDOM_LIB([TPMEMU], [berlios tpm emulator], [0.7.4], [https://download.berlios.de/tpm-emulator])
 
 #These stubdoms should be enabled if the dependent one is
 AX_STUBDOM_AUTO_DEPENDS([vtpmmgr], [vtpm])
 
+if test "x$vtpm" != xn || test "x$vtpmmgr" != xn; then
+    if test "x$extfiles" != xy; then
+        AC_MSG_ERROR([Sources needed for the vTPM and vTPM manager stubdomains are no longer at their original URLs])
+    fi
+fi
+
 #Conditionally enable these stubdoms based on the presense of dependencies
 AX_STUBDOM_CONDITIONAL_FINISH([vtpm-stubdom], [vtpm])
 AX_STUBDOM_CONDITIONAL_FINISH([vtpmmgr-stubdom], [vtpmmgr])
diff --git a/tools/firmware/etherboot/Makefile b/tools/firmware/etherboot/Makefile
index 4bc3633ba3d67ff9f52a9cb7923afea73c861da9..f08b2c847b6535e5c28b6576445d02c2ac9551eb 100644
--- a/tools/firmware/etherboot/Makefile
+++ b/tools/firmware/etherboot/Makefile
@@ -4,11 +4,7 @@ XEN_ROOT = $(CURDIR)/../../..
 include $(XEN_ROOT)/tools/Rules.mk
 include Config
 
-ifeq ($(GIT_HTTP),y)
-IPXE_GIT_URL ?= http://git.ipxe.org/ipxe.git
-else
-IPXE_GIT_URL ?= git://git.ipxe.org/ipxe.git
-endif
+IPXE_GIT_URL ?= https://git.ipxe.org/ipxe.git
 
 # put an updated tar.gz on xenbits after changes to this variable
 IPXE_GIT_TAG := 3c040ad387099483102708bb1839110bc788cefb
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
Re: [PATCH v2 1/4] Build system: Replace git:// and http:// with https://
Posted by George Dunlap 2 years, 12 months ago
On Wed, Feb 8, 2023 at 8:58 PM Demi Marie Obenour <
demi@invisiblethingslab.com> wrote:

> Obtaining code over an insecure transport is a terrible idea for
> blatently obvious reasons.  Even for non-executable data, insecure
> transports are considered deprecated.
>
> This patch enforces the use of secure transports in the build system.
>
> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
>

Hey Demi,

Thanks for this series -- we definitely want the build system to use secure
transports when available.  Can you confirm that you've tested the "+s"
versions of all the URLs in this patch, and verified that they actually
work?

If you haven't, I realize that may be somewhat tedious, but I think it's
pretty important.  You should be able to automate  a lot of it using `curl
--head --fail`. [1]

 -George

[1]
https://stackoverflow.com/questions/12199059/how-to-check-if-an-url-exists-with-the-shell-and-probably-curl
Re: [PATCH v2 1/4] Build system: Replace git:// and http:// with https://
Posted by Demi Marie Obenour 2 years, 12 months ago
On Thu, Feb 09, 2023 at 02:01:52PM +0000, George Dunlap wrote:
> On Wed, Feb 8, 2023 at 8:58 PM Demi Marie Obenour <
> demi@invisiblethingslab.com> wrote:
> 
> > Obtaining code over an insecure transport is a terrible idea for
> > blatently obvious reasons.  Even for non-executable data, insecure
> > transports are considered deprecated.
> >
> > This patch enforces the use of secure transports in the build system.
> >
> > Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> >
> 
> Hey Demi,
> 
> Thanks for this series -- we definitely want the build system to use secure
> transports when available.  Can you confirm that you've tested the "+s"
> versions of all the URLs in this patch, and verified that they actually
> work?

I had not, but a subsequent review indicated that most do work.  The
exceptions are:

- Neither the PolarSSL nor TPM emulator links work, but the http://
  verison of these links is also broken.  I added an AC_MSG_ERROR to
  fail the TPM emulator build if they would be used, but a Xen committer
  will need to regenerate configure.

- the newlib url should be https://sourceware.org/ftp/newlib, not
  https://source.redhat.com/ftp/newlib.  This was changed in
  configure.ac but not in configure.

> If you haven't, I realize that may be somewhat tedious, but I think it's
> pretty important.  You should be able to automate  a lot of it using `curl
> --head --fail`. [1]

That does not work for the Xen git repositories, but those all do work.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
Re: [PATCH v2 1/4] Build system: Replace git:// and http:// with https://
Posted by Anthony PERARD 2 years, 12 months ago
On Thu, Feb 09, 2023 at 02:01:52PM +0000, George Dunlap wrote:
> On Wed, Feb 8, 2023 at 8:58 PM Demi Marie Obenour <
> demi@invisiblethingslab.com> wrote:
> 
> > Obtaining code over an insecure transport is a terrible idea for
> > blatently obvious reasons.  Even for non-executable data, insecure
> > transports are considered deprecated.
> >
> > This patch enforces the use of secure transports in the build system.
> >
> > Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> >
> 
> Hey Demi,
> 
> Thanks for this series -- we definitely want the build system to use secure
> transports when available.  Can you confirm that you've tested the "+s"
> versions of all the URLs in this patch, and verified that they actually
> work?

:'(   -> https://gitlab.com/xen-project/patchew/xen/-/pipelines/771746628/

Our GitLab tests are very unhappy with the switch to TLS. Too many
containers aren't recent enough, and don't have the right certificates
(Let's encrypt I guess).

I've only looked at two failures:
    ubuntu-focal-clang:
        fatal: unable to access 'https://xenbits.xen.org/git-http/qemu-xen.git/': server certificate verification failed. CAfile: none CRLfile: none
    ubuntu-xenial-gcc:
        ERROR: cannot verify xenbits.xen.org's certificate, issued by 'CN=R3,O=Let\'s Encrypt,C=US':

I'll try to have a look at updating those containers.

Cheers,

-- 
Anthony PERARD
Re: [PATCH v2 1/4] Build system: Replace git:// and http:// with https://
Posted by George Dunlap 2 years, 12 months ago
On Thu, Feb 9, 2023 at 3:05 PM Anthony PERARD <anthony.perard@citrix.com>
wrote:

> On Thu, Feb 09, 2023 at 02:01:52PM +0000, George Dunlap wrote:
> > On Wed, Feb 8, 2023 at 8:58 PM Demi Marie Obenour <
> > demi@invisiblethingslab.com> wrote:
> >
> > > Obtaining code over an insecure transport is a terrible idea for
> > > blatently obvious reasons.  Even for non-executable data, insecure
> > > transports are considered deprecated.
> > >
> > > This patch enforces the use of secure transports in the build system.
> > >
> > > Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> > >
> >
> > Hey Demi,
> >
> > Thanks for this series -- we definitely want the build system to use
> secure
> > transports when available.  Can you confirm that you've tested the "+s"
> > versions of all the URLs in this patch, and verified that they actually
> > work?
>
> :'(   -> https://gitlab.com/xen-project/patchew/xen/-/pipelines/771746628/
>
> Our GitLab tests are very unhappy with the switch to TLS. Too many
> containers aren't recent enough, and don't have the right certificates
> (Let's encrypt I guess).
>
> I've only looked at two failures:
>     ubuntu-focal-clang:
>         fatal: unable to access '
> https://xenbits.xen.org/git-http/qemu-xen.git/': server certificate
> verification failed. CAfile: none CRLfile: none
>     ubuntu-xenial-gcc:
>         ERROR: cannot verify xenbits.xen.org's certificate, issued by
> 'CN=R3,O=Let\'s Encrypt,C=US':
>
> I'll try to have a look at updating those containers.
>

Just to clarify: This isn't an argument against the patch; only perhaps an
argument to delay it being checked in until we get the containers fixed.

Another advantage of this patch may be that it will naturally prod us to
update the containers whenever the root certificates expire. :-D

 -George