[gnutls-dev] GnuTLS Fix: Decrypting PKCS#8 privkey with invalid password has same error as parse failure

[Simon Josefsson]

Nate Nielsen <nielsen-list at memberwebs.com> writes:

> I'm working on using gnutls to parse certificates and keys in
> gnome-keyring.
>
> I've come across a problem where a invalid password for a PKCS#8
> encrypted key produces the same error as a ASN.1 parse failure.
>
> Attached is a patch which fixes the problem in a way that I hope makes
> sense. Any ASN.1 parse failures that late in the game, and immediately
> after decryption are treated as an invalid password.
>
> Given the terseness of DER encoded data, I don't think that there is a
> reliable (and quick) way of guaranteeing this in all extreme corner
> cases. However I believe that the fix makes gnutls more correct.
>
> Attached are test cases and patch.

Patch installed on 1.6.x branch, even though it is close to the ~10
lines limit that would require copyright assignments.  The self-test is
longer than 10 lines, though, and papers are required for it.  I'd like
to include it in 1.7.x with your co-operation, see the response to your
other report.

Thanks,
Simon

> In order for this to work, the patch from my earlier bug report (about
> PKCS#8 key parsing not reporting parse failures) will need to be applied.
>
> Cheers,
> Nate Nielsen
>
> /* gcc -g -O0 -o gnutls-test `pkg-config --libs --cflags gnutls` gnutls-test-pkcs8-password.c */
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
>
> #include <gnutls/gnutls.h>
> #include <gnutls/x509.h>
> #include <gcrypt.h>
>
> static void
> read_file (const char *file, gnutls_datum_t *datum)
> {
> 	FILE *f = fopen (file, "rb");
> 	if (f) {
> 		datum->data = malloc (8192);
> 		datum->size = fread (datum->data, 1, 8192, f);
> 	}
> 	if (!f || ferror (f))
> 		err (1, "couldn't read from file: %s", file);
> 	fclose (f);
> }
>
> int
> main (int argc, char **argv)
> {
> 	gnutls_x509_privkey_t privkey;
> 	gnutls_datum_t datum;
> 	int gerr;
>
> 	if (argc < 2)
> 		errx (1, "specify key to parse");
>
> 	gcry_check_version (GCRYPT_VERSION);
> 	gnutls_global_init ();
>
> 	read_file (argv[1], &datum);
> 				
> 	gnutls_x509_privkey_init (&privkey);
>
> 	gerr = gnutls_x509_privkey_import_pkcs8 (privkey, &datum, 
> 			GNUTLS_X509_FMT_DER, "test", GNUTLS_PKCS_PLAIN);
> 	printf ("no password: %d\n", gerr);
> 	printf ("       algo: %d\n", gnutls_x509_privkey_get_pk_algorithm (privkey));		
>
>
> 	gerr = gnutls_x509_privkey_import_pkcs8 (privkey, &datum, 
> 			GNUTLS_X509_FMT_DER, "test", GNUTLS_PKCS_USE_PBES2_3DES);
> 	printf ("pasword 'test': %d\n", gerr);		
> 	printf ("          algo: %d\n", gnutls_x509_privkey_get_pk_algorithm (privkey));		
>
> 	gerr = gnutls_x509_privkey_import_pkcs8 (privkey, &datum, 
> 			GNUTLS_X509_FMT_DER, "bork", GNUTLS_PKCS_USE_PBES2_3DES);
> 	printf ("pasword 'bork': %d\n", gerr);		
> 	printf ("          algo: %d\n", gnutls_x509_privkey_get_pk_algorithm (privkey));		
> 	
> 	gnutls_global_deinit ();
> 	return 0;
> }
>
> --- lib/x509/privkey_pkcs8.c.orig	2007-05-23 22:11:51.000000000 -0000
> +++ lib/x509/privkey_pkcs8.c	2007-05-23 22:43:49.000000000 -0000
> @@ -739,6 +739,25 @@
>  
>    if (result < 0)
>      {
> +      /* We've gotten this far. In the real world it's almost certain
> +       * that we're dealing with a good file, but wrong password. 
> +       * Sadly like 90% of random data is somehow valid DER for the 
> +       * a first small number of bytes, so no easy way to guarantee. */
> +      if (result == GNUTLS_E_ASN1_ELEMENT_NOT_FOUND ||
> +          result == GNUTLS_E_ASN1_IDENTIFIER_NOT_FOUND ||
> +          result == GNUTLS_E_ASN1_DER_ERROR ||
> +          result == GNUTLS_E_ASN1_VALUE_NOT_FOUND ||
> +          result == GNUTLS_E_ASN1_GENERIC_ERROR ||
> +          result == GNUTLS_E_ASN1_VALUE_NOT_VALID ||
> +          result == GNUTLS_E_ASN1_TAG_ERROR || 
> +          result == GNUTLS_E_ASN1_TAG_IMPLICIT ||
> +          result == GNUTLS_E_ASN1_TYPE_ANY_ERROR ||
> +          result == GNUTLS_E_ASN1_SYNTAX_ERROR ||
> +          result == GNUTLS_E_ASN1_DER_OVERFLOW)
> +        {
> +           result = GNUTLS_E_DECRYPTION_FAILED;
> +        }
> +
>        gnutls_assert ();
>        goto error;
>      }