release 2.5.1: -S flag on p11keygen + bug fix for EC public key import

Author: keldonin

CHANGELOG.md

@@ -4,8 +4,9 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-# Unreleased
+# [2.5.1]
 - adding `-S` option flag for `p11keygen`, for enabling key generation when logged in as Security Officer (PR #33)
+- fixed a few memory management issues, preventing to import EC public keys when using `p11keygen`, `p11unwrap` and `p11importpubk`.
 
 # [2.5.0]
 ### Added
@@ -127,6 +128,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - Initial public release
 
+[2.5.1]: https://github.com/Mastercard/pkcs11-tools/tree/v2.5.1
 [2.5.0]: https://github.com/Mastercard/pkcs11-tools/tree/v2.5.0
 [2.4.2]: https://github.com/Mastercard/pkcs11-tools/tree/v2.4.2
 [2.4.1]: https://github.com/Mastercard/pkcs11-tools/tree/v2.4.1

configure.ac

@@ -14,7 +14,7 @@ dnl limitations under the License.
 
 
 AC_PREREQ([2.64])
-AC_INIT([pkcs11-tools], [2.5.0], [https://github.com/Mastercard/pkcs11-tools/issues], [pkcs11-tools], [https://github.com/Mastercard/pkcs11-tools])
+AC_INIT([pkcs11-tools], [2.5.1], [https://github.com/Mastercard/pkcs11-tools/issues], [pkcs11-tools], [https://github.com/Mastercard/pkcs11-tools])
 AC_CONFIG_MACRO_DIR([m4])
 
 dnl adding AM_MAINTAINER_MODE to address autotools issues with git

docs/INSTALL.md

@@ -134,11 +134,11 @@ On previous FreeBSD versions, you will have to build it. Deploy first the OpenSS
 ```bash
 $ pkg install openssl
 ```
-Then proceed as with Linux. 
+Then proceed as with Linux. Note that clang should be used instead of gcc.
 
 If you had to install OpenSSL differently (e.g. older versions of FreeBSD), and if the path to OpenSSL libraries is not configured on the system, you need to specify an additional parameter (`LIBCRYPTO_RPATH`) when configuring the pkcs11-tools package, to set a run path to the libraries. See [rtld(1)](https://www.freebsd.org/cgi/man.cgi?query=rtld&apropos=0&sektion=1&manpath=FreeBSD+12.0-RELEASE&arch=default&format=html) for more information.
 ```bash
-$ ./configure PKG_CONFIG_PATH=/opt/openssl-1.1.1/lib/pkgconfig LIBCRYPTO_RPATH=/opt/openssl-1.1.1/lib
+$ ./configure CC=clang PKG_CONFIG_PATH=/opt/openssl-1.1.1/lib/pkgconfig LIBCRYPTO_RPATH=/opt/openssl-1.1.1/lib
 $ make
 $ sudo make install
 ```

lib/pkcs11_pubk.c

@@ -453,6 +453,7 @@ static CK_ULONG get_EC_point(EVP_PKEY *pubkey, CK_BYTE_PTR *buf)
     EC_KEY* ec=NULL;
     int i2dlen=0;
     unsigned char *octp = NULL, *octbuf = NULL;
+    ASN1_OCTET_STRING *wrapped = NULL;
 
     if ( pubkey && EVP_PKEY_base_id(pubkey)==EVP_PKEY_EC  ) {
 
@@ -508,7 +509,7 @@ static CK_ULONG get_EC_point(EVP_PKEY *pubkey, CK_BYTE_PTR *buf)
 	/* DER-encoded of point in octbuf  */
 	/* now wrap this into OCTET_STRING */
 
-	ASN1_OCTET_STRING *wrapped = ASN1_OCTET_STRING_new();
+	wrapped = ASN1_OCTET_STRING_new();
 
 	if(wrapped==NULL) {
 	    P_ERR();
@@ -551,6 +552,7 @@ static CK_ULONG get_EC_point(EVP_PKEY *pubkey, CK_BYTE_PTR *buf)
     }
 error:
     if(octbuf != NULL) { OPENSSL_free(octbuf); }
+    if(wrapped != NULL) { ASN1_OCTET_STRING_free(wrapped); }
 
     return rv;
 }
@@ -624,50 +626,50 @@ static CK_ULONG get_ED_point(EVP_PKEY *pubkey, CK_BYTE_PTR *buf)
 
     const uint8_t *pk;
     int pklen;
-    
+
     X509_PUBKEY_get0_param(NULL, &pk, &pklen, NULL, x509_pk); /* nothing to test, always returns 1 */
 
     if( (point = ASN1_OCTET_STRING_new()) == NULL ) {
 	P_ERR();
 	goto error;
     }
     ASN1_OCTET_STRING_set(point, pk, pklen); /* assign */
-    
+
     len = i2d_ASN1_OCTET_STRING(point, buf);
     if(len<0) {
 	P_ERR();
 	goto error;
     }
 
     rv = len;
-    
+
 error:
     if(point) { ASN1_OCTET_STRING_free(point); }
     if(x509_pk) { X509_PUBKEY_free(x509_pk); }
-    if(pkeybuf) { OPENSSL_free(pkeybuf); } 
+    if(pkeybuf) { OPENSSL_free(pkeybuf); }
     return rv;
 }
 
 static CK_ULONG get_ED_params(EVP_PKEY *pubkey, CK_BYTE_PTR *buf)
 {
     CK_ULONG rv = 0;
     ASN1_OBJECT *obj = NULL;
-    
+
     obj = OBJ_nid2obj(EVP_PKEY_base_id(pubkey));
     if(!obj) {
 	P_ERR();
 	goto error;
     }
 
-    assert( *buf == NULL );		/* make sure we point to nowhere */    
+    assert( *buf == NULL );		/* make sure we point to nowhere */
     int len = i2d_ASN1_OBJECT(obj, buf);
     if(len<0) {
 	P_ERR();
 	goto error;
     }
 
     rv = len;
-    
+
 error:
     if(obj) { ASN1_OBJECT_free(obj); }
     return rv;
@@ -870,7 +872,6 @@ static CK_ULONG get_EVP_PKEY_sha1(EVP_PKEY *pubkey, CK_BYTE_PTR *buf) {
 		    }
 		    /* get0 on ec_point & ec_group, we can safely forget */
 		}
-		EC_KEY_free(ec);
 	    }
 	}
 	    break;
@@ -904,7 +905,7 @@ static CK_ULONG get_EVP_PKEY_sha1(EVP_PKEY *pubkey, CK_BYTE_PTR *buf) {
 	    }
 	}
 	    break;
-	    
+
 
 	case EVP_PKEY_DH: {
 	    DH *dh;
@@ -1043,7 +1044,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 		{0L, NULL, 0L},
 		{0L, NULL, 0L},
 	    };
-	    
+
 	    size_t pubk_template_len_max = (sizeof(pubktemplate)/sizeof(CK_ATTRIBUTE));
 	    size_t pubk_template_len_min = pubk_template_len_max - 12;
 	    size_t pubk_num_elems = pubk_template_len_min;
@@ -1077,7 +1078,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 		    case CKA_VERIFY:
 		    case CKA_VERIFY_RECOVER: /* not in template onwards */
 		    case CKA_DERIVE:
-		    case CKA_TRUSTED: 
+		    case CKA_TRUSTED:
 		    case CKA_PRIVATE:
 		    case CKA_WRAP_TEMPLATE:
 		    case CKA_COPYABLE:
@@ -1089,7 +1090,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 		    case CKA_PUBLIC_KEY_INFO:
 		    {
 			size_t next_pubk_num_elems = pubk_num_elems;
-			
+
 			CK_ATTRIBUTE_PTR match = lsearch( &attrs[i],
 							  pubktemplate,
 							  &next_pubk_num_elems,
@@ -1108,7 +1109,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 			    else {
 				/* everything was copied by lsearch */
 				/* just increment array length */
-				pubk_num_elems = next_pubk_num_elems;      
+				pubk_num_elems = next_pubk_num_elems;
 			    }
 			} else {
 			    fprintf(stderr, "***Error: can't update attribute array - skipping 0x%08lx\n", attrs[i].type);
@@ -1125,7 +1126,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 			break;
 		    }
 		}
-		
+
 		retCode = p11Context->FunctionList.C_CreateObject(p11Context->Session,
 								  pubktemplate,
 								  pubk_num_elems,
@@ -1229,7 +1230,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 		    case CKA_VERIFY:
 		    case CKA_VERIFY_RECOVER: /* not in template onwards */
 		    case CKA_DERIVE:
-		    case CKA_TRUSTED: 
+		    case CKA_TRUSTED:
 		    case CKA_PRIVATE:
 		    case CKA_COPYABLE:
 		    case CKA_MODIFIABLE:
@@ -1240,7 +1241,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 		    case CKA_PUBLIC_KEY_INFO:
 		    {
 			size_t next_pubk_num_elems = pubk_num_elems;
-			
+
 			CK_ATTRIBUTE_PTR match = lsearch( &attrs[i],
 							  pubktemplate,
 							  &next_pubk_num_elems,
@@ -1259,7 +1260,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 			    else {
 				/* everything was copied by lsearch */
 				/* just increment array length */
-				pubk_num_elems = next_pubk_num_elems;      
+				pubk_num_elems = next_pubk_num_elems;
 			    }
 			} else {
 			    fprintf(stderr, "***Error: can't update attribute array - skipping 0x%08lx\n", attrs[i].type);
@@ -1380,7 +1381,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 		    case CKA_PUBLIC_KEY_INFO:
 		    {
 			size_t next_pubk_num_elems = pubk_num_elems;
-			
+
 			CK_ATTRIBUTE_PTR match = lsearch( &attrs[i],
 							  pubktemplate,
 							  &next_pubk_num_elems,
@@ -1399,7 +1400,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 			    else {
 				/* everything was copied by lsearch */
 				/* just increment array length */
-				pubk_num_elems = next_pubk_num_elems;      
+				pubk_num_elems = next_pubk_num_elems;
 			    }
 			} else {
 			    fprintf(stderr, "***Error: can't update attribute array - skipping 0x%08lx\n", attrs[i].type);
@@ -1416,7 +1417,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 			break;
 		    }
 		}
-		
+
 		retCode = p11Context->FunctionList.C_CreateObject(p11Context->Session,
 								  pubktemplate,
 								  pubk_num_elems,
@@ -1500,7 +1501,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 		    case CKA_VERIFY:
 		    case CKA_VERIFY_RECOVER: /* not in template onwards */
 		    case CKA_DERIVE:
-		    case CKA_TRUSTED: 
+		    case CKA_TRUSTED:
 		    case CKA_PRIVATE:
 		    case CKA_COPYABLE:
 		    case CKA_MODIFIABLE:
@@ -1511,7 +1512,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 		    case CKA_PUBLIC_KEY_INFO:
 		    {
 			size_t next_pubk_num_elems = pubk_num_elems;
-			
+
 			CK_ATTRIBUTE_PTR match = lsearch( &attrs[i],
 							  pubktemplate,
 							  &next_pubk_num_elems,
@@ -1530,7 +1531,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 			    else {
 				/* everything was copied by lsearch */
 				/* just increment array length */
-				pubk_num_elems = next_pubk_num_elems;      
+				pubk_num_elems = next_pubk_num_elems;
 			    }
 			} else {
 			    fprintf(stderr, "***Error: can't update attribute array - skipping 0x%08lx\n", attrs[i].type);
@@ -1547,7 +1548,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 			break;
 		    }
 		}
-		
+
 		retCode = p11Context->FunctionList.C_CreateObject(p11Context->Session,
 								  pubktemplate,
 								  pubk_num_elems,
@@ -1595,9 +1596,9 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 		{0L, NULL, 0L},
 		{0L, NULL, 0L},
 		{0L, NULL, 0L},
-		{0L, NULL, 0L},		
 		{0L, NULL, 0L},
-		{0L, NULL, 0L},		
+		{0L, NULL, 0L},
+		{0L, NULL, 0L},
 	    };
 
 	    size_t pubk_template_len_max = (sizeof(pubktemplate)/sizeof(CK_ATTRIBUTE));
@@ -1633,7 +1634,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 		    case CKA_VERIFY:
 		    case CKA_VERIFY_RECOVER: /* not in template onwards */
 		    case CKA_DERIVE:
-		    case CKA_TRUSTED: 
+		    case CKA_TRUSTED:
 		    case CKA_PRIVATE:
 		    case CKA_WRAP_TEMPLATE:
 		    case CKA_COPYABLE:
@@ -1645,7 +1646,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 		    case CKA_PUBLIC_KEY_INFO:
 		    {
 			size_t next_pubk_num_elems = pubk_num_elems;
-			
+
 			CK_ATTRIBUTE_PTR match = lsearch( &attrs[i],
 							  pubktemplate,
 							  &next_pubk_num_elems,
@@ -1664,7 +1665,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 			    else {
 				/* everything was copied by lsearch */
 				/* just increment array length */
-				pubk_num_elems = next_pubk_num_elems;      
+				pubk_num_elems = next_pubk_num_elems;
 			    }
 			} else {
 			    fprintf(stderr, "***Error: can't update attribute array - skipping 0x%08lx\n", attrs[i].type);
@@ -1681,7 +1682,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 			break;
 		    }
 		}
-		
+
 		retCode = p11Context->FunctionList.C_CreateObject(p11Context->Session,
 								  pubktemplate,
 								  pubk_num_elems,
@@ -1705,7 +1706,7 @@ static CK_OBJECT_HANDLE _importpubk( pkcs11Context * p11Context,
 	    break;
 	}
 
-	OPENSSL_free(pubk);
+	EVP_PKEY_free(pubk);
 
     }
     return pubkhandle;
@@ -1734,4 +1735,3 @@ inline CK_OBJECT_HANDLE pkcs11_importpubk_from_buffer( pkcs11Context * p11Contex
 						       CK_ULONG numattrs ) {
     return _importpubk(p11Context, NULL, buffer, len, label, attrs, numattrs, source_buffer);
 }
-