Bug#856212: cdebootstrap: please implement SHA256 verification of .deb files

[Steven Chamberlain]

Control: tags -1 + patch

Hi,

The attached patch is based on having
libdebian-installer_bug856210_v3.patch applied:

  * libdebian-installer4-dev would not change its name
  * sum[1] is already empty/unusable
  * sum[0] would become the sha256 field

Thanks,
Regards,
-- 
Steven Chamberlain
ste...@pyro.eu.org
diff --git a/debian/changelog b/debian/changelog
index c73f96e..96b0eee 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,17 @@
+cdebootstrap (0.7.7) UNRELEASED; urgency=medium
+
+  * Team upload.
+  * Implement SHA256 verification of .deb files (Closes: #856212).
+  * Implement SHA256 verification of Packages files.
+- (Build-)Depend on libdebian-installer4-dev >= 0.109 which provides
+  those sha256 fields
+  * Remove support for SHA1 hashes, since they are no longer published
+in Release files, and the full length of them was not previously
+being checked against the expected values (Closes: #856213).
+  * Disallow fallback to MD5-only verification (Closes: #856215).
+
+ -- Steven Chamberlain   Mon, 27 Feb 2017 15:53:40 +
+
 cdebootstrap (0.7.6) unstable; urgency=medium
 
   * Make generation of tar reproducible. (closes: #37)
diff --git a/debian/control b/debian/control
index 49bb110..6996ce5 100644
--- a/debian/control
+++ b/debian/control
@@ -9,7 +9,7 @@ Build-Depends:
  libbz2-dev,
  libcurl4-gnutls-dev,
  libdebconfclient0-dev (>= 0.40),
- libdebian-installer4-dev (>= 0.81~),
+ libdebian-installer4-dev (>= 0.109~),
  liblzma-dev,
  pkg-config,
  zlib1g-dev
diff --git a/src/check.c b/src/check.c
index 34e96da..c41d9e4 100644
--- a/src/check.c
+++ b/src/check.c
@@ -32,6 +32,9 @@
 #include "frontend.h"
 #include "suite.h"
 
+/* Length of a SHA256 hash in hex representation */
+#define SHA256_HEX_LENGTH 64
+
 static int check_sum (const char *target, const char *exec, const char *sum, const char *message)
 {
   int ret;
@@ -51,14 +54,14 @@ static int check_sum (const char *target, const char *exec, const char *sum, con
   if (ret)
 return 1;
 
-  if (!strncmp (buf, sum, 32))
+  if (!strncmp (buf, sum, SHA256_HEX_LENGTH))
 return 0;
   return 1;
 }
 
 int check_deb (const char *target, di_package *p, const char *message)
 {
-  return check_sum (target, "md5sum", p->md5sum, message);
+  return check_sum (target, "sha256sum", p->sha256, message);
 }
 
 int check_packages (const char *target, const char *ext, di_release *rel)
@@ -72,14 +75,12 @@ int check_packages (const char *target, const char *ext, di_release *rel)
   snprintf (buf_file, sizeof (buf_file), "main/binary-%s/Packages%s", arch, ext);
   key.string = (char *) buf_file;
   key.size = strlen (buf_file);
-  item = di_hash_table_lookup (rel->md5sum, );
+  item = di_hash_table_lookup (rel->sha256, );
   if (!item)
 log_text (DI_LOG_LEVEL_ERROR, "Can't find checksum for Packages file");
 
-  if (item->sum[1])
-return check_sum (target, "sha1sum", item->sum[1], buf_name);
   if (item->sum[0])
-return check_sum (target, "md5sum", item->sum[0], buf_name);
+return check_sum (target, "sha256sum", item->sum[0], buf_name);
   return 1;
 }
 


signature.asc
Description: Digital signature

Processed: Re: Bug#856212: cdebootstrap: please implement SHA256 verification of .deb files

[Debian Bug Tracking System]

Processing control commands:

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

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

Bug#856212: cdebootstrap: please implement SHA256 verification of .deb files

[Steven Chamberlain]

Hi,

Bastian Blank wrote:
> I was not able to provide a real fix as I'm rather time constrained.

Don't worry, I'm prepared to write patches.  But I wonder:

  * is it okay to drop MD5 support, when implementing SHA256?
  * must we fix this before the stretch release?  or otherwise, would it
be possible to make such a big change in a stable point release?

> However please provide this information, as I only found something with
> about 2^120 for preimage attacks on MD5, which is still not fesable in
> real live.

Last time I brought up the topic, that argument was given.

But maybe it's the wrong approach to ask "are we *sure* MD5 is broken
and we must replace it?".  We need to make a prediction that lasts the
supported lifetime of stretch (until 2022?);  and some adversaries do
not reveal their capabilities.

It's actually kind of bizarre that we've published SHA256 sums in the
archive since 2007 and *still* don't use them here.  I think there is a
greater risk that we forget, or be too lazy, than we do this 'too soon'.

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


signature.asc
Description: Digital signature

Bug#856212: cdebootstrap: please implement SHA256 verification of .deb files

[Bastian Blank]

On Sun, Feb 26, 2017 at 04:32:43PM +, Steven Chamberlain wrote:
> To date, cdebootstrap still only implements MD5 verification of .deb
> files, despite its formal deprecation as a digital signature algorithm
> by RFC6151 (2011) and recommendations of academic literature years
> prior.

I was not able to provide a real fix as I'm rather time constrained.
However please provide this information, as I only found something with
about 2^120 for preimage attacks on MD5, which is still not fesable in
real live.

Bastian

-- 
Klingon phaser attack from front!
100% Damage to life support

Bug#856212: cdebootstrap: please implement SHA256 verification of .deb files

[Steven Chamberlain]

Source: cdebootstrap
Version: 0.7.6
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, cdebootstrap still only implements MD5 verification of .deb
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