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.

diff --git a/NEWS b/NEWS
index eee6dd652c2d3d31419eaea496711a8d99340037..aa7f8d0595de997810fa9878979caa14dd3fc640 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,9 @@ See the end for copying conditions.
 
 * Version 2.9.10 (unreleased)
 
+** libgnutls: When checking self signature also check the signatures of all subkeys.                                                                                                             
+Ilari Liusvaara noticed and reported the issue and provided test vectors as well.
+
 ** libgnutls: Added cryptodev support (/dev/crypto). Tested with
 http://www.logix.cz/michal/devel/cryptodev/. Added benchmark utility 
 for AES. Exported API to access encryption and hash algorithms.
@@ -18,6 +21,7 @@ them does not seem to offer anything (anyway you need the signer's certificate
 to verify thus public key will be available). Found thanks to Boyan Kasarov.
 This however has the side-effect that public key IDs shown by certtool are
 now different than previous gnutls releases.
+(3) the option --pgp-certificate-info will verify self signatures
 
 ** certtool: Allow exporting of Certificate requests on DER format.
 

diff --git a/lib/opencdk/sig-check.c b/lib/opencdk/sig-check.c
index 5dc9da6fd16b9da1fdb97d8477516e4a5aac8fc4..f4e8ccf0f0dc28c155ff0372dae1297477579300 100644 (file)
--- 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 826aff70d1e7497cdeaa5002a601cc25d7ae6487..8f373606c241adb423e321ed972832220573aaa0 100644 (file)
--- a/src/certtool.c
+++ b/src/certtool.c
@@ -1094,6 +1094,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;
@@ -1120,6 +1121,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)
@@ -1129,7 +1146,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 d437dc51bedd708bcf6597b8b6a7f72982d83403..76fbf3a82b818f9adea18c481fa529a680099405 100644 (file)
--- 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 (file)
index 0000000..dd4dab1
Binary files /dev/null and b/tests/openpgp-certs/selfsigs/alice-mallory-badsig18.pub differ

diff --git a/tests/openpgp-certs/selfsigs/alice-mallory-irrelevantsig.pub b/tests/openpgp-certs/selfsigs/alice-mallory-irrelevantsig.pub
new file mode 100644 (file)
index 0000000..03caa9d
Binary files /dev/null and b/tests/openpgp-certs/selfsigs/alice-mallory-irrelevantsig.pub differ

diff --git a/tests/openpgp-certs/selfsigs/alice-mallory-nosig18.pub b/tests/openpgp-certs/selfsigs/alice-mallory-nosig18.pub
new file mode 100644 (file)
index 0000000..59f077a
Binary files /dev/null and b/tests/openpgp-certs/selfsigs/alice-mallory-nosig18.pub differ

diff --git a/tests/openpgp-certs/selfsigs/alice.pub b/tests/openpgp-certs/selfsigs/alice.pub
new file mode 100644 (file)
index 0000000..399a0ba
Binary files /dev/null and b/tests/openpgp-certs/selfsigs/alice.pub differ

diff --git a/tests/openpgp-certs/testselfsigs b/tests/openpgp-certs/testselfsigs
new file mode 100755 (executable)
index 0000000..51acd86
--- /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}