Re: [PATCH] Replace git:// and http:// with https://

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Mon, Feb 06, 2023 at 10:10:33PM -0500, Demi Marie Obenour 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 was created by doing a tree-wide search and replace with sed,
> then reverting changes that were pointless or wrong.
> 
> Signed-off-by: Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx>

Hi,

Thanks for the patch.

Could you split this patch into several patches? I think that would make
it easier to review. Not all URL are equal, or have the same purpose.
They need to be check one by one to make sure that the right change to
do.

I propose at least 3 patches:

> ---

"automation / CI" changes:
>  automation/build/centos/CentOS-7.2.repo      |  8 ++++----
>  automation/build/debian/stretch-llvm-8.list  |  4 ++--
>  automation/build/debian/unstable-llvm-8.list |  4 ++--
>  automation/scripts/qemu-smoke-dom0-arm32.sh  |  2 +-

"build system" changes:
>  Config.mk                                    | 20 ++++++--------------
>  stubdom/configure                            | 18 +++++++++---------
>  stubdom/configure.ac                         | 18 +++++++++---------
>  tools/firmware/etherboot/Makefile            |  4 ++--


"miscellaneous and documentation":
>  README                                       |  4 ++--
>  docs/README.remus                            |  2 +-
>  docs/conf.py                                 |  2 +-
>  scripts/get_maintainer.pl                    |  2 +-
>  stubdom/grub.patches/10graphics.diff         |  2 +-
>  tools/examples/xeninfo.pl                    |  2 +-
>  tools/firmware/etherboot/README              |  2 +-
>  tools/firmware/hvmloader/pci_regs.h          |  2 +-
>  tools/firmware/hvmloader/pir.c               |  2 +-
>  tools/firmware/hvmloader/pir_types.h         |  2 +-
>  tools/firmware/hvmloader/smbios_types.h      |  2 +-
>  19 files changed, 47 insertions(+), 55 deletions(-)

In or after the "build system" patch, you could propose to remove
GIT_HTTP variable, and only use https:// urls. A separate change could
be better as we could easily revert it if that an issue.
We could start with a patch to default to GIT_HTTP=y, even if all urls
would bu https.


> diff --git a/automation/scripts/qemu-smoke-dom0-arm32.sh 
> b/automation/scripts/qemu-smoke-dom0-arm32.sh
> index 
> 98e4d481f65c2b29ac935ddf6247132ddf94fa1d..22ec2f2474ed3d1f1f614085f9bd832be4d42496
>  100755
> --- a/automation/scripts/qemu-smoke-dom0-arm32.sh
> +++ b/automation/scripts/qemu-smoke-dom0-arm32.sh
> @@ -4,7 +4,7 @@ set -ex
>  
>  cd binaries
>  # Use the kernel from Debian
> -curl --fail --silent --show-error --location --output vmlinuz 
> http://http.us.debian.org/debian/dists/bullseye/main/installer-armhf/current/images/netboot/vmlinuz
> +curl --fail --silent --show-error --location --output vmlinuz 
> https://http.us.debian.org/debian/dists/bullseye/main/installer-armhf/current/images/netboot/vmlinuz

This url change is wrong, the certificate provided aren't for debian.org.

>  # Use a tiny initrd based on busybox from Alpine Linux
>  curl --fail --silent --show-error --location --output initrd.tar.gz 
> https://dl-cdn.alpinelinux.org/alpine/v3.15/releases/armhf/alpine-minirootfs-3.15.1-armhf.tar.gz
>  
> diff --git a/docs/README.remus b/docs/README.remus
> index 
> e41e045a109466213b39bf5099ee16652b229ccc..5e411addb1a63f1b2c01ff165fc6f74d6d81c8ea
>  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.carisma.slowglass.com/~tgr/libnl/

This url doesn't exist anymore, so better not change it, or fix the
comment.

>  
>  Disk replication:
>   VMs protected by Remus need to use DRBD based disk backends. Specifically, 
> you
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 
> 48e07370e8d462ced70a1de13ec8134b4eed65ba..9f32e0e0c0ec5c491209d6e73232b5a388b0a588
>  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/xen.git\n";);

This url is wrong, xenbits provides git trees on http via a different path, see 
in Config.mk.

>       $printed_novcs = 1;
>      }
>      return 0;
> diff --git a/tools/examples/xeninfo.pl b/tools/examples/xeninfo.pl
> index 
> 95a15d9595c0f4bf09c9429099bd3b27e98ebf3e..1815d1d9f219799a83fbc0329a4a1a010264893e
>  100644
> --- a/tools/examples/xeninfo.pl
> +++ b/tools/examples/xeninfo.pl
> @@ -36,7 +36,7 @@ sub apiconnect
>  {
>       foreach my $xenhost (keys %xenhosts)
>       {
> -             my $xen = 
> RPC::XML::Client->new("http://$xenhost:$xenhosts{$xenhost}{'port'}");
> +             my $xen = 
> RPC::XML::Client->new("https://$xenhost:$xenhosts{$xenhost}{'port'}");

I'm pretty sure that the wrong change to do here.

>               my $session = 
> $xen->simple_request("session.login_with_password", "user","");
>               if (! $session)
>               {

I haven't check every urls, there's  probably other that want to be
change to something else rather that just s/http/https/.

Cheers,

-- 
Anthony PERARD