From 23069d10341ce637fdad7321d447c53752dba48c Mon Sep 17 00:00:00 2001

From: Nikias Bassen <nikias@gmx.li>

Date: Fri, 4 Nov 2016 02:11:39 +0100

Subject: [PATCH] userpref: [GnuTLS] Fix pairing record generation and improve

 error handling

In newer GnuTLS versions the parameters supplied to

gnutls_x509_privkey_import_rsa_raw() are actually checked for somewhat

sane values. Since we were passing the same values for all parameters,

this check fails and the device certificate is never generated.

However due to missing checks the pairing record was saved anyway, with

an empty device certificate. This led to TLS errors during communication,

leading to the "GnuTLS: Error in pull function" error message appearing

and the communication to fail.

This commit fixes the issue by passing some sane values, and also improves

the overall error handling during generation of the paring record.

---

 common/userpref.c | 85 +++++++++++++++++++++++++++++--------------------------

 1 file changed, 45 insertions(+), 40 deletions(-)

diff --git a/common/userpref.c b/common/userpref.c

index d22c7f5..3ae503a 100644

--- a/common/userpref.c

+++ b/common/userpref.c

@@ -643,15 +643,13 @@ userpref_error_t pair_record_generate_keys_and_certs(plist_t pair_record, key_da

 	gnutls_x509_crt_export(host_cert, GNUTLS_X509_FMT_PEM, host_cert_pem.data, &host_cert_export_size);

 	host_cert_pem.size = host_cert_export_size;

-	ret = USERPREF_E_UNKNOWN_ERROR;

-

 	gnutls_datum_t modulus = { NULL, 0 };

 	gnutls_datum_t exponent = { NULL, 0 };

 	/* now decode the PEM encoded key */

-	gnutls_datum_t der_pub_key;

-	if (GNUTLS_E_SUCCESS == gnutls_pem_base64_decode_alloc("RSA PUBLIC KEY", &public_key, &der_pub_key)) {

-

+	gnutls_datum_t der_pub_key = { NULL, 0 };

+	int gnutls_error = gnutls_pem_base64_decode_alloc("RSA PUBLIC KEY", &public_key, &der_pub_key);

+	if (GNUTLS_E_SUCCESS == gnutls_error) {

 		/* initalize asn.1 parser */

 		ASN1_TYPE pkcs1 = ASN1_TYPE_EMPTY;

 		if (ASN1_SUCCESS == asn1_array2tree(pkcs1_asn1_tab, &pkcs1, NULL)) {

@@ -670,8 +668,14 @@ userpref_error_t pair_record_generate_keys_and_certs(plist_t pair_record, key_da

 				ret1 = asn1_read_value(asn1_pub_key, "modulus", modulus.data, (int*)&modulus.size);

 				ret2 = asn1_read_value(asn1_pub_key, "publicExponent", exponent.data, (int*)&exponent.size);

-				if (ASN1_SUCCESS == ret1 && ASN1_SUCCESS == ret2)

-					ret = USERPREF_E_SUCCESS;

+				if (ret1 != ASN1_SUCCESS || ret2 != ASN1_SUCCESS) {

+					gnutls_free(modulus.data);

+					modulus.data = NULL;

+					modulus.size = 0;

+					gnutls_free(exponent.data);

+					exponent.data = NULL;

+					exponent.size = 0;

+				}

 			}

 			if (asn1_pub_key)

 				asn1_delete_structure(&asn1_pub_key);

@@ -679,12 +683,15 @@ userpref_error_t pair_record_generate_keys_and_certs(plist_t pair_record, key_da

 		if (pkcs1)

 			asn1_delete_structure(&pkcs1);

 	} else {

-		debug_info("WARNING: Could not read public key");

+		debug_info("ERROR: Could not parse public key: %s", gnutls_strerror(gnutls_error));

 	}

-	/* now generate certificates */

-	if (USERPREF_E_SUCCESS == ret && 0 != modulus.size && 0 != exponent.size) {

-		gnutls_datum_t essentially_null = { (unsigned char*)strdup("abababababababab"), strlen("abababababababab") };

+	/* generate device certificate */

+	if (modulus.data && 0 != modulus.size && exponent.data && 0 != exponent.size) {

+

+		gnutls_datum_t prime_p = { (unsigned char*)"\x00\xca\x4a\x03\x13\xdf\x9d\x7a\xfd", 9 };

+		gnutls_datum_t prime_q = { (unsigned char*)"\x00\xf2\xff\xe0\x15\xd1\x60\x37\x63", 9 };

+		gnutls_datum_t coeff = { (unsigned char*)"\x32\x07\xf1\x68\x57\xdf\x9a\xf4", 8 };

 		gnutls_x509_privkey_t fake_privkey;

 		gnutls_x509_crt_t dev_cert;

@@ -692,8 +699,9 @@ userpref_error_t pair_record_generate_keys_and_certs(plist_t pair_record, key_da

 		gnutls_x509_privkey_init(&fake_privkey);

 		gnutls_x509_crt_init(&dev_cert);

-		if (GNUTLS_E_SUCCESS == gnutls_x509_privkey_import_rsa_raw(fake_privkey, &modulus, &exponent, &essentially_null, &essentially_null, &essentially_null, &essentially_null)) {

-			/* generate device certificate */

+		gnutls_error = gnutls_x509_privkey_import_rsa_raw(fake_privkey, &modulus, &exponent, &exponent, &prime_p, &prime_q, &coeff);

+		if (GNUTLS_E_SUCCESS == gnutls_error) {

+			/* now generate device certificate */

 			gnutls_x509_crt_set_key(dev_cert, fake_privkey);

 			gnutls_x509_crt_set_serial(dev_cert, "\x00", 1);

 			gnutls_x509_crt_set_version(dev_cert, 3);

@@ -712,9 +720,8 @@ userpref_error_t pair_record_generate_keys_and_certs(plist_t pair_record, key_da

 			}

 			gnutls_x509_crt_set_key_usage(dev_cert, GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT);

-			gnutls_x509_crt_sign(dev_cert, root_cert, root_privkey);

-

-			if (USERPREF_E_SUCCESS == ret) {

+			gnutls_error = gnutls_x509_crt_sign(dev_cert, root_cert, root_privkey);

+			if (GNUTLS_E_SUCCESS == gnutls_error) {

 				/* if everything went well, export in PEM format */

 				size_t export_size = 0;

 				gnutls_x509_crt_export(dev_cert, GNUTLS_X509_FMT_PEM, NULL, &export_size);

@@ -722,13 +729,11 @@ userpref_error_t pair_record_generate_keys_and_certs(plist_t pair_record, key_da

 				gnutls_x509_crt_export(dev_cert, GNUTLS_X509_FMT_PEM, dev_cert_pem.data, &export_size);

 				dev_cert_pem.size = export_size;

 			} else {

-				debug_info("ERROR: Signing device certificate with root private key failed!");

+				debug_info("ERROR: Signing device certificate with root private key failed: %s", gnutls_strerror(gnutls_error));

 			}

+		} else {

+			debug_info("ERROR: Failed to import RSA key data: %s", gnutls_strerror(gnutls_error));

 		}

-

-		if (essentially_null.data)

-			free(essentially_null.data);

-

 		gnutls_x509_crt_deinit(dev_cert);

 		gnutls_x509_privkey_deinit(fake_privkey);

 	}

@@ -743,27 +748,27 @@ userpref_error_t pair_record_generate_keys_and_certs(plist_t pair_record, key_da

 	gnutls_free(der_pub_key.data);

 #endif

-	if (NULL != root_cert_pem.data && 0 != root_cert_pem.size &&

-		NULL != host_cert_pem.data && 0 != host_cert_pem.size)

+

+	/* make sure that we have all we need */

+	if (root_cert_pem.data && 0 != root_cert_pem.size

+	    && root_key_pem.data && 0 != root_key_pem.size

+	    && host_cert_pem.data && 0 != host_cert_pem.size

+	    && host_key_pem.data && 0 != host_key_pem.size

+	    && dev_cert_pem.data && 0 != dev_cert_pem.size) {

+		/* now set keys and certificates */

+		pair_record_set_item_from_key_data(pair_record, USERPREF_DEVICE_CERTIFICATE_KEY, &dev_cert_pem);

+		pair_record_set_item_from_key_data(pair_record, USERPREF_HOST_PRIVATE_KEY_KEY, &host_key_pem);

+		pair_record_set_item_from_key_data(pair_record, USERPREF_HOST_CERTIFICATE_KEY, &host_cert_pem);

+		pair_record_set_item_from_key_data(pair_record, USERPREF_ROOT_PRIVATE_KEY_KEY, &root_key_pem);

+		pair_record_set_item_from_key_data(pair_record, USERPREF_ROOT_CERTIFICATE_KEY, &root_cert_pem);

 		ret = USERPREF_E_SUCCESS;

+	}

-	/* now set keys and certificates */

-	pair_record_set_item_from_key_data(pair_record, USERPREF_DEVICE_CERTIFICATE_KEY, &dev_cert_pem);

-	pair_record_set_item_from_key_data(pair_record, USERPREF_HOST_PRIVATE_KEY_KEY, &host_key_pem);

-	pair_record_set_item_from_key_data(pair_record, USERPREF_HOST_CERTIFICATE_KEY, &host_cert_pem);

-	pair_record_set_item_from_key_data(pair_record, USERPREF_ROOT_PRIVATE_KEY_KEY, &root_key_pem);

-	pair_record_set_item_from_key_data(pair_record, USERPREF_ROOT_CERTIFICATE_KEY, &root_cert_pem);

-

-	if (dev_cert_pem.data)

-		free(dev_cert_pem.data);

-	if (root_key_pem.data)

-		free(root_key_pem.data);

-	if (root_cert_pem.data)

-		free(root_cert_pem.data);

-	if (host_key_pem.data)

-		free(host_key_pem.data);

-	if (host_cert_pem.data)

-		free(host_cert_pem.data);

+	free(dev_cert_pem.data);

+	free(root_key_pem.data);

+	free(root_cert_pem.data);

+	free(host_key_pem.data);

+	free(host_cert_pem.data);

 	return ret;

 }

-- 

2.9.3