diff --git a/README.md b/README.md index c09210371..3ad859371 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,6 @@ The Rust library crate provides: * `fetch_remote_manifests` enables the verification step to retrieve externally referenced manifest stores. External manifests are only fetched if there is no embedded manifest store and no locally adjacent .c2pa manifest store file of the same name. * `json_schema` is used by `make schema` to produce a JSON schema document that represents the `ManifestStore` data structures. * `psxxx_ocsp_stapling_experimental` this is an demonstration feature that will attempt to fetch the OCSP data from the OCSP responders listed in the manifest signing certificate. The response becomes part of the manifest and is used to prove the certificate was not revoked at the time of signing. This is only implemented for PS256, PS384 and PS512 signatures and is intended as a demonstration. -* `openssl_ffi_mutex` prevents multiple threads from accessing the C OpenSSL library simultaneously. (This library is not re-entrant.) In a multi-threaded process (such as Cargo's test runner), this can lead to unpredictable behavior. ## Example code diff --git a/internal/crypto/Cargo.toml b/internal/crypto/Cargo.toml index 698b5b718..92cb17960 100644 --- a/internal/crypto/Cargo.toml +++ b/internal/crypto/Cargo.toml @@ -27,6 +27,7 @@ rustdoc-args = ["--cfg", "docsrs"] [features] json_schema = ["dep:schemars"] +openssl = ["dep:openssl"] [dependencies] base64 = "0.22.1" @@ -43,10 +44,14 @@ thiserror = "1.0.61" x509-certificate = "0.21.0" [target.'cfg(not(target_arch = "wasm32"))'.dependencies] +openssl = { version = "0.10.61", features = ["vendored"], optional = true } ureq = "2.4.0" url = "=2.5.2" # Can't advance to 2.5.3 until Unicode-3.0 license is reviewed. x509-parser = "0.16.0" +[package.metadata.cargo-udeps.ignore] +normal = ["openssl"] # TEMPORARY: Remove after openssl transition complete. + [dependencies.chrono] version = "0.4.38" default-features = false diff --git a/internal/crypto/src/lib.rs b/internal/crypto/src/lib.rs index 2eb04043b..8c461da83 100644 --- a/internal/crypto/src/lib.rs +++ b/internal/crypto/src/lib.rs @@ -25,6 +25,13 @@ pub mod hash; pub(crate) mod internal; pub mod ocsp; + +#[cfg(all(feature = "openssl", not(target_arch = "wasm32")))] +pub mod openssl; + +#[cfg(all(feature = "openssl", target_arch = "wasm32"))] +compile_error!("OpenSSL feature is not compatible with WASM platform"); + pub mod validation_codes; mod signing_alg; diff --git a/internal/crypto/src/openssl/ffi_mutex.rs b/internal/crypto/src/openssl/ffi_mutex.rs new file mode 100644 index 000000000..67f1263e4 --- /dev/null +++ b/internal/crypto/src/openssl/ffi_mutex.rs @@ -0,0 +1,79 @@ +// Copyright 2024 Adobe. All rights reserved. +// This file is licensed to you under the Apache License, +// Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0) +// or the MIT license (http://opensource.org/licenses/MIT), +// at your option. + +// Unless required by applicable law or agreed to in writing, +// this software is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or +// implied. See the LICENSE-MIT and LICENSE-APACHE files for the +// specific language governing permissions and limitations under +// each license. + +use std::{ + error::Error, + fmt, + sync::{Mutex, MutexGuard}, +}; + +static FFI_MUTEX: Mutex<()> = Mutex::new(()); + +/// This mutex must be used by all code that accesses OpenSSL native code since +/// the OpenSSL native code library is not re-entrant. +/// +/// Failure to do so has been observed to lead to unexpected behavior including +/// process crashes. +pub struct OpenSslMutex<'a> { + // The dead code bypass is intentional. We don't need to read the () contents of this guard. We + // only need to ensure that the guard is dropped when this struct is dropped. + #[allow(dead_code)] + guard: MutexGuard<'a, ()>, +} + +impl OpenSslMutex<'_> { + /// Acquire a mutex on OpenSSL FFI code. + /// + /// WARNING: Calling code MUST NOT PANIC inside this function or + /// anything called by it, even in test code. This will poison the FFI mutex + /// and leave OpenSSL unusable for the remainder of the process lifetime. + pub fn acquire() -> Result { + // Useful for debugging. + // eprintln!( + // "ACQUIRING FFI MUTEX at\n{}", + // std::backtrace::Backtrace::force_capture() + // ); + + match FFI_MUTEX.lock() { + Ok(guard) => Ok(Self { guard }), + Err(_) => Err(OpenSslMutexUnavailable {}), + } + } +} + +// Useful for debugging. +// impl<'a> Drop for OpenSslMutex<'a> { +// fn drop(&mut self) { +// eprintln!("Releasing FFI mutex\n\n\n"); +// } +// } + +/// Error returned when unable to acquire the OpenSSL native code mutex. +/// +/// If this occurs, it's likely that a prior invocation of OpenSSL code panicked +/// while holding the mutex. When this happens, the OpenSSL native code mutex is +/// considered poisoned for the remainder of the process lifetime. +/// +/// See [Rustnomicon: Poisoning] for more information. +/// +/// [Rustnomicon: Poisoning]: https://doc.rust-lang.org/nomicon/poisoning.html +#[derive(Debug, Eq, PartialEq)] +pub struct OpenSslMutexUnavailable; + +impl fmt::Display for OpenSslMutexUnavailable { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "Unable to acquire OpenSSL native code mutex") + } +} + +impl Error for OpenSslMutexUnavailable {} diff --git a/internal/crypto/src/openssl/mod.rs b/internal/crypto/src/openssl/mod.rs new file mode 100644 index 000000000..f45e7d70e --- /dev/null +++ b/internal/crypto/src/openssl/mod.rs @@ -0,0 +1,22 @@ +// Copyright 2022 Adobe. All rights reserved. +// This file is licensed to you under the Apache License, +// Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0) +// or the MIT license (http://opensource.org/licenses/MIT), +// at your option. + +// Unless required by applicable law or agreed to in writing, +// this software is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or +// implied. See the LICENSE-MIT and LICENSE-APACHE files for the +// specific language governing permissions and limitations under +// each license. + +//! This module provides functions for working with the [`openssl` native code +//! library]. +//! +//! It is only available if the `openssl` feature is enabled. +//! +//! [`openssl` native code library]: https://crates.io/crates/openssl + +mod ffi_mutex; +pub use ffi_mutex::{OpenSslMutex, OpenSslMutexUnavailable}; diff --git a/internal/crypto/src/tests/mod.rs b/internal/crypto/src/tests/mod.rs index dc797c08b..2f763b478 100644 --- a/internal/crypto/src/tests/mod.rs +++ b/internal/crypto/src/tests/mod.rs @@ -25,4 +25,8 @@ mod base64; mod hash; mod internal; mod ocsp; + +#[cfg(all(feature = "openssl", not(target_arch = "wasm32")))] +mod openssl; + mod signing_alg; diff --git a/internal/crypto/src/tests/openssl/ffi_mutex.rs b/internal/crypto/src/tests/openssl/ffi_mutex.rs new file mode 100644 index 000000000..6b118e814 --- /dev/null +++ b/internal/crypto/src/tests/openssl/ffi_mutex.rs @@ -0,0 +1,29 @@ +// Copyright 2024 Adobe. All rights reserved. +// This file is licensed to you under the Apache License, +// Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0) +// or the MIT license (http://opensource.org/licenses/MIT), +// at your option. + +// Unless required by applicable law or agreed to in writing, +// this software is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or +// implied. See the LICENSE-MIT and LICENSE-APACHE files for the +// specific language governing permissions and limitations under +// each license. + +use crate::openssl::OpenSslMutexUnavailable; + +#[test] +fn impl_display() { + let err = OpenSslMutexUnavailable {}; + assert_eq!( + err.to_string(), + "Unable to acquire OpenSSL native code mutex" + ); +} + +#[test] +fn impl_debug() { + let err = OpenSslMutexUnavailable {}; + assert_eq!(format!("{err:?}"), "OpenSslMutexUnavailable"); +} diff --git a/internal/crypto/src/tests/openssl/mod.rs b/internal/crypto/src/tests/openssl/mod.rs new file mode 100644 index 000000000..b4d2829f9 --- /dev/null +++ b/internal/crypto/src/tests/openssl/mod.rs @@ -0,0 +1,14 @@ +// Copyright 2024 Adobe. All rights reserved. +// This file is licensed to you under the Apache License, +// Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0) +// or the MIT license (http://opensource.org/licenses/MIT), +// at your option. + +// Unless required by applicable law or agreed to in writing, +// this software is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or +// implied. See the LICENSE-MIT and LICENSE-APACHE files for the +// specific language governing permissions and limitations under +// each license. + +mod ffi_mutex; diff --git a/sdk/Cargo.toml b/sdk/Cargo.toml index 32db6c53f..a4e92dfa6 100644 --- a/sdk/Cargo.toml +++ b/sdk/Cargo.toml @@ -33,12 +33,12 @@ file_io = ["openssl_sign"] serialize_thumbnails = [] no_interleaved_io = ["file_io"] fetch_remote_manifests = [] -openssl_sign = ["openssl"] +openssl = ["dep:openssl", "c2pa-crypto/openssl"] +openssl_sign = ["openssl", "c2pa-crypto/openssl"] json_schema = ["dep:schemars"] pdf = ["dep:lopdf"] v1_api = [] unstable_api = [] -openssl_ffi_mutex = [] # The diagnostics feature is unsupported and might be removed. # It enables some low-overhead timing features used in our development cycle. @@ -60,7 +60,6 @@ required-features = ["unstable_api"] name = "v2api" required-features = ["unstable_api"] - [lib] crate-type = ["lib"] @@ -171,7 +170,6 @@ c2pa = { path = ".", features = [ glob = "0.3.1" jumbf = "0.4.0" - [target.'cfg(target_arch = "wasm32")'.dev-dependencies] wasm-bindgen-test = "0.3.31" diff --git a/sdk/src/error.rs b/sdk/src/error.rs index a41909e62..e14e1879b 100644 --- a/sdk/src/error.rs +++ b/sdk/src/error.rs @@ -281,11 +281,12 @@ pub enum Error { #[error(transparent)] CborError(#[from] serde_cbor::Error), + #[cfg(feature = "openssl")] #[error("could not acquire OpenSSL FFI mutex")] OpenSslMutexError, - #[error(transparent)] #[cfg(feature = "openssl")] + #[error(transparent)] OpenSslError(#[from] openssl::error::ErrorStack), #[error(transparent)] @@ -303,3 +304,10 @@ pub enum Error { /// A specialized `Result` type for C2PA toolkit operations. pub type Result = std::result::Result; + +#[cfg(feature = "openssl")] +impl From for Error { + fn from(_err: c2pa_crypto::openssl::OpenSslMutexUnavailable) -> Self { + Self::OpenSslMutexError + } +} diff --git a/sdk/src/openssl/ec_signer.rs b/sdk/src/openssl/ec_signer.rs index d8ab1a510..136cf328e 100644 --- a/sdk/src/openssl/ec_signer.rs +++ b/sdk/src/openssl/ec_signer.rs @@ -11,7 +11,7 @@ // specific language governing permissions and limitations under // each license. -use c2pa_crypto::SigningAlg; +use c2pa_crypto::{openssl::OpenSslMutex, SigningAlg}; use openssl::{ ec::EcKey, hash::MessageDigest, @@ -47,7 +47,7 @@ impl ConfigurableSigner for EcSigner { alg: SigningAlg, tsa_url: Option, ) -> Result { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; let certs_size = signcert.len(); let pkey = EcKey::private_key_from_pem(pkey).map_err(Error::OpenSslError)?; @@ -73,7 +73,7 @@ impl ConfigurableSigner for EcSigner { impl Signer for EcSigner { fn sign(&self, data: &[u8]) -> Result> { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; let key = PKey::from_ec_key(self.pkey.clone()).map_err(Error::OpenSslError)?; @@ -95,7 +95,7 @@ impl Signer for EcSigner { } fn certs(&self) -> Result>> { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; let mut certs: Vec> = Vec::new(); diff --git a/sdk/src/openssl/ec_validator.rs b/sdk/src/openssl/ec_validator.rs index 1facac9dd..3bd1516ae 100644 --- a/sdk/src/openssl/ec_validator.rs +++ b/sdk/src/openssl/ec_validator.rs @@ -11,7 +11,7 @@ // specific language governing permissions and limitations under // each license. -use c2pa_crypto::SigningAlg; +use c2pa_crypto::{openssl::OpenSslMutex, SigningAlg}; use openssl::{ec::EcKey, hash::MessageDigest, pkey::PKey}; use crate::{validator::CoseValidator, Error, Result}; @@ -28,7 +28,7 @@ impl EcValidator { impl CoseValidator for EcValidator { fn validate(&self, sig: &[u8], data: &[u8], pkey: &[u8]) -> Result { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; let public_key = EcKey::public_key_from_der(pkey).map_err(|_err| Error::CoseSignature)?; let key = PKey::from_ec_key(public_key).map_err(wrap_openssl_err)?; diff --git a/sdk/src/openssl/ed_signer.rs b/sdk/src/openssl/ed_signer.rs index a62b6e95e..21202eebf 100644 --- a/sdk/src/openssl/ed_signer.rs +++ b/sdk/src/openssl/ed_signer.rs @@ -11,7 +11,7 @@ // specific language governing permissions and limitations under // each license. -use c2pa_crypto::SigningAlg; +use c2pa_crypto::{openssl::OpenSslMutex, SigningAlg}; use openssl::{ pkey::{PKey, Private}, x509::X509, @@ -40,7 +40,7 @@ impl ConfigurableSigner for EdSigner { alg: SigningAlg, tsa_url: Option, ) -> Result { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; let certs_size = signcert.len(); let signcerts = X509::stack_from_pem(signcert).map_err(Error::OpenSslError)?; @@ -70,7 +70,7 @@ impl ConfigurableSigner for EdSigner { impl Signer for EdSigner { fn sign(&self, data: &[u8]) -> Result> { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; let mut signer = openssl::sign::Signer::new_without_digest(&self.pkey).map_err(Error::OpenSslError)?; @@ -85,7 +85,7 @@ impl Signer for EdSigner { } fn certs(&self) -> Result>> { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; let mut certs: Vec> = Vec::new(); diff --git a/sdk/src/openssl/ed_validator.rs b/sdk/src/openssl/ed_validator.rs index 1af95ca36..7942d8a4a 100644 --- a/sdk/src/openssl/ed_validator.rs +++ b/sdk/src/openssl/ed_validator.rs @@ -11,7 +11,7 @@ // specific language governing permissions and limitations under // each license. -use c2pa_crypto::SigningAlg; +use c2pa_crypto::{openssl::OpenSslMutex, SigningAlg}; use openssl::pkey::PKey; use crate::{validator::CoseValidator, Error, Result}; @@ -28,7 +28,7 @@ impl EdValidator { impl CoseValidator for EdValidator { fn validate(&self, sig: &[u8], data: &[u8], pkey: &[u8]) -> Result { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; let public_key = PKey::public_key_from_der(pkey).map_err(|_err| Error::CoseSignature)?; diff --git a/sdk/src/openssl/ffi_mutex.rs b/sdk/src/openssl/ffi_mutex.rs deleted file mode 100644 index dcaf7980b..000000000 --- a/sdk/src/openssl/ffi_mutex.rs +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright 2024 Adobe. All rights reserved. -// This file is licensed to you under the Apache License, -// Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0) -// or the MIT license (http://opensource.org/licenses/MIT), -// at your option. - -// Unless required by applicable law or agreed to in writing, -// this software is distributed on an "AS IS" BASIS, WITHOUT -// WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or -// implied. See the LICENSE-MIT and LICENSE-APACHE files for the -// specific language governing permissions and limitations under -// each license. - -// OpenSSL code is not re-entrant. Use this to guard against race conditions. -use crate::Result; - -#[cfg(feature = "openssl_ffi_mutex")] -static FFI_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(()); - -pub(crate) struct OpenSslMutex<'a> { - // Dead code here is intentional. We don't need to read the () contents - // of this guard. We only need to ensure that the guard is dropped when - // this struct is dropped. - #[allow(dead_code)] - #[cfg(feature = "openssl_ffi_mutex")] - guard: std::sync::MutexGuard<'a, ()>, - - #[allow(dead_code)] - #[cfg(not(feature = "openssl_ffi_mutex"))] - guard: &'a str, -} - -impl OpenSslMutex<'_> { - /// Acquire a mutex on OpenSSL FFI code. - /// - /// WARNING: Calling code MUST NOT PANIC inside this function or - /// anything called by it, even in test code. This will poison the FFI mutex - /// and leave OpenSSL unusable for the remainder of the process lifetime. - pub(crate) fn acquire() -> Result { - // Useful for debugging. - // eprintln!( - // "ACQUIRING FFI MUTEX at\n{}", - // std::backtrace::Backtrace::force_capture() - // ); - - #[cfg(feature = "openssl_ffi_mutex")] - match FFI_MUTEX.lock() { - Ok(guard) => Ok(Self { guard }), - Err(_) => Err(crate::Error::OpenSslMutexError), - } - - #[cfg(not(feature = "openssl_ffi_mutex"))] - Ok(Self { guard: "foo" }) - } -} - -// Useful for debugging. -// impl<'a> Drop for OpenSslMutex<'a> { -// fn drop(&mut self) { -// eprintln!("Releasing FFI mutex\n\n\n"); -// } -// } diff --git a/sdk/src/openssl/mod.rs b/sdk/src/openssl/mod.rs index 0023ef22e..08f262a83 100644 --- a/sdk/src/openssl/mod.rs +++ b/sdk/src/openssl/mod.rs @@ -53,9 +53,6 @@ pub(crate) use openssl_trust_handler::verify_trust; #[cfg(feature = "openssl")] pub(crate) use openssl_trust_handler::OpenSSLTrustHandlerConfig; -mod ffi_mutex; -pub(crate) use ffi_mutex::OpenSslMutex; - #[cfg(test)] pub(crate) mod temp_signer_async; diff --git a/sdk/src/openssl/openssl_trust_handler.rs b/sdk/src/openssl/openssl_trust_handler.rs index 6219955d6..d41c5ab14 100644 --- a/sdk/src/openssl/openssl_trust_handler.rs +++ b/sdk/src/openssl/openssl_trust_handler.rs @@ -18,7 +18,7 @@ use std::{ }; use asn1_rs::Oid; -use c2pa_crypto::base64; +use c2pa_crypto::{base64, openssl::OpenSslMutex}; use openssl::x509::verify::X509VerifyFlags; use crate::{ @@ -43,7 +43,7 @@ fn certs_der_to_x509(ders: &[Vec]) -> Result> { } fn load_trust_from_pem_data(trust_data: &[u8]) -> Result> { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; openssl::x509::X509::stack_from_pem(trust_data).map_err(Error::OpenSslError) } @@ -75,7 +75,7 @@ impl OpenSSLTrustHandlerConfig { } fn update_store(&mut self) -> Result<()> { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; let mut builder = openssl::x509::store::X509StoreBuilder::new().map_err(Error::OpenSslError)?; @@ -143,7 +143,7 @@ impl TrustHandlerConfig for OpenSSLTrustHandlerConfig { allowed_list.read_to_end(&mut buffer)?; { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; if let Ok(cert_list) = openssl::x509::X509::stack_from_pem(&buffer) { for cert in &cert_list { let cert_der = cert.to_der().map_err(Error::OpenSslError)?; @@ -249,7 +249,7 @@ pub(crate) fn verify_trust( return Ok(true); } - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; let mut cert_chain = openssl::stack::Stack::new().map_err(Error::OpenSslError)?; let mut store_ctx = openssl::x509::X509StoreContext::new().map_err(Error::OpenSslError)?; diff --git a/sdk/src/openssl/rsa_signer.rs b/sdk/src/openssl/rsa_signer.rs index b3078489f..2ffe14933 100644 --- a/sdk/src/openssl/rsa_signer.rs +++ b/sdk/src/openssl/rsa_signer.rs @@ -13,7 +13,7 @@ use std::cell::Cell; -use c2pa_crypto::{ocsp::OcspResponse, SigningAlg}; +use c2pa_crypto::{ocsp::OcspResponse, openssl::OpenSslMutex, SigningAlg}; use openssl::{ hash::MessageDigest, pkey::{PKey, Private}, @@ -97,7 +97,7 @@ impl ConfigurableSigner for RsaSigner { alg: SigningAlg, tsa_url: Option, ) -> Result { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; let signcerts = X509::stack_from_pem(signcert).map_err(wrap_openssl_err)?; let rsa = Rsa::private_key_from_pem(pkey).map_err(wrap_openssl_err)?; @@ -211,7 +211,7 @@ impl Signer for RsaSigner { } fn certs(&self) -> Result>> { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; self.certs_internal() } @@ -224,7 +224,7 @@ impl Signer for RsaSigner { } fn ocsp_val(&self) -> Option> { - let _openssl = super::OpenSslMutex::acquire().ok()?; + let _openssl = OpenSslMutex::acquire().ok()?; // update OCSP if needed self.update_ocsp(); diff --git a/sdk/src/openssl/rsa_validator.rs b/sdk/src/openssl/rsa_validator.rs index 0bbb3ba1d..e4ee6e371 100644 --- a/sdk/src/openssl/rsa_validator.rs +++ b/sdk/src/openssl/rsa_validator.rs @@ -11,7 +11,7 @@ // specific language governing permissions and limitations under // each license. -use c2pa_crypto::SigningAlg; +use c2pa_crypto::{openssl::OpenSslMutex, SigningAlg}; use openssl::{hash::MessageDigest, pkey::PKey, rsa::Rsa}; use crate::{validator::CoseValidator, Error, Result}; @@ -28,7 +28,7 @@ impl RsaValidator { impl CoseValidator for RsaValidator { fn validate(&self, sig: &[u8], data: &[u8], pkey: &[u8]) -> Result { - let _openssl = super::OpenSslMutex::acquire()?; + let _openssl = OpenSslMutex::acquire()?; let rsa = Rsa::public_key_from_der(pkey)?;