Remove PKCS11 single thread lock (parallaxsecond#264)

This commit removes the mutex that was used to force the PKCS11 provider to work in a single-threaded way. The issue was fixed in softhsm/SoftHSMv2#576 and our Dockerfiles have been updated to use that commit. Signed-off-by: Ionut Mihalcea <ionut.mihalcea@arm.com>
ionut-arm · Oct 22, 2020 · 23d9ee7 · 23d9ee7
1 parent 875c2b6
commit 23d9ee7
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 31 deletions.
diff --git a/e2e_tests/provider_cfg/all/Dockerfile b/e2e_tests/provider_cfg/all/Dockerfile
@@ -17,6 +17,15 @@ RUN cd mbed-crypto-mbedcrypto-2.0.0 \
 	&& make SHARED=0
 
 WORKDIR /tmp
+# Download and install SoftHSM2
+RUN git clone https://github.com/opendnssec/SoftHSMv2.git \
+	&& cd SoftHSMv2 \
+	&& git reset --hard 20a53bd083a6134ce2230f80edda5dc8be0366bd \
+    && sh autogen.sh \
+	&& ./configure --disable-gost \
+	&& make \
+	&& make install
+
 # Download and install TSS 2.0
 RUN git clone https://github.com/tpm2-software/tpm2-tss.git --branch 2.3.3
 RUN cd tpm2-tss \
@@ -33,15 +42,6 @@ RUN cd tpm2-tools \
 	&& ./configure --enable-unit \
 	&& make install
 
-WORKDIR /tmp
-RUN wget https://github.com/opendnssec/SoftHSMv2/archive/2.5.0.tar.gz
-RUN tar xf 2.5.0.tar.gz
-RUN cd SoftHSMv2-2.5.0 \
-	&& sh autogen.sh \
-	&& ./configure --disable-gost \
-	&& make \
-	&& make install
-
 # Create a new token in a new slot. The slot number assigned will be random
 # and is found with the find_slot_number script.
 RUN softhsm2-util --init-token --slot 0 --label "Parsec Tests" --pin 123456 --so-pin 123456

diff --git a/e2e_tests/provider_cfg/pkcs11/Dockerfile b/e2e_tests/provider_cfg/pkcs11/Dockerfile
@@ -10,13 +10,13 @@ RUN apt-get update && \
 	apt-get install -y pkg-config libssl-dev
 
 WORKDIR /tmp
-RUN wget https://github.com/opendnssec/SoftHSMv2/archive/2.5.0.tar.gz
-RUN tar xf 2.5.0.tar.gz
-RUN cd SoftHSMv2-2.5.0 \
-		&& sh autogen.sh \
-		&& ./configure --disable-gost \
-		&& make \
-		&& make install
+RUN git clone https://github.com/opendnssec/SoftHSMv2.git \
+	&& cd SoftHSMv2 \
+	&& git reset --hard 20a53bd083a6134ce2230f80edda5dc8be0366bd \
+    && sh autogen.sh \
+	&& ./configure --disable-gost \
+	&& make \
+	&& make install
 
 # Install Rust toolchain
 RUN curl https://sh.rustup.rs -sSf | bash -s -- -y

diff --git a/e2e_tests/src/stress.rs b/e2e_tests/src/stress.rs
@@ -315,7 +315,7 @@ impl ServiceChecker {
     }
 
     fn check_sign(client: &mut TestClient) {
-        let sign_key_name = String::from("checking_key");
+        let sign_key_name = String::from("sign_checking_key");
         info!("Verifying signing");
         client
             .generate_rsa_sign_key(sign_key_name.clone())
@@ -335,11 +335,11 @@ impl ServiceChecker {
     }
 
     fn check_encrypt(client: &mut TestClient) {
-        let encr_key_name = String::from("checking_key");
-        info!("Verifying signing");
+        let encr_key_name = String::from("encrypt_checking_key");
+        info!("Verifying encryption");
         client
             .generate_rsa_encryption_keys_rsapkcs1v15crypt(encr_key_name.clone())
-            .expect("Failed to create signing key");
+            .expect("Failed to create encryption key");
 
         let ciphertext = client
             .asymmetric_encrypt_message_with_rsapkcs1v15(encr_key_name.clone(), vec![0xa5; 16])

diff --git a/src/providers/pkcs11/mod.rs b/src/providers/pkcs11/mod.rs
@@ -70,8 +70,6 @@ pub struct Provider {
     slot_number: CK_SLOT_ID,
     // Some PKCS 11 devices do not need a pin, the None variant means that.
     user_pin: Option<SecretString>,
-    // TODO: Figure out why the SoftHSM2 throws errors when multithreading and remove this when fixed.
-    temp_mutex: Mutex<()>,
     software_public_operations: bool,
 }
 
@@ -95,7 +93,6 @@ impl Provider {
             backend,
             slot_number: slot_number as CK_SLOT_ID,
             user_pin,
-            temp_mutex: Mutex::new(()),
             software_public_operations,
         };
         {
@@ -233,7 +230,6 @@ impl Provide for Provider {
         app_name: ApplicationName,
         op: psa_generate_key::Operation,
     ) -> Result<psa_generate_key::Result> {
-        let _guard = self.temp_mutex.lock().expect("temp_mutex poisoned");
         trace!("psa_generate_key ingress");
         self.psa_generate_key_internal(app_name, op)
     }
@@ -243,7 +239,6 @@ impl Provide for Provider {
         app_name: ApplicationName,
         op: psa_import_key::Operation,
     ) -> Result<psa_import_key::Result> {
-        let _guard = self.temp_mutex.lock().expect("temp_mutex poisoned");
         trace!("psa_import_key ingress");
         self.psa_import_key_internal(app_name, op)
     }
@@ -253,7 +248,6 @@ impl Provide for Provider {
         app_name: ApplicationName,
         op: psa_export_public_key::Operation,
     ) -> Result<psa_export_public_key::Result> {
-        let _guard = self.temp_mutex.lock().expect("temp_mutex poisoned");
         trace!("psa_export_public_key ingress");
         self.psa_export_public_key_internal(app_name, op)
     }
@@ -263,7 +257,6 @@ impl Provide for Provider {
         app_name: ApplicationName,
         op: psa_destroy_key::Operation,
     ) -> Result<psa_destroy_key::Result> {
-        let _guard = self.temp_mutex.lock().expect("temp_mutex poisoned");
         trace!("psa_destroy_key ingress");
         self.psa_destroy_key_internal(app_name, op)
     }
@@ -273,7 +266,6 @@ impl Provide for Provider {
         app_name: ApplicationName,
         op: psa_sign_hash::Operation,
     ) -> Result<psa_sign_hash::Result> {
-        let _guard = self.temp_mutex.lock().expect("temp_mutex poisoned");
         trace!("psa_sign_hash ingress");
         self.psa_sign_hash_internal(app_name, op)
     }
@@ -283,7 +275,6 @@ impl Provide for Provider {
         app_name: ApplicationName,
         op: psa_verify_hash::Operation,
     ) -> Result<psa_verify_hash::Result> {
-        let _guard = self.temp_mutex.lock().expect("temp_mutex poisoned");
         if self.software_public_operations {
             trace!("software_psa_verify_hash ingress");
             self.software_psa_verify_hash_internal(app_name, op)
@@ -298,7 +289,6 @@ impl Provide for Provider {
         app_name: ApplicationName,
         op: psa_asymmetric_encrypt::Operation,
     ) -> Result<psa_asymmetric_encrypt::Result> {
-        let _guard = self.temp_mutex.lock().expect("temp_mutex poisoned");
         if self.software_public_operations {
             trace!("software_psa_asymmetric_encrypt ingress");
             self.software_psa_asymmetric_encrypt_internal(app_name, op)
@@ -313,7 +303,6 @@ impl Provide for Provider {
         app_name: ApplicationName,
         op: psa_asymmetric_decrypt::Operation,
     ) -> Result<psa_asymmetric_decrypt::Result> {
-        let _guard = self.temp_mutex.lock().expect("temp_mutex poisoned");
         trace!("psa_asymmetric_decrypt ingress");
         self.psa_asymmetric_decrypt_internal(app_name, op)
     }