azure_core/azure_identity: introduce SecretBytes for ClientCertificateCredential

sdk/identity/azure_identity/CHANGELOG.md

@@ -4,9 +4,12 @@
 
 ### Features Added
 
+- Added `SecretBytes` for securely passing certificate bytes without printing them in `Debug` or `Display` output.
+
 ### Breaking Changes
 
 - Support for `wasm32-unknown-unknown` has been removed ([#3377](https://github.com/Azure/azure-sdk-for-rust/issues/3377))
+- `ClientCertificateCredential::new()` now takes `SecretBytes` instead of `Secret` for the `certificate` parameter. Pass the raw PKCS12 bytes wrapped in `SecretBytes` instead of a base64-encoded string wrapped in `Secret`.
 
 ### Bugs Fixed
 

sdk/identity/azure_identity/src/client_certificate_credential.rs

@@ -2,8 +2,8 @@
 // Licensed under the MIT License.
 
 use crate::{
-    authentication_error, env::Env, get_authority_host, validate_not_empty, validate_tenant_id,
-    TokenCache,
+    authentication_error, env::Env, get_authority_host, secret_bytes::SecretBytes,
+    validate_not_empty, validate_tenant_id, TokenCache,
 };
 use azure_core::{
     base64,
@@ -64,26 +64,22 @@ impl ClientCertificateCredential {
     /// # Arguments
     /// - `tenant_id`: The tenant (directory) ID of the service principal.
     /// - `client_id`: The client (application) ID of the service principal.
-    /// - `certificate`: A base64-encoded PKCS12 certificate with its RSA private key.
+    /// - `certificate`: The PKCS12 certificate bytes with its RSA private key.
     /// - `options`: Options for configuring the credential. If `None`, the credential uses its default options.
     ///
     pub fn new(
         tenant_id: String,
         client_id: String,
-        certificate: Secret,
+        certificate: SecretBytes,
         options: Option<ClientCertificateCredentialOptions>,
     ) -> azure_core::Result<Arc<ClientCertificateCredential>> {
         validate_tenant_id(&tenant_id)?;
         validate_not_empty(&client_id, "no client ID specified")?;
 
         let options = options.unwrap_or_default();
 
-        let cert_bytes = base64::decode(certificate.secret())
-            .with_context_fn(ErrorKind::Credential, || {
-                "failed to decode base64 certificate data"
-            })?;
-
-        let (key, cert, ca_chain) = parse_certificate(&cert_bytes, options.password.as_ref())?;
+        let (key, cert, ca_chain) =
+            parse_certificate(certificate.bytes(), options.password.as_ref())?;
         let thumbprint = cert
             .digest(MessageDigest::sha1())
             .with_context(ErrorKind::Credential, "failed to compute thumbprint")?
@@ -212,7 +208,7 @@ impl ClientCertificateCredential {
     }
 }
 
-/// Parse a base64-encoded PKCS12 certificate into key, certificate, and optional CA chain.
+/// Parse a PKCS12 certificate into key, certificate, and optional CA chain.
 fn parse_certificate(
     cert_bytes: &[u8],
     password: Option<&Secret>,
@@ -287,7 +283,9 @@ impl TokenCredential for ClientCertificateCredential {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{client_assertion_credential::tests::is_valid_request, tests::*};
+    use crate::{
+        client_assertion_credential::tests::is_valid_request, secret_bytes::SecretBytes, tests::*,
+    };
     use azure_core::{
         http::{
             headers::Headers,
@@ -302,13 +300,12 @@ mod tests {
         sync::{Arc, LazyLock},
     };
 
-    static TEST_CERT: LazyLock<String> = LazyLock::new(|| {
-        let pfx = std::fs::read(concat!(
+    static TEST_CERT: LazyLock<Vec<u8>> = LazyLock::new(|| {
+        std::fs::read(concat!(
             env!("CARGO_MANIFEST_DIR"),
             "/tests/certificate.pfx"
         ))
-        .expect("failed to read test certificate");
-        base64::encode(pfx)
+        .expect("failed to read test certificate")
     });
 
     #[derive(Debug, Clone)]
@@ -319,9 +316,8 @@ mod tests {
     }
 
     impl VerifyAssertionPolicy {
-        fn new(certificate: String, expect_x5c: bool) -> Self {
-            let pfx = base64::decode(certificate).expect("base64 encoding");
-            let (_, cert, _) = parse_certificate(&pfx, None).expect("valid certificate");
+        fn new(certificate: &[u8], expect_x5c: bool) -> Self {
+            let (_, cert, _) = parse_certificate(certificate, None).expect("valid certificate");
             let public_key = cert.public_key().expect("public key");
             let cert_der = cert.to_der().expect("valid certificate");
             Self {
@@ -430,7 +426,7 @@ mod tests {
             let credential = ClientCertificateCredential::new(
                 FAKE_TENANT_ID.to_string(),
                 FAKE_CLIENT_ID.to_string(),
-                Secret::new(TEST_CERT.to_string()),
+                SecretBytes::from(TEST_CERT.as_slice()),
                 Some(ClientCertificateCredentialOptions {
                     client_options: ClientOptions {
                         transport: Some(Transport::new(Arc::new(sts))),
@@ -468,7 +464,7 @@ mod tests {
         let credential = ClientCertificateCredential::new(
             FAKE_TENANT_ID.to_string(),
             FAKE_CLIENT_ID.to_string(),
-            TEST_CERT.as_str().into(),
+            TEST_CERT.as_slice().into(),
             Some(ClientCertificateCredentialOptions {
                 client_options: ClientOptions {
                     transport: Some(Transport::new(Arc::new(sts))),
@@ -518,14 +514,11 @@ mod tests {
         let credential = ClientCertificateCredential::new(
             FAKE_TENANT_ID.to_string(),
             FAKE_CLIENT_ID.to_string(),
-            TEST_CERT.as_str().into(),
+            TEST_CERT.as_slice().into(),
             Some(ClientCertificateCredentialOptions {
                 client_options: ClientOptions {
                     transport: Some(Transport::new(Arc::new(sts))),
-                    per_try_policies: vec![Arc::new(VerifyAssertionPolicy::new(
-                        TEST_CERT.to_string(),
-                        false,
-                    ))],
+                    per_try_policies: vec![Arc::new(VerifyAssertionPolicy::new(&TEST_CERT, false))],
                     ..Default::default()
                 },
                 ..Default::default()
@@ -559,7 +552,7 @@ mod tests {
         ClientCertificateCredential::new(
             FAKE_TENANT_ID.to_string(),
             FAKE_CLIENT_ID.to_string(),
-            "not a certificate".into(),
+            b"not a certificate".as_slice().into(),
             None,
         )
         .expect_err("invalid certificate");
@@ -570,7 +563,7 @@ mod tests {
         ClientCertificateCredential::new(
             "not a valid tenant".to_string(),
             FAKE_CLIENT_ID.to_string(),
-            TEST_CERT.as_str().into(),
+            TEST_CERT.as_slice().into(),
             None,
         )
         .expect_err("invalid tenant ID");
@@ -581,7 +574,7 @@ mod tests {
         ClientCertificateCredential::new(
             FAKE_TENANT_ID.to_string(),
             FAKE_CLIENT_ID.to_string(),
-            TEST_CERT.as_str().into(),
+            TEST_CERT.as_slice().into(),
             None,
         )
         .expect("valid credential")
@@ -602,14 +595,11 @@ mod tests {
         let credential = ClientCertificateCredential::new(
             FAKE_TENANT_ID.to_string(),
             FAKE_CLIENT_ID.to_string(),
-            TEST_CERT.as_str().into(),
+            TEST_CERT.as_slice().into(),
             Some(ClientCertificateCredentialOptions {
                 client_options: ClientOptions {
                     transport: Some(Transport::new(Arc::new(sts))),
-                    per_try_policies: vec![Arc::new(VerifyAssertionPolicy::new(
-                        TEST_CERT.to_string(),
-                        true,
-                    ))],
+                    per_try_policies: vec![Arc::new(VerifyAssertionPolicy::new(&TEST_CERT, true))],
                     ..Default::default()
                 },
                 env: Some(Env::from(

sdk/identity/azure_identity/src/lib.rs

@@ -19,6 +19,7 @@ mod env;
 mod imds_managed_identity_credential;
 mod managed_identity_credential;
 mod process;
+mod secret_bytes;
 mod virtual_machine_managed_identity_credential;
 mod workload_identity_credential;
 
@@ -32,6 +33,7 @@ pub use client_secret_credential::*;
 pub use developer_tools_credential::*;
 pub use managed_identity_credential::*;
 pub use process::{new_executor, Executor};
+pub use secret_bytes::SecretBytes;
 pub use workload_identity_credential::*;
 
 pub(crate) use app_service_managed_identity_credential::*;

sdk/identity/azure_identity/src/secret_bytes.rs

@@ -0,0 +1,126 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+use azure_core::Bytes;
+use std::fmt;
+
+/// Represents secret bytes, e.g., certificate data.
+///
+/// Neither the [`Debug`] nor the [`fmt::Display`] implementation will print the bytes.
+#[derive(Clone, Eq)]
+pub struct SecretBytes(Vec<u8>);
+
+impl SecretBytes {
+    /// Create a new `SecretBytes`.
+    pub fn new(bytes: impl Into<Vec<u8>>) -> Self {
+        Self(bytes.into())
+    }
+
+    /// Get the secret bytes.
+    pub fn bytes(&self) -> &[u8] {
+        &self.0
+    }
+}
+
+// NOTE: this is a constant time compare, however LLVM may (and probably will)
+// optimize this in unexpected ways.
+impl PartialEq for SecretBytes {
+    fn eq(&self, other: &Self) -> bool {
+        let a = self.bytes();
+        let b = other.bytes();
+
+        if a.len() != b.len() {
+            return false;
+        }
+
+        a.iter().zip(b.iter()).fold(0, |acc, (a, b)| acc | (a ^ b)) == 0
+    }
+}
+
+impl fmt::Debug for SecretBytes {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str("SecretBytes")
+    }
+}
+
+impl fmt::Display for SecretBytes {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str("SecretBytes")
+    }
+}
+
+impl From<Bytes> for SecretBytes {
+    fn from(bytes: Bytes) -> Self {
+        Self(bytes.to_vec())
+    }
+}
+
+impl From<&[u8]> for SecretBytes {
+    fn from(bytes: &[u8]) -> Self {
+        Self(bytes.to_vec())
+    }
+}
+
+impl From<Vec<u8>> for SecretBytes {
+    fn from(bytes: Vec<u8>) -> Self {
+        Self(bytes)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn debug_does_not_print_bytes() {
+        let secret = SecretBytes::new(b"super-secret".to_vec());
+        assert_eq!("SecretBytes", format!("{secret:?}"));
+    }
+
+    #[test]
+    fn display_does_not_print_bytes() {
+        let secret = SecretBytes::new(b"super-secret".to_vec());
+        assert_eq!("SecretBytes", format!("{secret}"));
+    }
+
+    #[test]
+    fn eq_same_bytes() {
+        let a = SecretBytes::new(b"hello".to_vec());
+        let b = SecretBytes::new(b"hello".to_vec());
+        assert_eq!(a, b);
+    }
+
+    #[test]
+    fn ne_different_bytes() {
+        let a = SecretBytes::new(b"hello".to_vec());
+        let b = SecretBytes::new(b"world".to_vec());
+        assert_ne!(a, b);
+    }
+
+    #[test]
+    fn ne_different_lengths() {
+        let a = SecretBytes::new(b"hello".to_vec());
+        let b = SecretBytes::new(b"hello!".to_vec());
+        assert_ne!(a, b);
+    }
+
+    #[test]
+    fn from_bytes_type() {
+        let bytes = Bytes::from_static(b"data");
+        let secret = SecretBytes::from(bytes);
+        assert_eq!(b"data", secret.bytes());
+    }
+
+    #[test]
+    fn from_slice() {
+        let data: &[u8] = b"data";
+        let secret = SecretBytes::from(data);
+        assert_eq!(b"data", secret.bytes());
+    }
+
+    #[test]
+    fn from_vec() {
+        let secret = SecretBytes::from(b"data".to_vec());
+        assert_eq!(b"data", secret.bytes());
+    }
+}