diff --git a/Cargo.lock b/Cargo.lock index 585372ed3b..7891aad7de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1649,6 +1649,7 @@ dependencies = [ "base64", "bytes", "chrono", + "der", "google-cloud-gax", "hex", "hmac 0.13.0", diff --git a/Cargo.toml b/Cargo.toml index 6062af7060..839860a134 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -373,6 +373,7 @@ chrono = { default-features = false, version = "0.4.44" } crates_io_api = { default-features = false, version = "0.12" } clap = { default-features = false, version = "4" } crc32c = { default-features = false, version = "0.6.8" } +der = { default-features = false, version = "0.7", features = ["std"] } futures = { default-features = false, version = "0.3" } h2 = { default-features = false, version = "0.4.13" } hex = { default-features = false, version = "0.4.3" } diff --git a/src/auth/Cargo.toml b/src/auth/Cargo.toml index 2d1c009384..fd7651f315 100644 --- a/src/auth/Cargo.toml +++ b/src/auth/Cargo.toml @@ -51,7 +51,7 @@ thiserror.workspace = true time = { workspace = true, features = ["serde"] } tokio = { workspace = true, features = ["fs", "process"] } url.workspace = true -p256 = { workspace = true, features = ["ecdsa", "pem"], optional = true } +der = { workspace = true, optional = true } jsonwebtoken = { workspace = true, optional = true } # We do not use this directly, but without it the minimal-versions build breaks. # See: https://github.com/Keats/jsonwebtoken/pull/481 @@ -81,7 +81,7 @@ default = ["default-idtoken-backend", "default-rustls-provider"] # See the create top-level documentation for more information. idtoken = ["dep:jsonwebtoken", "jsonwebtoken"] # The `__gdch` feature enables support for GDCH service account credentials. -__gdch = ["dep:p256"] +__gdch = ["dep:der"] # By default this crate enables the `aws_lc_rs` backend. Applications can # link `google-cloud-auth` with `default-features = false, features = ["idtoken"] # and then directly configure the `jsonwebtoken` features to select the diff --git a/src/auth/src/credentials/gdch.rs b/src/auth/src/credentials/gdch.rs index 61bf83d286..8e8bad3c68 100644 --- a/src/auth/src/credentials/gdch.rs +++ b/src/auth/src/credentials/gdch.rs @@ -26,6 +26,8 @@ use crate::token_cache::TokenCache; use crate::{Result, errors}; use async_trait::async_trait; use base64::prelude::{BASE64_URL_SAFE_NO_PAD, Engine as _}; +use der::asn1::UintRef; +use der::{Reader, SliceReader}; use google_cloud_gax::backoff_policy::BackoffPolicyArg; use google_cloud_gax::retry_policy::RetryPolicyArg; use google_cloud_gax::retry_throttler::RetryThrottlerArg; @@ -146,15 +148,63 @@ impl GdchServiceAccountTokenProvider { let sig_der = signer .sign(to_sign.as_bytes()) .map_err(errors::non_retryable)?; - let sig = p256::ecdsa::Signature::from_der(&sig_der).map_err(|e| { - errors::non_retryable_from_str(format!("failed to parse ecdsa DER signature: {}", e)) - })?; - let encoded_sig = BASE64_URL_SAFE_NO_PAD.encode(&sig.to_bytes()[..]); + let sig_bytes = ecdsa_der_to_jose(&sig_der)?; + let encoded_sig = BASE64_URL_SAFE_NO_PAD.encode(&sig_bytes[..]); Ok(format!("{}.{}", to_sign, encoded_sig)) } } +/// Converts an ASN.1 DER-encoded ECDSA signature (`SEQUENCE { r INTEGER, s INTEGER }`) +/// into the fixed-size 64-byte raw concatenation (`r || s`) required by the JWS/JWT specification. +/// +/// This implementation is zero-copy during parsing and avoids heap allocations entirely by writing +/// directly to a stack-allocated 64-byte array. +fn ecdsa_der_to_jose(der: &[u8]) -> Result<[u8; 64]> { + let mut reader = SliceReader::new(der).map_err(errors::non_retryable)?; + let (r_ref, s_ref) = reader + .sequence(|nested| { + let r = nested.decode::>()?; + let s = nested.decode::>()?; + Ok((r, s)) + }) + .and_then(|res| reader.finish(res)) + .map_err(errors::non_retryable)?; + + let mut jose = [0u8; 64]; + copy_jose_integer(&mut jose[..32], r_ref.as_bytes())?; + copy_jose_integer(&mut jose[32..], s_ref.as_bytes())?; + Ok(jose) +} + +/// Copies a variable-length canonical ASN.1 DER integer into a fixed 32-byte big-endian buffer. +/// +/// ASN.1 DER canonical unsigned integers enforce the minimum number of bytes. Critically: +/// - If the integer's most significant bit is set (`>= 0x80`), DER canonicalization prepends a `0x00` +/// byte to prevent negative interpretation in two's complement, resulting in a 33-byte slice. This +/// function securely strips that leading zero byte. +/// - If the integer is shorter than 32 bytes, it is prepended with leading zero padding to fill the +/// destination buffer precisely. +fn copy_jose_integer(dest: &mut [u8], bytes: &[u8]) -> Result<()> { + if bytes.len() > 33 { + return Err(errors::non_retryable_from_str(format!( + "invalid GDCH ECDSA signature integer length: {} bytes", + bytes.len() + ))); + } else if bytes.len() == 33 { + if bytes[0] != 0 { + return Err(errors::non_retryable_from_str( + "invalid 33-byte GDCH ECDSA signature integer: leading byte is not zero", + )); + } + dest.copy_from_slice(&bytes[1..]); + } else { + let offset = 32 - bytes.len(); + dest[offset..].copy_from_slice(bytes); + } + Ok(()) +} + #[async_trait] impl TokenProvider for GdchServiceAccountTokenProvider { async fn token(&self) -> Result { @@ -324,6 +374,7 @@ impl AccessTokenCredentialsProvider for GdchServiceAccountCredentials { mod tests { use super::*; use httptest::{Expectation, Server, matchers::*, responders::*}; + use p256::ecdsa::signature::Verifier; use serde_json::json; type TestResult = anyhow::Result<()>; @@ -443,6 +494,17 @@ mod tests { "system:serviceaccount:test-project:test-name" ); assert_eq!(claims_json["aud"], "http://localhost/token"); + + let public_key = crate::credentials::tests::ES256_PRIVATE_KEY.public_key(); + let verifying_key = p256::ecdsa::VerifyingKey::from(&public_key); + let sig_bytes = BASE64_URL_SAFE_NO_PAD.decode(parts[2])?; + let signature = p256::ecdsa::Signature::from_slice(&sig_bytes) + .map_err(|e| anyhow::anyhow!("invalid signature: {:?}", e))?; + let signed_data = format!("{}.{}", parts[0], parts[1]); + verifying_key + .verify(signed_data.as_bytes(), &signature) + .map_err(|e| anyhow::anyhow!("verification failed: {:?}", e))?; + Ok(()) }