From: Nikos Mavrogiannopoulos Date: Tue, 12 Jan 2010 17:34:39 +0000 (+0100) Subject: When checking self signature also check the signatures of all subkeys. X-Git-Tag: gnutls_2_9_10~175 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3275e1c09bc9c74946ebdaa87c71b71371aeccc4;p=thirdparty%2Fgnutls.git 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 eee6dd652c..aa7f8d0595 100644 --- 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 5dc9da6fd1..f4e8ccf0f0 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 826aff70d1..8f373606c2 100644 --- 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 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 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 index 0000000000..03caa9d122 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 index 0000000000..59f077afcb 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 index 0000000000..399a0ba4ec 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 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}