[OpenWrt-Devel] [PATCH] [packages] opkg: Make opkg understand old md5

John Crispin blogic at openwrt.org
Fri Feb 26 03:17:11 EST 2016


[...]

>>>  
>>> +define Package/opkg-smime/config
>>> +config OPKG_SMIME_SUPPORT_MD5
>>> +  bool
>>> +  default n
>>> +  depends on PACKAGE_opkg-smime
>>> +  prompt "Support reading old md5 hashes."
>>> +  help
>>> +	Old opkg used md5s, new uses sha. This options enables understanding both while prefering sha.
>>> +endef
>>
>> why add a new build variant ?
> 
> There are two opkg variants - one named opkg and one named opkg-smime. I added
> same option to both as maybe someone might want to enable backward
> compatibility in just one of them. For example to save some space or because he
> is more afraid of MD5 in one of the situations.
> 

erm yes ... i missed the /config part ;)


>>> +
>>>  Package/opkg-smime/conffiles = $(Package/opkg/conffiles)
>>>  
>>>  TARGET_CFLAGS += -ffunction-sections -fdata-sections
>>> @@ -98,10 +119,20 @@ CONFIGURE_ARGS += \
>>>  
>>>  ifeq ($(BUILD_VARIANT),smime)
>>>  	CONFIGURE_ARGS += --enable-openssl --disable-usign
>>> +  ifeq ($(CONFIG_OPKG_SMIME_SUPPORT_MD5),y)
>>> +		CONFIGURE_ARGS += --enable-md5
>>> +  else
>>> +		CONFIGURE_ARGS += --disable-md5
>>> +  endif
>>
>> might want to put this into its own block, set a variable and then reuse
>> that var here and below ...
>>
>>>  else
>>>    ifndef CONFIG_SIGNED_PACKAGES
>>>      CONFIGURE_ARGS += --disable-usign
>>>    endif
>>> +  ifeq ($(CONFIG_OPKG_SUPPORT_MD5),y)
>>> +		CONFIGURE_ARGS += --enable-md5
>>> +  else
>>> +		CONFIGURE_ARGS += --disable-md5
>>> +  endif
>>
>> to avoid duplication here
> 
> Yep, was thinking about it, but given that there are two variants of the same
> package and options might affect either of them, couldn't figure out better way
> how to do it. Except maybe rewriting opkg-smime as an compile time option of
> opkg instead of different subpackage, but that seemed like different and too
> big change.


ok, make sense i guess. can you change the indentation to be the same as
the rest of the code please ?

> 
>>>  endif
>>>  
>>>  MAKE_FLAGS = \
>>> diff --git a/package/system/opkg/patches/230-drop_md5_support.patch b/package/system/opkg/patches/230-drop_md5_support.patch
>>> index 63671e5..07673db 100644
>>> --- a/package/system/opkg/patches/230-drop_md5_support.patch
>>> +++ b/package/system/opkg/patches/230-drop_md5_support.patch
>>> @@ -1,6 +1,6 @@
>>>  --- a/libopkg/conffile.c
>>>  +++ b/libopkg/conffile.c
>>> -@@ -36,7 +36,7 @@
>>> +@@ -36,7 +36,7 @@ void conffile_deinit(conffile_t *conffil
>>>   
>>>   int conffile_has_been_modified(conffile_t *conffile)
>>>   {
>>> @@ -9,7 +9,7 @@
>>>       char *filename = conffile->name;
>>>       char *root_filename;
>>>       int ret = 1;
>>> -@@ -48,16 +48,19 @@
>>> +@@ -48,16 +48,23 @@ int conffile_has_been_modified(conffile_
>>>   
>>>       root_filename = root_filename_alloc(filename);
>>>   
>>> @@ -19,7 +19,11 @@
>>>  -        opkg_msg(INFO, "Conffile %s:\n\told md5=%s\n\tnew md5=%s\n",
>>>  -		conffile->name, md5sum, conffile->value);
>>>  +#ifdef HAVE_MD5
>>> -+    chksum = file_md5sum_alloc(root_filename);
>>> ++    if(conffile->value && strlen(conffile->value) > 33) {
>>
>> is there no better way than checking for the length ?
> 
> Don't know about any other way. Hashes don't have signature as far as I know
> but they should have pretty constant length. Or I could compute both sums and
> try if at least one of them matches, but this seemed more straight forward.

its fine, was just wondering

> 
>>> ++        chksum = file_sha256sum_alloc(root_filename);
>>> ++    } else {
>>> ++        chksum = file_md5sum_alloc(root_filename);
>>> ++    }
>>>  +#else
>>>  +    chksum = file_sha256sum_alloc(root_filename);
>>>  +#endif
>>> @@ -48,7 +52,7 @@
>>>   #include "libbb/libbb.h"
>>>   
>>>   #if defined HAVE_SHA256
>>> -@@ -135,6 +137,7 @@
>>> +@@ -135,6 +137,7 @@ file_mkdir_hier(const char *path, long m
>>>   	return make_directory(path, mode, FILEUTILS_RECUR);
>>>   }
>>>   
>>> @@ -56,7 +60,7 @@
>>>   char *file_md5sum_alloc(const char *file_name)
>>>   {
>>>       static const int md5sum_bin_len = 16;
>>> -@@ -180,6 +183,7 @@
>>> +@@ -180,6 +183,7 @@ char *file_md5sum_alloc(const char *file
>>>   
>>>       return md5sum_hex;
>>>   }
>>> @@ -66,7 +70,7 @@
>>>   char *file_sha256sum_alloc(const char *file_name)
>>>  --- a/libopkg/opkg_install.c
>>>  +++ b/libopkg/opkg_install.c
>>> -@@ -1082,7 +1082,7 @@
>>> +@@ -1082,7 +1082,7 @@ resolve_conffiles(pkg_t *pkg)
>>>        conffile_list_elt_t *iter;
>>>        conffile_t *cf;
>>>        char *cf_backup;
>>> @@ -75,11 +79,11 @@
>>>   
>>>        if (conf->noaction) return 0;
>>>   
>>> -@@ -1093,7 +1093,11 @@
>>> +@@ -1093,7 +1093,11 @@ resolve_conffiles(pkg_t *pkg)
>>>   
>>>   	  /* Might need to initialize the md5sum for each conffile */
>>>   	  if (cf->value == NULL) {
>>> -+#ifdef HAVE_MD5
>>> ++#ifdef PREFER_MD5
>>
>> why is this change needed ? and does PREFER_MD5 actually exit ?
> 
> It does not exist :-) Thought about adding option for it but in the end I
> personally don't need it and it is still quite easy to override and I'm happy
> that I managed to create config above. I still want to use sha sums as
> default. That's why shasum stayed here. Point of the patch is to add
> possibility to migrate from old setup, not to mimic old behaviour.
> 

can you remove it please if it is not needed or define PREFER_MD5 to 0
somewhere

	John


>>>   	       cf->value = file_md5sum_alloc(root_filename);
>>>  +#else
>>>  +	       cf->value = file_sha256sum_alloc(root_filename);
>>> @@ -87,14 +91,18 @@
>>>   	  }
>>>   
>>>   	  if (!file_exists(root_filename)) {
>>> -@@ -1105,8 +1109,12 @@
>>> +@@ -1105,8 +1109,16 @@ resolve_conffiles(pkg_t *pkg)
>>>   
>>>             if (file_exists(cf_backup)) {
>>>                 /* Let's compute md5 to test if files are changed */
>>>  -              md5sum = file_md5sum_alloc(cf_backup);
>>>  -              if (md5sum && cf->value && strcmp(cf->value,md5sum) != 0 ) {
>>>  +#ifdef HAVE_MD5
>>> -+              chksum = file_md5sum_alloc(cf_backup);
>>> ++              if(cf->value && strlen(cf->value) > 33) {
>>> ++                  chksum = file_sha256sum_alloc(cf_backup);
>>> ++              } else {
>>> ++                  chksum = file_md5sum_alloc(cf_backup);
>>> ++              }
>>>  +#else
>>>  +              chksum = file_sha256sum_alloc(cf_backup);
>>>  +#endif
>>> @@ -102,7 +110,7 @@
>>>                     if (conf->force_maintainer) {
>>>                         opkg_msg(NOTICE, "Conffile %s using maintainer's setting.\n",
>>>   				      cf_backup);
>>> -@@ -1123,8 +1131,8 @@
>>> +@@ -1123,8 +1135,8 @@ resolve_conffiles(pkg_t *pkg)
>>>   		  }
>>>                 }
>>>                 unlink(cf_backup);
>>> @@ -113,7 +121,7 @@
>>>             }
>>>   
>>>   	  free(cf_backup);
>>> -@@ -1323,6 +1331,7 @@
>>> +@@ -1323,6 +1335,7 @@ opkg_install_pkg(pkg_t *pkg, int from_up
>>>        }
>>>        #endif
>>>   
>>> @@ -121,7 +129,7 @@
>>>        /* Check for md5 values */
>>>        if (pkg->md5sum)
>>>        {
>>> -@@ -1346,6 +1355,7 @@
>>> +@@ -1346,6 +1359,7 @@ opkg_install_pkg(pkg_t *pkg, int from_up
>>>   	 if (file_md5)
>>>                 free(file_md5);
>>>        }
>>> @@ -131,7 +139,7 @@
>>>        /* Check for sha256 value */
>>>  --- a/libopkg/Makefile.am
>>>  +++ b/libopkg/Makefile.am
>>> -@@ -25,13 +25,16 @@
>>> +@@ -25,13 +25,16 @@ opkg_list_sources = conffile.c conffile.
>>>   		    pkg_src.c pkg_src.h pkg_src_list.c pkg_src_list.h \
>>>   		    str_list.c str_list.h void_list.c void_list.h \
>>>   		    active_list.c active_list.h list.h 
>>> @@ -151,11 +159,23 @@
>>>   endif
>>>  --- a/configure.ac
>>>  +++ b/configure.ac
>>> -@@ -72,6 +72,7 @@
>>> +@@ -68,10 +68,19 @@ AC_ARG_ENABLE(sha256,
>>> +       (sha256.{c,h} are GPLv3 licensed) [[default=no]] ]),
>>> +     [want_sha256="$enableval"], [want_sha256="no"])
>>> + 
>>> ++AC_ARG_ENABLE(md5,
>>> ++              AC_HELP_STRING([--enable-md5], [Enable md5sum check
>>> ++      (md5.{c,h} are GPLv3 licensed) [[default=no]] ]),
>>> ++    [want_md5="$enableval"], [want_md5="yes"])
>>> ++
>>> + if test "x$want_sha256" = "xyes"; then
>>>     AC_DEFINE(HAVE_SHA256, 1, [Define if you want sha256 support])
>>>   fi
>>> ++if test "x$want_md5" = "xyes"; then
>>> ++  AC_DEFINE(HAVE_MD5, 1, [Define if you want md5 support])
>>> ++fi
>>>   AM_CONDITIONAL(HAVE_SHA256, test "x$want_sha256" = "xyes")
>>> -+AM_CONDITIONAL(HAVE_MD5, test "x$want_sha256" = "xno")
>>> ++AM_CONDITIONAL(HAVE_MD5, test "x$want_md5" = "xyes")
>>>   
>>>   # check for openssl
>>>   AC_ARG_ENABLE(openssl,
>>>
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel



More information about the openwrt-devel mailing list