Initial support for asignify signatures

Message ID 20211226220108.13797-1-jeremy@merelinux.org
State Superseded, archived
Headers show
Series Initial support for asignify signatures | expand

Commit Message

Jeremy Huntwork Dec. 26, 2021, 10:01 p.m. UTC
From: Jeremy Huntwork <jeremy@merelinux.org>

This is a proof of concept that shows how asignify can be used instead
of gpgme to validate packages signed with the asignify tool.

Signed-off-by: Jeremy Huntwork <jeremy@merelinux.org>
---
 lib/libalpm/alpm.c       |  2 +-
 lib/libalpm/be_package.c | 10 ++++++
 lib/libalpm/be_sync.c    |  2 +-
 lib/libalpm/handle.c     |  6 ++--
 lib/libalpm/signing.c    | 67 ++++++++++++++++++++++++++++++++++++++++
 lib/libalpm/signing.h    | 11 ++++---
 meson.build              | 11 ++++++-
 meson_options.txt        |  3 ++
 8 files changed, 101 insertions(+), 11 deletions(-)

Comments

Jeremy Huntwork Dec. 26, 2021, 10:18 p.m. UTC | #1
On Sun, Dec 26, 2021 at 5:01 PM <jeremy@merelinux.org> wrote:
>
> From: Jeremy Huntwork <jeremy@merelinux.org>
>
> This is a proof of concept that shows how asignify can be used instead
> of gpgme to validate packages signed with the asignify tool.

Just want to reiterate that this is a proof of concept. There is one
hard-coded path in there for now, the location of the trusted public
keys. That at the very least will need to be updated to a
configuration option. I also noticed that there are one or two
auto-formatting changes here that I didn't intend to include. Anyway,
thanks for reviewing. Looking forward to the feedback.

JH
Allan McRae Jan. 1, 2022, 6:19 a.m. UTC | #2
On 27/12/21 08:01, jeremy@merelinux.org wrote:
> From: Jeremy Huntwork <jeremy@merelinux.org>
> 
> This is a proof of concept that shows how asignify can be used instead
> of gpgme to validate packages signed with the asignify tool.
> 

I'm not doing a full review of the patch (although I note some pure 
whitespace changes creeping in).

This looks a reasonable approach to include.  Note it would need to be 
setup as an either/or with gpgme, much like the openssl/nettle 
implementations are.

If you go ahead and work this into a formal patchset submission (please 
not all one patch!), I can provide specific comments.

Cheers,
Allan
Danilo Jan. 10, 2022, 10:40 a.m. UTC | #3
On 27/12/21 08:01, jeremy at merelinux.org wrote:
> From: Jeremy Huntwork <jeremy at merelinux.org>
> 
> This is a proof of concept that shows how asignify can be used instead
> of gpgme to validate packages signed with the asignify tool.

Nice! This is the first time I'm hearing of asignify. It seems to have
similar goals to Minisign[1] by jedisct1 (maintainer of libsodium).

Minisign is backwards compatible with signify when using the legacy
signature format (PureEdDSA), but is using a blake2b based pre-hashed
approach by default (HashEdDSA). A comparison of the two formats can be
found here[2].

asignify also seems to make use of blake2b, however I don't know
in what form. The signature schemes are probably not compatible, right?
In my bubble Minisign seemed to gain some traction lately, and
according to pkgs.org it also seems to be much more widely packaged
than asignify. Is there a particular reason why you picked asignify?
(The dependencies seem to be simpler, libsodium vs tweetnacl.)

There's more prior art, Debian recently started developing AptSign to
replace OpenPGP: [3]

Cheers,
Danilo

[1] https://jedisct1.github.io/minisign/
[2] https://github.com/jedisct1/minisign/releases/tag/0.6
[3] https://wiki.debian.org/Teams/Apt/Spec/AptSign
Jeremy Huntwork Jan. 10, 2022, 1:31 p.m. UTC | #4
On Mon, Jan 10, 2022 at 5:41 AM Danilo <mail@dbrgn.ch> wrote:
> Is there a particular reason why you picked asignify?
> (The dependencies seem to be simpler, libsodium vs tweetnacl.)

Yeah, basically because it seemed simpler to integrate into pacman,
and had no external dependencies. Its API is very straightforward. I
looked at minisign as well and that too would be a fine solution. It
does seem like it has wider use.

JH

Patch

diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c
index 5b326ab0..d1261660 100644
--- a/lib/libalpm/alpm.c
+++ b/lib/libalpm/alpm.c
@@ -137,7 +137,7 @@  int SYMEXPORT alpm_capabilities(void)
 #ifdef HAVE_LIBCURL
 		| ALPM_CAPABILITY_DOWNLOADER
 #endif
-#ifdef HAVE_LIBGPGME
+#if defined(HAVE_LIBGPGME) || defined(HAVE_LIBASIGNIFY)
 		| ALPM_CAPABILITY_SIGNATURES
 #endif
 		| 0;
diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c
index 5ca2865c..1b6887ce 100644
--- a/lib/libalpm/be_package.c
+++ b/lib/libalpm/be_package.c
@@ -342,12 +342,20 @@  int _alpm_pkg_validate_internal(alpm_handle_t *handle,
 			handle->pm_errno = ALPM_ERR_PKG_MISSING_SIG;
 			return -1;
 		}
+#ifdef HAVE_LIBGPGME
 		if(_alpm_check_pgp_helper(handle, pkgfile, sig,
 					level & ALPM_SIG_PACKAGE_OPTIONAL, level & ALPM_SIG_PACKAGE_MARGINAL_OK,
 					level & ALPM_SIG_PACKAGE_UNKNOWN_OK, sigdata)) {
 			handle->pm_errno = ALPM_ERR_PKG_INVALID_SIG;
 			return -1;
 		}
+#endif
+#ifdef HAVE_LIBASIGNIFY
+		if(_alpm_check_asignify_helper(handle, pkgfile, sigdata)) {
+			handle->pm_errno = ALPM_ERR_PKG_INVALID_SIG;
+			return -1;
+		}
+#endif
 		if(validation && has_sig) {
 			*validation |= ALPM_PKG_VALIDATION_SIGNATURE;
 		}
@@ -743,6 +751,7 @@  int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const char *filename, int ful
 				return -1;
 			}
 
+#ifndef HAVE_LIBASIGNIFY
 			if(alpm_extract_keyid(handle, filename, sig, len, &keys) == 0) {
 				alpm_list_t *k;
 				for(k = keys; k; k = k->next) {
@@ -771,6 +780,7 @@  int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const char *filename, int ful
 				free(sigpath);
 				return -1;
 			}
+#endif
 		}
 	}
 	free(sigpath);
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index ede25985..81654902 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -697,7 +697,7 @@  alpm_db_t *_alpm_db_register_sync(alpm_handle_t *handle, const char *treename,
 
 	_alpm_log(handle, ALPM_LOG_DEBUG, "registering sync database '%s'\n", treename);
 
-#ifndef HAVE_LIBGPGME
+#if !defined(HAVE_LIBGPGME) || !defined(HAVE_LIBASIGNIFY)
 	if(level != 0 && level != ALPM_SIG_USE_DEFAULT) {
 		RET_ERR(handle, ALPM_ERR_MISSING_CAPABILITY_SIGNATURES, NULL);
 	}
diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
index 101d4a78..78c7d7bd 100644
--- a/lib/libalpm/handle.c
+++ b/lib/libalpm/handle.c
@@ -829,7 +829,7 @@  int SYMEXPORT alpm_option_set_default_siglevel(alpm_handle_t *handle,
 	if(level == ALPM_SIG_USE_DEFAULT) {
 		RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1);
 	}
-#ifdef HAVE_LIBGPGME
+#if defined(HAVE_LIBGPGME) || defined(HAVE_LIBASIGNIFY)
 	handle->siglevel = level;
 #else
 	if(level != 0) {
@@ -849,7 +849,7 @@  int SYMEXPORT alpm_option_set_local_file_siglevel(alpm_handle_t *handle,
 		int level)
 {
 	CHECK_HANDLE(handle, return -1);
-#ifdef HAVE_LIBGPGME
+#if defined(HAVE_LIBGPGME) || defined(HAVE_LIBASIGNIFY)
 	handle->localfilesiglevel = level;
 #else
 	if(level != 0 && level != ALPM_SIG_USE_DEFAULT) {
@@ -873,7 +873,7 @@  int SYMEXPORT alpm_option_set_remote_file_siglevel(alpm_handle_t *handle,
 		int level)
 {
 	CHECK_HANDLE(handle, return -1);
-#ifdef HAVE_LIBGPGME
+#if defined(HAVE_LIBGPGME) || defined(HAVE_LIBASIGNIFY)
 	handle->remotefilesiglevel = level;
 #else
 	if(level != 0 && level != ALPM_SIG_USE_DEFAULT) {
diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c
index 66cc3923..cf778f9d 100644
--- a/lib/libalpm/signing.c
+++ b/lib/libalpm/signing.c
@@ -26,6 +26,12 @@ 
 #include <gpgme.h>
 #endif
 
+#ifdef HAVE_LIBASIGNIFY
+/* libasignify */
+#include <asignify.h>
+#include <dirent.h>
+#endif
+
 /* libalpm */
 #include "signing.h"
 #include "package.h"
@@ -810,6 +816,67 @@  char *_alpm_sigpath(alpm_handle_t *handle, const char *path)
 	return sigpath;
 }
 
+/**
+ * Helper for checking the asignify signature for the given file path.
+ * @param handle the context handle
+ * @param path the full path to a file
+ * @param sigdata a pointer to storage for signature results
+ * @return 0 on success, -1 on error (consult pm_errno or sigdata)
+ */
+int _alpm_check_asignify_helper(alpm_handle_t *handle, const char *path,
+		alpm_siglist_t **sigdata)
+{
+		int ret = 0;
+		struct dirent *entry;
+		struct stat statbuf;
+		const char *pubkeydir = "/etc/pacman.d/trustedkeys"; /* Add config for this */
+
+		char *sigpath = _alpm_sigpath(handle, path);
+		asignify_verify_t *vrf = asignify_verify_init();
+
+		DIR *pkd = opendir(pubkeydir);
+		if(pkd == NULL) {
+			_alpm_log(handle, ALPM_LOG_DEBUG, "cannot open directory: %s\n", pubkeydir);
+			return -1;
+		}
+
+		while((entry = readdir(pkd)) != NULL) {
+			char *fullpath = malloc(strlen(pubkeydir) + strlen(entry->d_name) + 2);
+			if (fullpath == NULL) {
+				_alpm_log(handle, ALPM_LOG_DEBUG, "malloc error\n");
+				return -1;
+			}
+			sprintf(fullpath, "%s/%s", pubkeydir, entry->d_name);
+			stat(fullpath, &statbuf);
+			if (S_ISREG(statbuf.st_mode)) {
+				if (!asignify_verify_load_pubkey(vrf, fullpath)) {
+					/* Don't return here because there may be multiple public keys to load. */
+					_alpm_log(handle, ALPM_LOG_DEBUG, "cannot load public key file: %s\n", fullpath);
+				}
+			}
+			free(fullpath);
+		}
+
+		if (!asignify_verify_load_signature(vrf, sigpath)) {
+			_alpm_log(handle, ALPM_LOG_DEBUG, "cannot load signature\n");
+			ret = -1;
+			goto asignify_cleanup;
+		}
+
+		if (!asignify_verify_file(vrf, path)) {
+			_alpm_log(handle, ALPM_LOG_DEBUG, "signature is not valid\n");
+			ret = -1;
+			goto asignify_cleanup;
+		}
+
+		_alpm_log(handle, ALPM_LOG_DEBUG, "signature is valid\n");
+
+asignify_cleanup:
+		free(sigpath);
+		asignify_verify_free(vrf);
+		return ret;
+}
+
 /**
  * Helper for checking the PGP signature for the given file path.
  * This wraps #_alpm_gpgme_checksig in a slightly friendlier manner to simplify
diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h
index 112b2a1f..ce859373 100644
--- a/lib/libalpm/signing.h
+++ b/lib/libalpm/signing.h
@@ -23,13 +23,14 @@ 
 
 char *_alpm_sigpath(alpm_handle_t *handle, const char *path);
 int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path,
-		const char *base64_sig, alpm_siglist_t *result);
-
+						 const char *base64_sig, alpm_siglist_t *result);
+int _alpm_check_asignify_helper(alpm_handle_t *handle, const char *path,
+								alpm_siglist_t **sigdata);
 int _alpm_check_pgp_helper(alpm_handle_t *handle, const char *path,
-		const char *base64_sig, int optional, int marginal, int unknown,
-		alpm_siglist_t **sigdata);
+						   const char *base64_sig, int optional, int marginal, int unknown,
+						   alpm_siglist_t **sigdata);
 int _alpm_process_siglist(alpm_handle_t *handle, const char *identifier,
-		alpm_siglist_t *siglist, int optional, int marginal, int unknown);
+						  alpm_siglist_t *siglist, int optional, int marginal, int unknown);
 
 int _alpm_key_in_keychain(alpm_handle_t *handle, const char *fpr);
 int _alpm_key_import(alpm_handle_t *handle, const char *uid, const char *fpr);
diff --git a/meson.build b/meson.build
index 76b9d2aa..fc9a6f1c 100644
--- a/meson.build
+++ b/meson.build
@@ -105,6 +105,14 @@  gpgme = dependency('gpgme',
                    not_found_message : 'gpgme @0@ is needed for GPG signature support'.format(needed_gpgme_version))
 conf.set('HAVE_LIBGPGME', gpgme.found())
 
+needed_libasignify_version = '>=1.0'
+libasignify = dependency('libasignify',
+                   version : needed_libasignify_version,
+                   required : get_option('asignify'),
+                   static : get_option('buildstatic'),
+                   not_found_message : 'libasignify @0@ is needed for asignify signature support'.format(needed_libasignify_version))
+conf.set('HAVE_LIBASIGNIFY', libasignify.found())
+
 want_crypto = get_option('crypto')
 if want_crypto == 'openssl'
   libcrypto = dependency('libcrypto', static : get_option('buildstatic'),
@@ -305,7 +313,7 @@  libcommon = static_library(
   gnu_symbol_visibility : 'hidden',
   install : false)
 
-alpm_deps = [crypto_provider, libarchive, libcurl, libintl, gpgme]
+alpm_deps = [crypto_provider, libarchive, libasignify, libcurl, libintl, gpgme]
 
 libalpm_a = static_library(
   'alpm_objlib',
@@ -453,6 +461,7 @@  message('\n    '.join([
   '  i18n support             : @0@'.format(get_option('i18n')),
   '  Build docs               : @0@'.format(build_doc),
   '  debug build              : @0@'.format(get_option('buildtype') == 'debug'),
+  '  Use libasignify          : @0@'.format(conf.get('HAVE_LIBASIGNIFY')),
   '  Use libcurl              : @0@'.format(conf.get('HAVE_LIBCURL')),
   '  Use GPGME                : @0@'.format(conf.get('HAVE_LIBGPGME')),
   '  Use OpenSSL              : @0@'.format(conf.has('HAVE_LIBSSL') and
diff --git a/meson_options.txt b/meson_options.txt
index 4d8cc300..a254a5d4 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -48,6 +48,9 @@  option('crypto', type : 'combo', choices : ['openssl', 'nettle'],
 option('gpgme', type : 'feature', value : 'auto',
        description : 'use GPGME for PGP signature verification')
 
+option('asignify', type : 'feature', value : 'auto',
+       description : 'use asignify for signature verification')
+
 option('i18n', type : 'boolean', value : true,
        description : 'enable localization of pacman, libalpm and scripts')