[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#856405: unblock: libdebian-installer/0.109 and others



Package: release.debian.org
User: release.debian.org@packages.debian.org
Usertags: unblock
X-Debbugs-Cc: debian-boot@lists.debian.org

Hi!

Attached are proposed debdiffs for anna, cdebootstrap and their
dependency libdebian-installer (Bug #856210).

Would the release team be willing to grant unblocks for these?
(It would also require an ACK from the d-i release manager).

In the installer, net-retriever verifies the Release file with SHA256,
but anna only validates the .udeb files with MD5, which was surprising.
The .udeb files are extracted and then their contents may be executed
with full privileges during the install (Bug #856211).

netboot images typically fetch .udeb files over unsecured HTTP.  Other
install media bundles those so they need not be downloaded, but it could
still happen if networking is configured during the install and a
network mirror has newer versions of any required .udeb files.  (Some
.udeb files are retrieved later, after installing the base system).

If not already considered a grave security flaw, it might be during the
lifetime of stretch (-2022?).  Even if fixed in a point release, any
install media created before then would remain vulnerable.

cdebootstrap - not used by the installer, but available in stretch - 
only verifies Release files using MD5 (Bug #856215), which is an
unintended regression since jessie.

The changes to libdebian-installer are ABI-compatible, such that only
reverse-dependencies that use the md5sum field should be affected
(thought to be just anna and cdebootstrap).  They would FTBFS until
patched, and already-built binaries would report a "md5sum mismatch" if
they used this new version of the library at run-time, since the new
SHA256 hashes would not match the MD5 hashes they expect.

unblock libdebian-installer/0.109
unblock anna/1.58
unblock cdebootstrap/0.7.7

Thanks!

-- System Information:
Debian Release: stretch/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: kfreebsd-amd64 (x86_64)

Kernel: kFreeBSD 10.1-0-amd64
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)
diff -Nru libdebian-installer-0.108/debian/changelog libdebian-installer-0.109/debian/changelog
--- libdebian-installer-0.108/debian/changelog	2016-08-30 05:45:17.000000000 +0100
+++ libdebian-installer-0.109/debian/changelog	2017-02-28 16:36:45.000000000 +0000
@@ -1,3 +1,15 @@
+libdebian-installer (0.109) unstable; urgency=high
+
+  * In structs di_release and di_package, replace md5sum member with a
+    sha256 member.  This is ABI-compatible, but reverse-dependencies
+    will fail if they still try to verify with MD5 (Closes: #856212).
+  * Parse SHA256 fields instead of MD5Sum fields in Packages files.
+  * Parse SHA256 fields instead of MD5Sum fields in Release files.
+  * No longer try to parse SHA1 field, which is no longer present in
+    Release files as of stretch.
+
+ -- Steven Chamberlain <stevenc@debian.org>  Tue, 28 Feb 2017 16:36:45 +0000
+
 libdebian-installer (0.108) unstable; urgency=medium
 
   [ Helmut Grohne ]
diff -Nru libdebian-installer-0.108/include/debian-installer/package.h libdebian-installer-0.109/include/debian-installer/package.h
--- libdebian-installer-0.108/include/debian-installer/package.h	2011-03-03 02:00:33.000000000 +0000
+++ libdebian-installer-0.109/include/debian-installer/package.h	2017-02-28 16:33:59.000000000 +0000
@@ -112,7 +112,7 @@
   di_slist depends;                                     /**< Any different dependency types */
   char *filename;                                       /**< Filename field */
   size_t size;                                          /**< Size field */
-  char *md5sum;                                         /**< MD5Sum field */
+  char *sha256;                                         /**< SHA256 field */
   char *short_description;                              /**< Description field, first part*/
   char *description;                                    /**< Description field, second part */
   unsigned int resolver;                                /**< @internal */
diff -Nru libdebian-installer-0.108/include/debian-installer/package_internal.h libdebian-installer-0.109/include/debian-installer/package_internal.h
--- libdebian-installer-0.108/include/debian-installer/package_internal.h	2011-03-03 02:00:33.000000000 +0000
+++ libdebian-installer-0.109/include/debian-installer/package_internal.h	2017-02-28 16:33:59.000000000 +0000
@@ -52,7 +52,7 @@
   internal_di_package_parser_field_enhances,
   internal_di_package_parser_field_filename,
   internal_di_package_parser_field_size,
-  internal_di_package_parser_field_md5sum,
+  internal_di_package_parser_field_sha256,
   internal_di_package_parser_field_description;
 
 /**
diff -Nru libdebian-installer-0.108/include/debian-installer/release.h libdebian-installer-0.109/include/debian-installer/release.h
--- libdebian-installer-0.108/include/debian-installer/release.h	2011-03-03 02:00:33.000000000 +0000
+++ libdebian-installer-0.109/include/debian-installer/release.h	2017-02-28 16:33:59.000000000 +0000
@@ -40,7 +40,7 @@
   char *origin;                                 /**< Origin field */
   char *suite;                                  /**< Suite field */
   char *codename;                               /**< Codename field */
-  di_hash_table *md5sum;                        /**< checksum fields, includes di_release_file */
+  di_hash_table *sha256;                        /**< checksum fields, includes di_release_file */
   di_mem_chunk *release_file_mem_chunk;         /**< @internal */
 };
 
@@ -55,7 +55,7 @@
     di_rstring key;                             /**< @internal */
   };
   unsigned int size;                            /**< size */
-  char *sum[2];                                 /**< checksums, currently md5 and sha1 */
+  char *sum[2];                                 /**< sum[0] is sha256, sum[1] is empty */
 };
 
 di_release *di_release_alloc (void);
diff -Nru libdebian-installer-0.108/src/package.c libdebian-installer-0.109/src/package.c
--- libdebian-installer-0.108/src/package.c	2011-03-03 02:00:33.000000000 +0000
+++ libdebian-installer-0.109/src/package.c	2017-02-28 16:33:59.000000000 +0000
@@ -38,7 +38,7 @@
   di_free (package->architecture);
   di_free (package->version);
   di_free (package->filename);
-  di_free (package->md5sum);
+  di_free (package->sha256);
   di_free (package->short_description);
   di_free (package->description);
 
diff -Nru libdebian-installer-0.108/src/package_parser.c libdebian-installer-0.109/src/package_parser.c
--- libdebian-installer-0.108/src/package_parser.c	2011-03-03 02:00:33.000000000 +0000
+++ libdebian-installer-0.109/src/package_parser.c	2017-02-28 16:33:59.000000000 +0000
@@ -180,13 +180,13 @@
       di_parser_write_int,
       offsetof (di_package, size)
     ),
-  internal_di_package_parser_field_md5sum =
+  internal_di_package_parser_field_sha256 =
     DI_PARSER_FIELDINFO
     (
-      "MD5sum",
+      "SHA256",
       di_parser_read_string,
       di_parser_write_string,
-      offsetof (di_package, md5sum)
+      offsetof (di_package, sha256)
     ),
   internal_di_package_parser_field_description =
     DI_PARSER_FIELDINFO
@@ -217,7 +217,7 @@
   &internal_di_package_parser_field_enhances,
   &internal_di_package_parser_field_filename,
   &internal_di_package_parser_field_size,
-  &internal_di_package_parser_field_md5sum,
+  &internal_di_package_parser_field_sha256,
   &internal_di_package_parser_field_description,
   NULL
 };
diff -Nru libdebian-installer-0.108/src/packages_parser.c libdebian-installer-0.109/src/packages_parser.c
--- libdebian-installer-0.108/src/packages_parser.c	2011-03-03 02:00:33.000000000 +0000
+++ libdebian-installer-0.109/src/packages_parser.c	2017-02-28 16:33:59.000000000 +0000
@@ -65,7 +65,7 @@
   &internal_di_package_parser_field_enhances,
   &internal_di_package_parser_field_filename,
   &internal_di_package_parser_field_size,
-  &internal_di_package_parser_field_md5sum,
+  &internal_di_package_parser_field_sha256,
   &internal_di_package_parser_field_description,
   NULL
 };
@@ -109,7 +109,7 @@
   &internal_di_package_parser_field_depends,
   &internal_di_package_parser_field_pre_depends,
   &internal_di_package_parser_field_filename,
-  &internal_di_package_parser_field_md5sum,
+  &internal_di_package_parser_field_sha256,
   &internal_di_package_parser_field_size,
   NULL
 };
diff -Nru libdebian-installer-0.108/src/release.c libdebian-installer-0.109/src/release.c
--- libdebian-installer-0.108/src/release.c	2011-03-03 02:00:33.000000000 +0000
+++ libdebian-installer-0.109/src/release.c	2017-02-28 16:33:59.000000000 +0000
@@ -61,21 +61,13 @@
       NULL,
       offsetof (di_release, codename)
     ),
-  internal_di_release_parser_field_md5sum =
+  internal_di_release_parser_field_sha256 =
     DI_PARSER_FIELDINFO
     (
-      "MD5Sum",
+      "SHA256",
       di_release_parser_read_file,
       NULL,
       0
-    ),
-  internal_di_release_parser_field_sha1 =
-    DI_PARSER_FIELDINFO
-    (
-      "SHA1",
-      di_release_parser_read_file,
-      NULL,
-      1
     );
 
 /**
@@ -86,8 +78,7 @@
   &internal_di_release_parser_field_origin,
   &internal_di_release_parser_field_suite,
   &internal_di_release_parser_field_codename,
-  &internal_di_release_parser_field_md5sum,
-  &internal_di_release_parser_field_sha1,
+  &internal_di_release_parser_field_sha256,
   NULL
 };
 
@@ -110,7 +101,7 @@
   di_release *ret;
 
   ret = di_new0 (di_release, 1);
-  ret->md5sum = di_hash_table_new_full (di_rstring_hash, di_rstring_equal, NULL, internal_di_release_file_destroy_func);
+  ret->sha256 = di_hash_table_new_full (di_rstring_hash, di_rstring_equal, NULL, internal_di_release_file_destroy_func);
   ret->release_file_mem_chunk = di_mem_chunk_new (sizeof (di_release_file), 4096);
 
   return ret;
@@ -124,7 +115,7 @@
   di_free (release->origin);
   di_free (release->suite);
   di_free (release->codename);
-  di_hash_table_destroy (release->md5sum);
+  di_hash_table_destroy (release->sha256);
   di_mem_chunk_destroy (release->release_file_mem_chunk);
   di_free (release);
 }
@@ -169,7 +160,7 @@
   int ret;
   size_t buf_size;
   di_release *release = *data;
-  di_hash_table *table = release->md5sum;
+  di_hash_table *table = release->sha256;
 
   while (1)
   {
diff -Nru anna-1.57/debian/changelog anna-1.58/debian/changelog
--- anna-1.57/debian/changelog	2017-02-13 06:08:47.000000000 +0000
+++ anna-1.58/debian/changelog	2017-02-28 16:41:04.000000000 +0000
@@ -1,3 +1,12 @@
+anna (1.58) unstable; urgency=high
+
+  * Team upload.
+  * Replace md5sum verification with sha256sum (Closes: #856211).
+    - (Build-)Depend on libdebian-installer4-dev >= 0.109 which provides
+      those sha256 fields.
+
+ -- Steven Chamberlain <stevenc@debian.org>  Tue, 28 Feb 2017 16:41:04 +0000
+
 anna (1.57) unstable; urgency=medium
 
   [ Updated translations ]
diff -Nru anna-1.57/debian/control anna-1.58/debian/control
--- anna-1.57/debian/control	2016-01-30 04:00:42.000000000 +0000
+++ anna-1.58/debian/control	2017-02-28 16:40:58.000000000 +0000
@@ -3,7 +3,7 @@
 Priority: standard
 Maintainer: Debian Install System Team <debian-boot@lists.debian.org>
 Uploaders: Bastian Blank <waldi@debian.org>, Christian Perrier <bubulle@debian.org>
-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 -Nru anna-1.57/anna.c anna-1.58/anna.c
--- anna-1.57/anna.c	2013-01-06 05:48:37.000000000 +0000
+++ anna-1.58/anna.c	2017-02-28 16:40:58.000000000 +0000
@@ -318,8 +318,8 @@
 					}
 				}
 
-				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 -Nru anna-1.57/util.c anna-1.58/util.c
--- anna-1.57/util.c	2013-01-06 05:48:37.000000000 +0000
+++ anna-1.58/util.c	2017-02-28 16:40:58.000000000 +0000
@@ -224,23 +224,26 @@
 }
 #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 -Nru anna-1.57/util.h anna-1.58/util.h
--- anna-1.57/util.h	2013-01-06 05:48:37.000000000 +0000
+++ anna-1.58/util.h	2017-02-28 16:40:58.000000000 +0000
@@ -10,7 +10,7 @@
 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);
diff -Nru cdebootstrap-0.7.6/debian/changelog cdebootstrap-0.7.7/debian/changelog
--- cdebootstrap-0.7.6/debian/changelog	2016-11-08 18:31:14.000000000 +0000
+++ cdebootstrap-0.7.7/debian/changelog	2017-02-28 16:44:30.000000000 +0000
@@ -1,3 +1,17 @@
+cdebootstrap (0.7.7) unstable; urgency=high
+
+  * 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 <stevenc@debian.org>  Tue, 28 Feb 2017 16:44:30 +0000
+
 cdebootstrap (0.7.6) unstable; urgency=medium
 
   * Make generation of tar reproducible. (closes: #777737)
diff -Nru cdebootstrap-0.7.6/debian/control cdebootstrap-0.7.7/debian/control
--- cdebootstrap-0.7.6/debian/control	2016-11-08 18:23:34.000000000 +0000
+++ cdebootstrap-0.7.7/debian/control	2017-02-28 15:46:16.000000000 +0000
@@ -9,7 +9,7 @@
  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 -Nru cdebootstrap-0.7.6/src/check.c cdebootstrap-0.7.7/src/check.c
--- cdebootstrap-0.7.6/src/check.c	2014-06-22 12:57:53.000000000 +0100
+++ cdebootstrap-0.7.7/src/check.c	2017-02-28 15:47:10.000000000 +0000
@@ -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 @@
   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 @@
   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, &key);
+  item = di_hash_table_lookup (rel->sha256, &key);
   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;
 }
 

Attachment: signature.asc
Description: Digital signature


Reply to: