Bug#856211: anna: please implement SHA256 verification of .udeb files

[Steven Chamberlain]

Updated patch, which assumes the libdebian-installer4-dev package will
not be renamed.  Build-Depend on a recent enough version that provides
sha256 fields.

Regards,
-- 
Steven Chamberlain
ste...@pyro.eu.org
diff --git a/debian/changelog b/debian/changelog
index d6682ca..20e33a0 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+anna (1.58) UNRELEASED; urgency=medium
+
+  * Team upload.
+  * Replace md5sum verification with sha256sum (Closes: #856211).
+- (Build-)Depend on libdebian-installer4-dev >= 0.109 which provides
+  those sha256 fields.
+
+ -- Steven Chamberlain   Mon, 27 Feb 2017 15:13:37 +
+
 anna (1.57) unstable; urgency=medium
 
   [ Updated translations ]
diff --git a/debian/control b/debian/control
index def2af9..4998489 100644
--- a/debian/control
+++ b/debian/control
@@ -3,7 +3,7 @@ Section: debian-installer
 Priority: standard
 Maintainer: Debian Install System Team 
 Uploaders: Bastian Blank , Christian Perrier 
-Build-Depends: debhelper (>= 9), dpkg-dev (>= 1.15.7), libdebconfclient0-dev (>= 0.46), libdebian-installer4-dev (>= 0.41)
+Build-Depends: debhelper (>= 9), dpkg-dev (>= 1.15.7), libdebconfclient0-dev (>= 0.46), libdebian-installer4-dev (>= 0.109)
 Vcs-Browser: https://anonscm.debian.org/cgit/d-i/anna.git
 Vcs-Git: https://anonscm.debian.org/git/d-i/anna.git
 
diff --git a/anna.c b/anna.c
index 4b68816..e03d34a 100644
--- a/anna.c
+++ b/anna.c
@@ -318,8 +318,8 @@ install_modules(di_packages *status, di_packages *packages) {
 	}
 }
 
-if (! md5sum(package->md5sum, dest_file)) {
-	di_log(DI_LOG_LEVEL_WARNING, "bad md5sum");
+if (! sha256sum(package->sha256, dest_file)) {
+	di_log(DI_LOG_LEVEL_WARNING, "bad sha256sum");
 	if (!quiet)
 		/* error handling may use a progress bar, so stop the current one */
 		debconf_progress_stop(debconf);
diff --git a/util.c b/util.c
index 39af3db..7d09cf8 100644
--- a/util.c
+++ b/util.c
@@ -224,23 +224,26 @@ int load_templates (di_packages *packages) {
 }
 #endif /* LOADTEMPLATES */
 
-/* Check whether the md5sum of file matches sum. If not, return 0. */
-int md5sum(const char *sum, const char *file) {
+/* Length of a SHA256 hash in hex representation */
+#define SHA256_HEX_LENGTH 64
+
+/* Check whether the sha256sum of file matches sum. If not, return 0. */
+int sha256sum(const char *sum, const char *file) {
 	FILE *fp;
 	char line[1024];
 
-	/* Trivially true if the Packages file doesn't have md5sum lines */
+	/* Trivially true if the Packages file doesn't have sha256sum lines */
 	if (sum == NULL)
 		return 1;
-	snprintf(line, sizeof(line), "/usr/bin/md5sum %s", file);
+	snprintf(line, sizeof(line), "/usr/bin/sha256sum %s", file);
 	fp = popen(line, "r");
 	if (fp == NULL)
 		return 0;
 	if (fgets(line, sizeof(line), fp) != NULL) {
 		pclose(fp);
-		if (strlen(line) < 32)
+		if (strlen(line) < SHA256_HEX_LENGTH)
 			return 0;
-		line[32] = '\0';
+		line[SHA256_HEX_LENGTH] = '\0';
 		return !strcmp(line, sum);
 	}
 	pclose(fp);
diff --git a/util.h b/util.h
index 71135e0..0c9e0a2 100644
--- a/util.h
+++ b/util.h
@@ -10,7 +10,7 @@ bool is_installed(di_package *p, di_packages *status);
 size_t package_to_choice(di_package *package, char *buf, size_t size);
 char *list_to_choices(di_package **packages, int c_values);
 int get_package (di_package *package, char *dest);
-int md5sum(const char* sum, const char *file);
+int sha256sum(const char* sum, const char *file);
 int skip_package(di_package *p);
 int package_name_compare(const void *v1, const void *v2);
 void take_includes(di_packages *packages);


signature.asc
Description: Digital signature

Processed: Re: Bug#856211: anna: please implement SHA256 verification of .udeb files

[Debian Bug Tracking System]

Processing control commands:

> tags -1 + patch
Bug #856211 [src:anna] anna: please implement SHA256 verification of .udeb files
Added tag(s) patch.

-- 
856211: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=856211
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems

Bug#856211: anna: please implement SHA256 verification of .udeb files

[Steven Chamberlain]

Control: tags -1 + patch

Hi,

Attached is a minimal patch intended to implement SHA256 verification.
It would depend on libdebian-installer being patched first (#856210) and
bumping the soname to 5.

"#define SHA256_HEX_LENGTH 64" is made explicit as possible so that one
remembers to increase it if changing SHA256 to SHA512 in the future.  A
more thorough rework of this code might store the hash type (as an enum)
and length, in the di_package struct instead.

Thanks,
Regards,
-- 
Steven Chamberlain
ste...@pyro.eu.org


signature.asc
Description: Digital signature

Bug#856211: anna: please implement SHA256 verification of .udeb files

[Steven Chamberlain]

Steven Chamberlain wrote:
> Attached is [...]

Regards,
-- 
Steven Chamberlain
ste...@pyro.eu.org
diff --git a/anna.c b/anna.c
index 4b68816..e03d34a 100644
--- a/anna.c
+++ b/anna.c
@@ -318,8 +318,8 @@ install_modules(di_packages *status, di_packages *packages) {
 	}
 }
 
-if (! md5sum(package->md5sum, dest_file)) {
-	di_log(DI_LOG_LEVEL_WARNING, "bad md5sum");
+if (! sha256sum(package->sha256, dest_file)) {
+	di_log(DI_LOG_LEVEL_WARNING, "bad sha256sum");
 	if (!quiet)
 		/* error handling may use a progress bar, so stop the current one */
 		debconf_progress_stop(debconf);
diff --git a/debian/changelog b/debian/changelog
index d6682ca..c885457 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+anna (1.58) UNRELEASED; urgency=medium
+
+  * Team upload.
+  * Replace md5sum verification with sha256sum (Closes: #856211).
+- (Build-)Depend on libdebian-installer soname version 5.
+
+ -- Steven Chamberlain   Mon, 27 Feb 2017 15:13:37 +
+
 anna (1.57) unstable; urgency=medium
 
   [ Updated translations ]
diff --git a/debian/control b/debian/control
index def2af9..20ff1c3 100644
--- a/debian/control
+++ b/debian/control
@@ -3,7 +3,7 @@ Section: debian-installer
 Priority: standard
 Maintainer: Debian Install System Team 
 Uploaders: Bastian Blank , Christian Perrier 
-Build-Depends: debhelper (>= 9), dpkg-dev (>= 1.15.7), libdebconfclient0-dev (>= 0.46), libdebian-installer4-dev (>= 0.41)
+Build-Depends: debhelper (>= 9), dpkg-dev (>= 1.15.7), libdebconfclient0-dev (>= 0.46), libdebian-installer5-dev
 Vcs-Browser: https://anonscm.debian.org/cgit/d-i/anna.git
 Vcs-Git: https://anonscm.debian.org/git/d-i/anna.git
 
diff --git a/util.c b/util.c
index 39af3db..7d09cf8 100644
--- a/util.c
+++ b/util.c
@@ -224,23 +224,26 @@ int load_templates (di_packages *packages) {
 }
 #endif /* LOADTEMPLATES */
 
-/* Check whether the md5sum of file matches sum. If not, return 0. */
-int md5sum(const char *sum, const char *file) {
+/* Length of a SHA256 hash in hex representation */
+#define SHA256_HEX_LENGTH 64
+
+/* Check whether the sha256sum of file matches sum. If not, return 0. */
+int sha256sum(const char *sum, const char *file) {
 	FILE *fp;
 	char line[1024];
 
-	/* Trivially true if the Packages file doesn't have md5sum lines */
+	/* Trivially true if the Packages file doesn't have sha256sum lines */
 	if (sum == NULL)
 		return 1;
-	snprintf(line, sizeof(line), "/usr/bin/md5sum %s", file);
+	snprintf(line, sizeof(line), "/usr/bin/sha256sum %s", file);
 	fp = popen(line, "r");
 	if (fp == NULL)
 		return 0;
 	if (fgets(line, sizeof(line), fp) != NULL) {
 		pclose(fp);
-		if (strlen(line) < 32)
+		if (strlen(line) < SHA256_HEX_LENGTH)
 			return 0;
-		line[32] = '\0';
+		line[SHA256_HEX_LENGTH] = '\0';
 		return !strcmp(line, sum);
 	}
 	pclose(fp);
diff --git a/util.h b/util.h
index 71135e0..0c9e0a2 100644
--- a/util.h
+++ b/util.h
@@ -10,7 +10,7 @@ bool is_installed(di_package *p, di_packages *status);
 size_t package_to_choice(di_package *package, char *buf, size_t size);
 char *list_to_choices(di_package **packages, int c_values);
 int get_package (di_package *package, char *dest);
-int md5sum(const char* sum, const char *file);
+int sha256sum(const char* sum, const char *file);
 int skip_package(di_package *p);
 int package_name_compare(const void *v1, const void *v2);
 void take_includes(di_packages *packages);


signature.asc
Description: Digital signature

Bug#856211: anna: please implement SHA256 verification of .udeb files

[Steven Chamberlain]

Cyril Brulebois wrote:
> IIRC MD5sum field was kept (as in: added
> back) because debian-cd needs it at the moment, which partly explains why this
> wasn't fixed earlier.

I think backward-compatibility would have been okay as long as *either*:

  * the archive published Release files with old+new hash algorithms; or
  * the utilities consuming it, supported the old/new hash algorithms;

but here we had done both of those things, which allowed for a downgrade
to go unnoticed.

I think right now it is easier to fix anna+cdebootstrap than debian-cd?

> but referencing places where stuff like parsing happens
> (Release, Packages, etc.), and where checkums are used,

Yesss, but only if someone updated that documentation with what the code
is doing.  Removal of SHA1 in Relases had an action-at-a-distance effect
on cdebootstrap, so it wouldn't be clear that the documentation needed
to change then.

In the ideal world, the code itself would be the clear, authoritative
reference of what it is doing.  I wish that we can remove all references
to md5 and sha1 there.

Regards,
-- 
Steven Chamberlain
ste...@pyro.eu.org


signature.asc
Description: Digital signature

Bug#856211: anna: please implement SHA256 verification of .udeb files

[Cyril Brulebois]

Hi,

Steven Chamberlain  (2017-02-27):
> Cyril Brulebois wrote:
> > AFAICT net-retriever does the fetching and checking work?
> 
> Mayyybe...
> 
> Although with 
> http://ftp.de.debian.org/debian/dists/testing/main/installer-i386/20170127/images/netboot/mini.iso
> I observed md5sum and sha256sum only being executed as indicated in the
> attached log.

So we're only checking newer checksums for Packages files (against what's in
Release files, bad bad bad us indeed. IIRC MD5sum field was kept (as in: added
back) because debian-cd needs it at the moment, which partly explains why this
wasn't fixed earlier.

I'm not sure whether this exists already (be it for the whole distribution or
for d-i specifically), but referencing places where stuff like parsing happens
(Release, Packages, etc.), and where checkums are used, would help figure out
what to change when the list of supported fields/checksums are updated. Might
be another way to leverage this whole debacle thing.


KiBi.


signature.asc
Description: Digital signature

Bug#856211: anna: please implement SHA256 verification of .udeb files

[Steven Chamberlain]

Hello!

Cyril Brulebois wrote:
> AFAICT net-retriever does the fetching and checking work?

Mayyybe...

Although with 
http://ftp.de.debian.org/debian/dists/testing/main/installer-i386/20170127/images/netboot/mini.iso
I observed md5sum and sha256sum only being executed as indicated in the
attached log.

Regards,
-- 
Steven Chamberlain
ste...@pyro.eu.org
/usr/bin/sha256sum /tmp/net-retriever-1817-Packages
/usr/bin/sha256sum /tmp/net-retriever-1872-Packages
/usr/bin/sha256sum /tmp/net-retriever-1872-Packages
/usr/bin/sha256sum /tmp/net-retriever-1872-Packages
/usr/bin/md5sum /var/cache/anna/apt-mirror-setup_0.123_all.udeb
/usr/bin/md5sum /var/cache/anna/apt-setup-udeb_0.123_i386.udeb
/usr/bin/md5sum /var/cache/anna/base-installer_1.167_all.udeb
/usr/bin/md5sum /var/cache/anna/bootstrap-base_1.167_i386.udeb
/usr/bin/md5sum /var/cache/anna/btrfs-progs-udeb_4.7.3-1_i386.udeb
/usr/bin/md5sum /var/cache/anna/clock-setup_0.132_i386.udeb
/usr/bin/md5sum /var/cache/anna/di-utils-mapdevfs_1.117_i386.udeb
/usr/bin/md5sum /var/cache/anna/debootstrap-udeb_1.0.88_all.udeb
/usr/bin/md5sum /var/cache/anna/dosfstools-udeb_4.1-1_i386.udeb
/usr/bin/md5sum /var/cache/anna/e2fsprogs-udeb_1.43.4-2_i386.udeb
/usr/bin/md5sum /var/cache/anna/elilo-installer_1.31_i386.udeb
/usr/bin/md5sum /var/cache/anna/finish-install_2.75_all.udeb
/usr/bin/md5sum /var/cache/anna/fuse-udeb_2.9.7-1_i386.udeb
/usr/bin/md5sum /var/cache/anna/libfuse2-udeb_2.9.7-1_i386.udeb
/usr/bin/md5sum /var/cache/anna/grub-installer_1.137_i386.udeb
/usr/bin/md5sum /var/cache/anna/grub-mount-udeb_2.02~beta3-5_i386.udeb
/usr/bin/md5sum /var/cache/anna/disk-detect_1.121_i386.udeb
/usr/bin/md5sum /var/cache/anna/installation-locale_1.7_i386.udeb
/usr/bin/md5sum /var/cache/anna/jfsutils-udeb_1.1.15-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/kickseed-common_0.62_all.udeb
/usr/bin/md5sum /var/cache/anna/libbsd0-udeb_0.8.3-1_i386.udeb
/usr/bin/md5sum /var/cache/anna/lilo-installer_1.52_i386.udeb
/usr/bin/md5sum /var/cache/anna/ata-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/btrfs-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum 
/var/cache/anna/cdrom-core-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/efi-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/ext4-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/fat-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum 
/var/cache/anna/firewire-core-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/isofs-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/jfs-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/kernel-image-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/loop-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/md-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/nic-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum 
/var/cache/anna/nic-pcmcia-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum 
/var/cache/anna/nic-shared-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/nic-usb-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum 
/var/cache/anna/nic-wireless-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/pata-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/pcmcia-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum 
/var/cache/anna/pcmcia-storage-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/sata-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum 
/var/cache/anna/scsi-core-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/scsi-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum 
/var/cache/anna/usb-storage-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/xfs-modules-4.9.0-1-686-di_4.9.6-3_i386.udeb
/usr/bin/md5sum /var/cache/anna/dmsetup-udeb_1.02.137-1_i386.udeb
/usr/bin/md5sum /var/cache/anna/libdevmapper1.02.1-udeb_1.02.137-1_i386.udeb
/usr/bin/md5sum /var/cache/anna/liblzo2-2-udeb_2.08-1.2_i386.udeb
/usr/bin/md5sum /var/cache/anna/mdadm-udeb_3.4-4_i386.udeb
/usr/bin/md5sum /var/cache/anna/network-console_1.62_i386.udeb
/usr/bin/md5sum /var/cache/anna/nobootloader_1.47_all.udeb
/usr/bin/md5sum /var/cache/anna/ntfs-3g-udeb_2016.2.22AR.1-4_i386.udeb
/usr/bin/md5sum /var/cache/anna/open-iscsi-udeb_2.0.874-2_i386.udeb
/usr/bin/md5sum /var/cache/anna/libisns-nocrypto0-udeb_0.97-1_i386.udeb
/usr/bin/md5sum /var/cache/anna/openssh-server-udeb_7.4p1-6_i386.udeb
/usr/bin/md5sum /var/cache/anna/os-prober-udeb_1.74_i386.udeb
/usr/bin/md5sum /var/cache/anna/partconf-find-partitions_1.50_i386.udeb
/usr/bin/md5sum /var/cache/anna/libparted-fs-resize0-udeb_3.2-17_i386.udeb
/usr/bin/md5sum /var/cache/anna/libparted2-udeb_3.2-17_i386.udeb
/usr/bin/md5sum /var/cache/anna/partman-auto_137_i386.udeb
/usr/bin/md5sum 

Bug#856211: anna: please implement SHA256 verification of .udeb files

[Cyril Brulebois]

Steven Chamberlain  (2017-02-26):
> To date, anna still only implements MD5 verification of .udeb files,
> despite its formal deprecation as a digital signature algorithm by
> RFC6151 (2011) and recommendations of academic literature years prior.
> 
> The files are typically downloaded via insecure HTTP transport, so the
> checksum verification is critical for the security of the installed
> system.  stretch is expected to be a supported release until 2022.  So
> I'm tentatively filing this bug as RC-severity.
> 
> Further context and an overview of related bugs will be published at:
> https://wiki.debian.org/InstallerDebacle

AFAICT net-retriever does the fetching and checking work?


KiBi.


signature.asc
Description: Digital signature

Bug#856211: anna: please implement SHA256 verification of .udeb files

[Steven Chamberlain]

Source: anna
Version: 1.57
Severity: grave
Tags: security
X-Debbugs-Cc: secur...@debian.org
User: debian-rele...@lists.debian.org
Usertags: bsp-2017-02-de-Berlin
Control: block -1 by 856210

Hi,

To date, anna still only implements MD5 verification of .udeb files,
despite its formal deprecation as a digital signature algorithm by
RFC6151 (2011) and recommendations of academic literature years prior.

The files are typically downloaded via insecure HTTP transport, so the
checksum verification is critical for the security of the installed
system.  stretch is expected to be a supported release until 2022.  So
I'm tentatively filing this bug as RC-severity.

Further context and an overview of related bugs will be published at:
https://wiki.debian.org/InstallerDebacle

Thanks,
Regards,
-- 
Steven Chamberlain
ste...@pyro.eu.org


signature.asc
Description: Digital signature