When checking self signature also check the signatures of all subkeys.

Ilari Liusvaara noticed and reported the issue and provided test vectors as well.

certtool --pgp-certificate-info will check self signatures.

Added self tests for self-sigs.

9 files changed, 96 insertions, 39 deletions

diff --git a/NEWS b/NEWS
index b36d363d99..60ad12b51d 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,9 @@ See the end for copying conditions.
 
 * Version 2.8.6 (unreleased)
 
+** libgnutls: When checking openpgp self signature also check the signatures of all subkeys.                                                                                                             
+Ilari Liusvaara noticed and reported the issue and provided test vectors as well.
+
 ** libgnutls: Cleanups and several bug fixes.
 Found by Steve Grubb and Tomas Mraz.
 
diff --git a/lib/opencdk/sig-check.c b/lib/opencdk/sig-check.c
index 38007299fa..9cebf164c4 100644
--- a/lib/opencdk/sig-check.c
+++ b/lib/opencdk/sig-check.c
@@ -288,7 +288,8 @@ _cdk_pk_check_sig (cdk_keydb_hd_t keydb,
 
   if (is_selfsig)
     *is_selfsig = 0;
-  if (knode->pkt->pkttype != CDK_PKT_PUBLIC_KEY ||
+  if ((knode->pkt->pkttype != CDK_PKT_PUBLIC_KEY &&
+      knode->pkt->pkttype != CDK_PKT_PUBLIC_SUBKEY) ||
       snode->pkt->pkttype != CDK_PKT_SIGNATURE)
     {
       gnutls_assert ();
@@ -591,45 +592,56 @@ cdk_pk_check_self_sig (cdk_kbnode_t key, int *r_status)
   cdk_error_t rc;
   u32 keyid[2], sigid[2];
   int is_selfsig, sig_ok;
+  cdk_kbnode_t p, ctx = NULL;
+  cdk_packet_t pkt;
 
   if (!key || !r_status)
     return CDK_Inv_Value;
 
-  node = cdk_kbnode_find (key, CDK_PKT_PUBLIC_KEY);
-  if (!node)
-    return CDK_Error_No_Key;
-  /* FIXME: we should set expire/revoke here also but callers
-     expect CDK_KEY_VALID=0 if the key is okay. */
   cdk_pk_get_keyid (key->pkt->pkt.public_key, keyid);
-  sig_ok = 0;
-  for (node = key; node; node = node->next)
-    {
-      if (node->pkt->pkttype != CDK_PKT_SIGNATURE)
-	continue;
-      sig = node->pkt->pkt.signature;
-      if (!IS_UID_SIG (sig))
-	continue;
-      cdk_sig_get_keyid (sig, sigid);
-      if (sigid[0] != keyid[0] || sigid[1] != keyid[1])
-	continue;
-      /* FIXME: Now we check all self signatures. */
-      rc = _cdk_pk_check_sig (NULL, key, node, &is_selfsig, NULL);
-      if (rc)
-	{
-	  *r_status = CDK_KEY_INVALID;
-	  return rc;
-	}
-      else			/* For each valid self sig we increase this counter. */
-	sig_ok++;
-    }
 
-  /* A key without a self signature is not valid. */
-  if (!sig_ok)
+  while ((p = cdk_kbnode_walk (key, &ctx, 0)))
     {
-      *r_status = CDK_KEY_INVALID;
-      return CDK_General_Error;
+      pkt = cdk_kbnode_get_packet (p);
+      if (pkt->pkttype != CDK_PKT_PUBLIC_SUBKEY && pkt->pkttype != CDK_PKT_PUBLIC_KEY)
+        continue;
+
+      /* FIXME: we should set expire/revoke here also but callers
+         expect CDK_KEY_VALID=0 if the key is okay. */
+      sig_ok = 0;
+      for (node = p; node; node = node->next)
+        {
+          if (node->pkt->pkttype != CDK_PKT_SIGNATURE)
+  	    continue;
+          sig = node->pkt->pkt.signature;
+
+          cdk_sig_get_keyid (sig, sigid);
+          if (sigid[0] != keyid[0] || sigid[1] != keyid[1])
+	    continue;
+          /* FIXME: Now we check all self signatures. */
+          rc = _cdk_pk_check_sig (NULL, p, node, &is_selfsig, NULL);
+          if (rc)
+	    {
+  	      *r_status = CDK_KEY_INVALID;
+	      return rc;
+  	    }
+          else			/* For each valid self sig we increase this counter. */
+	    sig_ok++;
+        }
+
+      /* A key without a self signature is not valid. At least one
+       * signature for the given key has to be found.
+       */
+      if (!sig_ok)
+        {
+          *r_status = CDK_KEY_INVALID;
+          return CDK_General_Error;
+        }
+
     }
-  /* No flags indicate a valid key. */
-  *r_status = CDK_KEY_VALID;
-  return 0;
+
+    /* No flags indicate a valid key. */
+    *r_status = CDK_KEY_VALID;
+
+    return 0;
 }
diff --git a/src/certtool.c b/src/certtool.c
index c4e92faf1a..5693cf2bb2 100644
--- a/src/certtool.c
+++ b/src/certtool.c
@@ -1088,6 +1088,7 @@ pgp_certificate_info (void)
   size_t size;
   int ret;
   gnutls_datum_t pem, out_data;
+  unsigned int verify_status;
 
   pem.data = fread_file (infile, &size);
   pem.size = size;
@@ -1114,6 +1115,22 @@ pgp_certificate_info (void)
 	}
     }
 
+
+  ret = gnutls_openpgp_crt_verify_self(crt, 0, &verify_status);
+  if (ret < 0) 
+    {
+      error (EXIT_FAILURE, 0, "verify signature error: %s", gnutls_strerror (ret));
+    }
+
+  if (verify_status & GNUTLS_CERT_INVALID)
+    {
+      fprintf (outfile, "Self Signature verification: failed\n\n");
+    }
+  else
+    {
+      fprintf (outfile, "Self Signature verification: ok (%x)\n\n", verify_status);
+    }
+
   size = sizeof (buffer);
   ret = gnutls_openpgp_crt_export (crt, info.outcert_format, buffer, &size);
   if (ret < 0)
@@ -1123,7 +1140,6 @@ pgp_certificate_info (void)
     }
 
   fprintf (outfile, "%s\n", buffer);
-
   gnutls_openpgp_crt_deinit (crt);
 }
 
diff --git a/tests/openpgp-certs/Makefile.am b/tests/openpgp-certs/Makefile.am
index d437dc51be..76fbf3a82b 100644
--- a/tests/openpgp-certs/Makefile.am
+++ b/tests/openpgp-certs/Makefile.am
@@ -21,13 +21,15 @@ if ENABLE_OPENPGP
 
 EXTRA_DIST = ca-public.gpg srv-public-all-signed.gpg srv-secret.gpg	\
 	ca-secret.gpg srv-public.gpg srv-public-127.0.0.1-signed.gpg	\
-	srv-public-localhost-signed.gpg
+	srv-public-localhost-signed.gpg selfsigs/alice-mallory-badsig18.pub \
+	selfsigs/alice-mallory-irrelevantsig.pub selfsigs/alice-mallory-nosig18.pub \
+	selfsigs/alice.pub
 
 # The selftest is disabled until we can make it work under Wine and
 # under Debian buildds (problem with 127.0.0.2?).  Just extra-dist it
 # for now.
-EXTRA_DIST += testcerts
-#dist_check_SCRIPTS = testcerts
-#TESTS = testcerts
+EXTRA_DIST += testcerts testselfsigs
+dist_check_SCRIPTS = testselfsigs #testcerts
+TESTS = testselfsigs #testcerts
 
 endif
diff --git a/tests/openpgp-certs/selfsigs/alice-mallory-badsig18.pub b/tests/openpgp-certs/selfsigs/alice-mallory-badsig18.pub
new file mode 100644
index 0000000000..dd4dab1156
--- /dev/null
+++ b/tests/openpgp-certs/selfsigs/alice-mallory-badsig18.pub
diff --git a/tests/openpgp-certs/selfsigs/alice-mallory-irrelevantsig.pub b/tests/openpgp-certs/selfsigs/alice-mallory-irrelevantsig.pub
new file mode 100644
index 0000000000..03caa9d122
--- /dev/null
+++ b/tests/openpgp-certs/selfsigs/alice-mallory-irrelevantsig.pub
diff --git a/tests/openpgp-certs/selfsigs/alice-mallory-nosig18.pub b/tests/openpgp-certs/selfsigs/alice-mallory-nosig18.pub
new file mode 100644
index 0000000000..59f077afcb
--- /dev/null
+++ b/tests/openpgp-certs/selfsigs/alice-mallory-nosig18.pub
diff --git a/tests/openpgp-certs/selfsigs/alice.pub b/tests/openpgp-certs/selfsigs/alice.pub
new file mode 100644
index 0000000000..399a0ba4ec
--- /dev/null
+++ b/tests/openpgp-certs/selfsigs/alice.pub
diff --git a/tests/openpgp-certs/testselfsigs b/tests/openpgp-certs/testselfsigs
new file mode 100755
index 0000000000..51acd86ef4
--- /dev/null
+++ b/tests/openpgp-certs/testselfsigs
@@ -0,0 +1,24 @@
+#!/bin/bash
+
+srcdir="${srcdir:-.}"
+CERTTOOL="${certtool:-../../src/certtool} -q"
+unset RETCODE
+
+fail() {
+   echo "Failure: $1" >&2
+   RETCODE=${RETCODE:-${2:-1}}
+}
+
+echo "Checking OpenPGP certificate self verification"
+
+( $CERTTOOL --pgp-certificate-info <$srcdir/selfsigs/alice.pub | grep -e "^Self Signature verification: ok" ) >/dev/null ||
+  fail "Self sig Verification should have succeeded!"
+
+( $CERTTOOL --pgp-certificate-info <$srcdir/selfsigs/alice-mallory-badsig18.pub | grep -e "^Self Signature verification: failed" ) >/dev/null ||
+  fail "Self sig Verification should have failed!"
+( $CERTTOOL --pgp-certificate-info <$srcdir/selfsigs/alice-mallory-irrelevantsig.pub | grep -e "^Self Signature verification: failed" ) >/dev/null ||
+  fail "Self sig Verification should have failed!"
+( $CERTTOOL --pgp-certificate-info <$srcdir/selfsigs/alice-mallory-nosig18.pub | grep -e "^Self Signature verification: failed" ) >/dev/null ||
+  fail "Self sig Verification should have failed!"
+
+exit ${RETCODE:-0}