diff mbox

opkg: Add option to enable package verification support with GnuPG

Message ID 64891469.UzFDExKfyb@arawn
State Superseded
Headers show

Commit Message

Philipp Claves Feb. 28, 2013, 2:04 p.m. UTC
Hello,

This set of patches enables the opkg support for verifying package signatures 
with gnupg.

The first two patches add the required libraries (libassuan and libgpgme).
The third adds new option: opkg-gnupg-support. It selects the needed packages 
and sets the --enable-gpg configure option for opkg.

The patch dependency chain is:
  (0003) opkg -> (0002) libgpgme -> (0001) libassuan

Comments are welcome.

Regards,
Philipp Claves

Comments

Thomas Petazzoni Feb. 28, 2013, 2:38 p.m. UTC | #1
Dear Philipp Claves,

On Thu, 28 Feb 2013 15:04:42 +0100, Philipp Claves wrote:

> This set of patches enables the opkg support for verifying package signatures 
> with gnupg.
> 
> The first two patches add the required libraries (libassuan and libgpgme).
> The third adds new option: opkg-gnupg-support. It selects the needed packages 
> and sets the --enable-gpg configure option for opkg.
> 
> The patch dependency chain is:
>   (0003) opkg -> (0002) libgpgme -> (0001) libassuan
> 
> Comments are welcome.

Thanks, they look (almost) good! Could you submit them, one mail per
patch, preferably using git send-email?

On libassuan, please wrap the help text, and add a final newline at the
end of the .mk file.

On libgpgme, please wrap the help text, add the final newline at the
end of the .mk file. The --with-gpg=/usr/bin/gpg doesn't look very
good. gpg is not a mandatory dependency of Buildroot, so if libgpgme
really needs gpg, then we should build host-gnupg I guess. But does it
really need it? Can you detail this --with-gpg option? Or maybe it's
just the path where gpg will be installed on the target? If it's the
case, then maybe a comment would be appropriate to clarify this.

In the opkg patch, in the .mk file, you should use
BR2_PACKAGE_OPKG_GPG_SIGN in your condition rather than
BR2_PACKAGE_LIBGPGME. Also, in your Config.in, you add GNUPG as a
dependency, but it is not listed in the .mk file. This is sometimes
correct if it is only a runtime dependency, in which case we usually
put a comment in the Config.in just above the corresponding select line.

Thanks!

Thomas
Thomas Petazzoni March 1, 2013, 10:18 a.m. UTC | #2
Dear Philipp Claves,

Please keep the Buildroot list posted for this discussion. Thanks!

On Fri, 01 Mar 2013 10:55:44 +0100, Philipp Claves wrote:

> > Thanks, they look (almost) good! Could you submit them, one mail per
> > patch, preferably using git send-email?
> Never used that and having my mail server password (--smtp-pass) logged in 
> .bash_history does not sound too appealing. Is there a good reason to use it?

You don't have to put your password on the command line. You can put
your password in ~/.gitconfig:

[sendemail]
smtppass = foobar

and of course make this file 600.

Another solution is to install msmtp, and then tell git to use it. In
this case, you have your SMTP password in the msmtp configuration file,
~/.msmtprc, which should be 600.

There are good reasons to use git send-email.

 * It allows to send the patches inline instead of as attachements.
   This is something most open-source communities want, because it
   allows anyone to hit the "Reply" button and make some review
   comments inline. See
   http://lxr.free-electrons.com/source/Documentation/SubmittingPatches#L219.

 * When sent inline, many e-mail clients wrap the lines, or do some
   funk replacement of tabs with spaces or stuff like that, which makes
   your patch impossible to apply.

 * When sent inline, one patch per e-mail, all patches get
   automatically picked up by our Patchwork tracking system. For now,
   only your last patch has been seen by Patchwork:
   http://patchwork.ozlabs.org/patch/223967/.

So really: one e-mail per patch, and patch inline. And to achieve that,
git send-email is the best solution.

> > On libassuan, please wrap the help text, and add a final newline at the
> > end of the .mk file.
> 80 columns in 2013? Anyway, it will be fixed.

Yes, 80 columns in 2013. The help text gets displayed in "menuconfig",
it isn't wrapped automatically by menuconfig and we want it to fit on
reasonably sized windows.

I guess all good text editors know how to automatically wrap stuff at
~80 columns, so it's just a matter of hitting a keystroke.

> > On libgpgme, please wrap the help text, add the final newline at the
> > end of the .mk file.
> Will be fixed.

Thanks.

> > The --with-gpg=/usr/bin/gpg doesn't look very
> > good. gpg is not a mandatory dependency of Buildroot, so if libgpgme
> > really needs gpg, then we should build host-gnupg I guess. But does it
> > really need it? Can you detail this --with-gpg option? Or maybe it's
> > just the path where gpg will be installed on the target? If it's the
> > case, then maybe a comment would be appropriate to clarify this.
> It is the install path on the target. Will add a comment.

Ok, thanks.

> > In the opkg patch, in the .mk file, you should use
> > BR2_PACKAGE_OPKG_GPG_SIGN in your condition rather than
> > BR2_PACKAGE_LIBGPGME.
> Oops, this is a leftover from testing. Will be fixed.

Perfect.

> 
> > Also, in your Config.in, you add GNUPG as a
> > dependency, but it is not listed in the .mk file. This is sometimes
> > correct if it is only a runtime dependency, in which case we usually
> > put a comment in the Config.in just above the corresponding select line.
> The dependencies are a little strange. Neither the libs nor opkg actually 
> require gnupg to build, but libgpgme and therefore opkg signature support 
> don't work without the binary in place. libassuan is standalone.
> 
> My solution is to hide (depends on) libgpgme if gnupg is not selected (because 
> it makes no sense without), but force gpg and libgpgme on if you want opkg 
> signature support. This option is intended as a user friendly way to select 
> all you need for it.
> 
> As an alternative it would be possible to make libgpg always visible and let 
> it automatically select gnupg.

If libgpgme practically cannot operate without /usr/bin/gpg being
present, then I would do:

config BR2_PACKAGE_LIBGPGME
	[...]
	# gnupg needed at runtime, but not a build dependency
	select BR2_PACKAGE_GNUPG

This way, if one day some other package uses libgpgme, it will
automatically ensure that gnupg is built/installed.

Best regards,

Thomas
diff mbox

Patch

From be7802c497d4be7d618b5bd312532a011f77ac82 Mon Sep 17 00:00:00 2001
From: Philipp Claves <claves@budelmann-elektronik.com>
Date: Thu, 28 Feb 2013 14:54:55 +0100
Subject: [PATCH 3/3] opkg: Add gnupg signature checking support.

---
 package/opkg/Config.in | 10 ++++++++++
 package/opkg/opkg.mk   |  9 ++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/package/opkg/Config.in b/package/opkg/Config.in
index eb997a7..fd168e4 100644
--- a/package/opkg/Config.in
+++ b/package/opkg/Config.in
@@ -11,3 +11,13 @@  config BR2_PACKAGE_OPKG
 
 	  http://code.google.com/p/opkg/
 
+if BR2_PACKAGE_OPKG
+
+config BR2_PACKAGE_OPKG_GPG_SIGN
+	bool "opkg-gnupg-support"
+	select BR2_PACKAGE_LIBGPGME
+	select BR2_PACKAGE_GNUPG
+	help
+	  Enable opkg package signature checking support using gnupg/libgpgme.
+
+endif
diff --git a/package/opkg/opkg.mk b/package/opkg/opkg.mk
index 9932d3f..ae0ba49 100644
--- a/package/opkg/opkg.mk
+++ b/package/opkg/opkg.mk
@@ -9,7 +9,7 @@  OPKG_SOURCE = opkg-$(OPKG_VERSION).tar.gz
 OPKG_SITE = http://opkg.googlecode.com/svn/trunk/
 OPKG_SITE_METHOD = svn
 OPKG_INSTALL_STAGING = YES
-OPKG_CONF_OPT = --disable-curl --disable-gpg
+OPKG_CONF_OPT = --disable-curl
 OPKG_AUTORECONF = YES
 # Uses PKG_CHECK_MODULES() in configure.ac
 OPKG_DEPENDENCIES = host-pkgconf
@@ -19,6 +19,13 @@  define OPKG_CREATE_LOCKDIR
 	mkdir -p $(TARGET_DIR)/usr/lib/opkg
 endef
 
+ifeq ($(BR2_PACKAGE_LIBGPGME),y)
+	OPKG_CONF_OPT += --enable-gpg
+	OPKG_DEPENDENCIES += libgpgme
+else
+	OPKG_CONF_OPT += --disable-gpg
+endif
+
 OPKG_POST_INSTALL_TARGET_HOOKS += OPKG_CREATE_LOCKDIR
 
 $(eval $(autotools-package))
-- 
1.8.1.4