[OpenWrt-Devel] [PATCH] [packages] opkg: Make opkg understand old md5
Michal Hrusecky
Michal.Hrusecky at nic.cz
Fri Feb 26 03:10:30 EST 2016
John Crispin - 8:40 26.02.16 wrote:
> Hi,
>
> few comments inline
>
> On 25/02/2016 13:39, Michal Hrusecky wrote:
> > Opkg now uses sha256 by default and expects them. Making it optionally
> > understand md5s also and detect md5 sum so we can migrate from configuration
> > that used md5.
> >
> > Signed-off-by: Michal Hrusecky <Michal.Hrusecky at nic.cz>
> > ---
> > package/system/opkg/Makefile | 33 +++++++++++++-
> > .../system/opkg/patches/230-drop_md5_support.patch | 52 +++++++++++++++-------
> > 2 files changed, 68 insertions(+), 17 deletions(-)
> >
> > diff --git a/package/system/opkg/Makefile b/package/system/opkg/Makefile
> > index e46c6b3..9b2aca1 100644
> > --- a/package/system/opkg/Makefile
> > +++ b/package/system/opkg/Makefile
> > @@ -12,7 +12,7 @@ include $(INCLUDE_DIR)/feeds.mk
> > PKG_NAME:=opkg
> > PKG_REV:=9c97d5ecd795709c8584e972bfdf3aee3a5b846d
> > PKG_VERSION:=$(PKG_REV)
> > -PKG_RELEASE:=10
> > +PKG_RELEASE:=11
> >
> > PKG_SOURCE_PROTO:=git
> > PKG_SOURCE_VERSION:=$(PKG_REV)
> > @@ -43,6 +43,7 @@ define Package/opkg/Default
> > TITLE:=opkg package manager
> > DEPENDS:=+uclient-fetch
> > URL:=http://wiki.openmoko.org/wiki/Opkg
> > + MENU:=1
> > endef
> >
> > define Package/opkg/Default/description
> > @@ -55,6 +56,16 @@ define Package/opkg/Default/description
> > opkg knows how to install both .ipk and .deb packages.
> > endef
> >
> > +define Package/opkg/config
> > +config OPKG_SUPPORT_MD5
> > + bool
> > + default n
> > + depends on PACKAGE_opkg
> > + prompt "Support reading old md5 hashes."
> > + help
> > + Old opkg used md5s, new uses sha. This options enables understanding both while prefering sha.
> > +endef
> > +
> > define Package/opkg
> > $(call Package/opkg/Default)
> > VARIANT:=unsigned
> > @@ -84,6 +95,16 @@ define Package/opkg-smime/description
> > This package allows the Package index to be verified with S/MIME.
> > endef
> >
> > +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.
> > +
> > 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.
> > 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.
> > ++ 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.
> > 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