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

Bug#856210: libdebian-installer: please parse SHA256 field and add it to di_* structs



Control: tags -1 + patch

Hi,

The regression in Bug#856215 in cdebootstrap:
"since SHA1 removal from Release file, only MD5sums are used"
could only be fixed by adding support for the SHA256 fields.

An open question is whether to preserve any support for MD5.
Keeping it would:

  + reduce potential for breakage (in case MD5 is "good enough" for some
    use-case or SHA256 is still impractical)
  + allow verifiers to check both MD5 *and* SHA256, for even stronger
    authentication in case one or both algorithms are broken
  - add complexity

Otherwise, dropping MD5 entirely would:

  * break reverse-dependencies (hopefully just anna, cdebootstrap) thus
    *forcing* us to stop using MD5 there, and implement SHA256

I've attached only the most minimal patch to allow reverse-depends do
implement SHA256.  They must adapt to the new names of struct members
*and* remember that the hash length is now different.  (The hash data is
stored in variable-length fields but the length is not recorded in the
structs, and the has is denoted by a magic number not an enum;  that
could be made better, but requiring a much larger diff).

A follow-up commit should extend the testsuite to check parsing of the
SHA256 fields;  that also would result in a larger diff however.

Regards,
-- 
Steven Chamberlain
steven@pyro.eu.org
diff --git a/debian/changelog b/debian/changelog
index 3dd29e1..1b7fcd8 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,7 +1,16 @@
 libdebian-installer (0.109) UNRELEASED; urgency=medium
 
+  [ Samuel Thibault ]
   * Fix build with gcc-7. Closes: #853489
 
+  [ Steven Chamberlain ]
+  * Parse SHA256 fields instead of MD5Sum fields in Packages files.
+  * Parse SHA256 fields instead of (no longer existing) SHA1 fields in
+    Release files.
+  * In structs di_release and di_package, add new sha256 member and
+    remove the md5sum member (a backward-incompatible change, this will
+    force reverse-dependencies to stop using MD5 for verification).
+
  -- Samuel Thibault <sthibault@debian.org>  Tue, 31 Jan 2017 11:09:16 +0100
 
 libdebian-installer (0.108) unstable; urgency=medium
diff --git a/include/debian-installer/package.h b/include/debian-installer/package.h
index 72d7444..e1f699d 100644
--- a/include/debian-installer/package.h
+++ b/include/debian-installer/package.h
@@ -112,7 +112,7 @@ struct di_package
   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 --git a/include/debian-installer/package_internal.h b/include/debian-installer/package_internal.h
index f6357d1..d410ce2 100644
--- a/include/debian-installer/package_internal.h
+++ b/include/debian-installer/package_internal.h
@@ -52,7 +52,7 @@ const di_parser_fieldinfo
   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 --git a/include/debian-installer/release.h b/include/debian-installer/release.h
index 223a4f8..8e3c572 100644
--- a/include/debian-installer/release.h
+++ b/include/debian-installer/release.h
@@ -40,7 +40,7 @@ struct di_release
   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 @@ struct di_release_file
     di_rstring key;                             /**< @internal */
   };
   unsigned int size;                            /**< size */
-  char *sum[2];                                 /**< checksums, currently md5 and sha1 */
+  char *sum[2];                                 /**< checksums, currently md5 and sha256 */
 };
 
 di_release *di_release_alloc (void);
diff --git a/src/package.c b/src/package.c
index 653b5dd..82c7653 100644
--- a/src/package.c
+++ b/src/package.c
@@ -38,7 +38,7 @@ void di_package_destroy (di_package *package)
   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 --git a/src/package_parser.c b/src/package_parser.c
index 6d6a5e7..de80d7e 100644
--- a/src/package_parser.c
+++ b/src/package_parser.c
@@ -180,13 +180,13 @@ const di_parser_fieldinfo
       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 @@ const di_parser_fieldinfo *di_package_parser_fieldinfo[] =
   &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 --git a/src/packages_parser.c b/src/packages_parser.c
index ac5c06b..30d66ba 100644
--- a/src/packages_parser.c
+++ b/src/packages_parser.c
@@ -65,7 +65,7 @@ const di_parser_fieldinfo *di_packages_parser_fieldinfo[] =
   &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 @@ const di_parser_fieldinfo *di_packages_minimal_parser_fieldinfo[] =
   &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 --git a/src/release.c b/src/release.c
index 7cc7cbf..7aff0d7 100644
--- a/src/release.c
+++ b/src/release.c
@@ -69,10 +69,10 @@ const di_parser_fieldinfo
       NULL,
       0
     ),
-  internal_di_release_parser_field_sha1 =
+  internal_di_release_parser_field_sha256 =
     DI_PARSER_FIELDINFO
     (
-      "SHA1",
+      "SHA256",
       di_release_parser_read_file,
       NULL,
       1
@@ -87,7 +87,7 @@ const di_parser_fieldinfo *di_release_parser_fieldinfo[] =
   &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 +110,7 @@ di_release *di_release_alloc (void)
   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 +124,7 @@ void di_release_free (di_release *release)
   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 +169,7 @@ void di_release_parser_read_file (data, fip, field_modifier, value, user_data)
   int ret;
   size_t buf_size;
   di_release *release = *data;
-  di_hash_table *table = release->md5sum;
+  di_hash_table *table = release->sha256;
 
   while (1)
   {

Attachment: signature.asc
Description: Digital signature


Reply to: