subkey binding signature with no usage flags and/or a critical notation

[Daniel Kahn Gillmor]

On Fri 2013-03-15 10:52:16 -0400, Werner Koch wrote:

> On Thu, 14 Mar 2013 17:05, dkg at fifthhorseman.net said:
>> On 03/13/2013 06:27 PM, David Shaw wrote:
>>> Yes. Having no flags set at all is treated as if there is no subpacket present.  This may not be the best behavior.
>>
>> yeah, i think this needs fixing.
>
> What about this untested patch against master:
>
> commit 8f8f3984e82a025cf1384132a419f67f39c7e07d 
> Author: Werner Koch <wk at gnupg.org>
> Date:   Fri Mar 15 15:46:03 2013 +0100
>
>     gpg: Distinguish between missing and cleared key flags.
>     
>     * include/cipher.h (PUBKEY_USAGE_NONE): New.
>     * g10/getkey.c (parse_key_usage): Set new flag.
>     --
>     
>     We do not want to use the default capabilities (derived from the
>     algorithm) if any key flags are given in a signature.  Thus if key
>     flags are used in any way, the default key capabilities are never
>     used.
>     
>     This allows to create a key with key flags set to all zero so it can't
>     be used.  This better reflects common sense.
>
> 	Modified g10/getkey.c
> diff --git a/g10/getkey.c b/g10/getkey.c
> index 9294273..8cc5601 100644
> --- a/g10/getkey.c
> +++ b/g10/getkey.c
> @@ -1276,13 +1276,19 @@ parse_key_usage (PKT_signature * sig)
>  
>        if (flags)
>  	key_usage |= PUBKEY_USAGE_UNKNOWN;
> +
> +      if (!key_usage)
> +	key_usage |= PUBKEY_USAGE_NONE;
>      }
> +  else if (p) /* Key flags of length zero.  */
> +    key_usage |= PUBKEY_USAGE_NONE;
>  
>    /* We set PUBKEY_USAGE_UNKNOWN to indicate that this key has a
>       capability that we do not handle.  This serves to distinguish
>       between a zero key usage which we handle as the default
>       capabilities for that algorithm, and a usage that we do not
> -     handle. */
> +     handle.  Likewise we use PUBKEY_USAGE_NONE to indicate that
> +     key_flags have been given but they do not specify any usage.  */
>  
>    return key_usage;
>  }
> 	Modified include/cipher.h
> diff --git a/include/cipher.h b/include/cipher.h
> index 191e197..557ab70 100644
> --- a/include/cipher.h
> +++ b/include/cipher.h
> @@ -54,9 +54,14 @@
>  
>  #define PUBKEY_USAGE_SIG     GCRY_PK_USAGE_SIGN  /* Good for signatures. */
>  #define PUBKEY_USAGE_ENC     GCRY_PK_USAGE_ENCR  /* Good for encryption. */
> -#define PUBKEY_USAGE_CERT    GCRY_PK_USAGE_CERT  /* Also good to certify keys. */
> +#define PUBKEY_USAGE_CERT    GCRY_PK_USAGE_CERT  /* Also good to certify keys.*/
>  #define PUBKEY_USAGE_AUTH    GCRY_PK_USAGE_AUTH  /* Good for authentication. */
>  #define PUBKEY_USAGE_UNKNOWN GCRY_PK_USAGE_UNKN  /* Unknown usage flag. */
> +#define PUBKEY_USAGE_NONE    256                 /* No usage given. */
> +#if  (GCRY_PK_USAGE_SIGN | GCRY_PK_USAGE_ENCR | GCRY_PK_USAGE_CERT \
> +      | GCRY_PK_USAGE_AUTH | GCRY_PK_USAGE_UNKN) >= 256
> +# error Please choose another value for PUBKEY_USAGE_NONE
> +#endif
>  
>  #define DIGEST_ALGO_MD5       /*  1 */ GCRY_MD_MD5
>  #define DIGEST_ALGO_SHA1      /*  2 */ GCRY_MD_SHA1
>

As i reported earlier, this fix works fine.  I've backported it against
1.4.14 and it works there too (below).  It does not seem to be applied
to the stable branches (1.4 and 2.0).  I think it needs to be applied
to the stable branches.

Without this patch, if a subkey with a usage flags subpacket that is
all-zero appears, GnuPG thinks that the key is valid for all
capabilities (usage: SCEA).

This is dangerous in several ways:

 * if it's your own key, GnuPG will use it to sign messages or certify
   other OpenPGP certificates

 * if it's someone else's key, GnuPG will encrypt to it

 * if it's someone else's key, GnuPG may be willing to rely on OpenPGP
   certifications made by the key, despite the owner having clearly
   marked that it is not acceptable for that use.

Depending on how such a key is actually used in practice, this may cause
unrelated confidential data to be revealed, or it may let GnuPG follow
spoofable paths through the WoT.  It can can also cause invalid
signatures to be issued, which has already affected me in as a mini
self-DoS (i have a primary key with a subkey with such an empty
usage-flags subpacket that i have avoided publishing out of wariness; i
sent a message to an automated service which requires signed mail, and
the message was rejected because it was made by this subkey).

While very few people have such keys in practice right now (i might be
the only person experimenting with them -- i'm trying to circumscribe
the use of experimental subkeys to much more specific application
domains than the usage flags permit), it would be bad if these keys
continue to misinterpreted by GnuPG.

I'm inclined to see this as a security vulnerability (because of the
confidentiality and certification-following concerns); i'd like to see
it fixed in the stable branches and assigned a CVE, so that i can push
for getting it resolved in those distros that care about CVE coverage.

Any objections to this plan?  I am happy to request the CVE myself, and
will report it back here once it is assigned.

     --dkg

-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-empty-usage-flags.patch
Type: text/x-diff
Size: 2015 bytes
Desc: handle keys with empty usage flag arguments properly
URL: </pipermail/attachments/20130911/da7cac88/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 965 bytes
Desc: not available
URL: </pipermail/attachments/20130911/da7cac88/attachment.sig>