Bug 1695974 - rework osclientcert signing on macOS for compatibility r=rmf

Previously, the macOS backend of osclientcerts used
kSecKeyAlgorithmRSASignatureDigestPKCS1v15Raw for RSA PKCS#1v1.5 signing, which
relies on the underlying implementation backing the signing key knowing how to
handle the given data to sign. On Catalina (which uses CryptoTokenKit as
opposed to TokenD), this doesn't appear to work (or, at least, there have been
reports of incompatibilities).
This patch parses out the data to be signed to determine the hash algorithm to
use and the hash data to sign, which is similar to how the Windows backend
works.

Differential Revision: https://phabricator.services.mozilla.com/D111344
This commit is contained in:
Dana Keeler 2021-04-12 18:12:29 +00:00
Родитель 6303ba95ce
Коммит 84e9f36dde
3 изменённых файлов: 142 добавлений и 58 удалений

Просмотреть файл

@ -80,6 +80,10 @@ enum SecStringConstant {
SecKeyAlgorithmECDSASignatureDigestX962SHA384,
SecKeyAlgorithmECDSASignatureDigestX962SHA512,
SecKeyAlgorithmRSASignatureDigestPKCS1v15Raw,
SecKeyAlgorithmRSASignatureDigestPKCS1v15SHA256,
SecKeyAlgorithmRSASignatureDigestPKCS1v15SHA384,
SecKeyAlgorithmRSASignatureDigestPKCS1v15SHA512,
SecKeyAlgorithmRSASignatureDigestPKCS1v15SHA1,
SecAttrKeyTypeECSECPrimeRandom,
// These are available in macOS 10.13
SecKeyAlgorithmRSASignatureDigestPSSSHA1,
@ -183,6 +187,22 @@ impl SecurityFramework {
b"kSecKeyAlgorithmRSASignatureDigestPKCS1v15Raw\0".as_ref(),
SecStringConstant::SecKeyAlgorithmRSASignatureDigestPKCS1v15Raw,
),
(
b"kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA256\0".as_ref(),
SecStringConstant::SecKeyAlgorithmRSASignatureDigestPKCS1v15SHA256,
),
(
b"kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA384\0".as_ref(),
SecStringConstant::SecKeyAlgorithmRSASignatureDigestPKCS1v15SHA384,
),
(
b"kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA512\0".as_ref(),
SecStringConstant::SecKeyAlgorithmRSASignatureDigestPKCS1v15SHA512,
),
(
b"kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA1\0".as_ref(),
SecStringConstant::SecKeyAlgorithmRSASignatureDigestPKCS1v15SHA1,
),
(
b"kSecKeyAlgorithmRSASignatureDigestPSSSHA1\0".as_ref(),
SecStringConstant::SecKeyAlgorithmRSASignatureDigestPSSSHA1,
@ -541,25 +561,25 @@ pub enum KeyType {
RSA,
}
enum SignParams {
EC(CFString),
RSA(CFString),
enum SignParams<'a> {
EC(CFString, &'a [u8]),
RSA(CFString, &'a [u8]),
}
impl SignParams {
impl<'a> SignParams<'a> {
fn new(
key_type: KeyType,
data_len: usize,
data: &'a [u8],
params: &Option<CK_RSA_PKCS_PSS_PARAMS>,
) -> Result<SignParams, ()> {
) -> Result<SignParams<'a>, ()> {
match key_type {
KeyType::EC(_) => SignParams::new_ec_params(data_len),
KeyType::RSA => SignParams::new_rsa_params(params),
KeyType::EC(_) => SignParams::new_ec_params(data),
KeyType::RSA => SignParams::new_rsa_params(params, data),
}
}
fn new_ec_params(data_len: usize) -> Result<SignParams, ()> {
let algorithm_id = match data_len {
fn new_ec_params(data: &'a [u8]) -> Result<SignParams<'a>, ()> {
let algorithm_id = match data.len() {
20 => SecStringConstant::SecKeyAlgorithmECDSASignatureDigestX962SHA1,
32 => SecStringConstant::SecKeyAlgorithmECDSASignatureDigestX962SHA256,
48 => SecStringConstant::SecKeyAlgorithmECDSASignatureDigestX962SHA384,
@ -567,49 +587,77 @@ impl SignParams {
_ => {
error!(
"Unexpected digested signature input length for ECDSA: {}",
data_len
data.len()
);
return Err(());
}
};
let algorithm = SECURITY_FRAMEWORK.get_sec_string_constant(algorithm_id)?;
Ok(SignParams::EC(algorithm))
Ok(SignParams::EC(algorithm, data))
}
fn new_rsa_params(params: &Option<CK_RSA_PKCS_PSS_PARAMS>) -> Result<SignParams, ()> {
let pss_params = match params {
Some(pss_params) => pss_params,
None => {
return Ok(SignParams::RSA(
SECURITY_FRAMEWORK.get_sec_string_constant(
SecStringConstant::SecKeyAlgorithmRSASignatureDigestPKCS1v15Raw,
)?,
));
}
};
let algorithm = {
let algorithm_id = match pss_params.hashAlg {
CKM_SHA_1 => SecStringConstant::SecKeyAlgorithmRSASignatureDigestPSSSHA1,
CKM_SHA256 => SecStringConstant::SecKeyAlgorithmRSASignatureDigestPSSSHA256,
CKM_SHA384 => SecStringConstant::SecKeyAlgorithmRSASignatureDigestPSSSHA384,
CKM_SHA512 => SecStringConstant::SecKeyAlgorithmRSASignatureDigestPSSSHA512,
_ => {
error!(
"unsupported algorithm to use with RSA-PSS: {}",
unsafe_packed_field_access!(pss_params.hashAlg)
);
return Err(());
}
fn new_rsa_params(
params: &Option<CK_RSA_PKCS_PSS_PARAMS>,
data: &'a [u8],
) -> Result<SignParams<'a>, ()> {
if let Some(pss_params) = params {
let algorithm = {
let algorithm_id = match pss_params.hashAlg {
CKM_SHA_1 => SecStringConstant::SecKeyAlgorithmRSASignatureDigestPSSSHA1,
CKM_SHA256 => SecStringConstant::SecKeyAlgorithmRSASignatureDigestPSSSHA256,
CKM_SHA384 => SecStringConstant::SecKeyAlgorithmRSASignatureDigestPSSSHA384,
CKM_SHA512 => SecStringConstant::SecKeyAlgorithmRSASignatureDigestPSSSHA512,
_ => {
error!(
"unsupported algorithm to use with RSA-PSS: {}",
unsafe_packed_field_access!(pss_params.hashAlg)
);
return Err(());
}
};
SECURITY_FRAMEWORK.get_sec_string_constant(algorithm_id)?
};
SECURITY_FRAMEWORK.get_sec_string_constant(algorithm_id)?
return Ok(SignParams::RSA(algorithm, data));
}
// Handle the case where this is a TLS 1.0 MD5/SHA1 hash.
if data.len() == 36 {
let algorithm_id = SecStringConstant::SecKeyAlgorithmRSASignatureDigestPKCS1v15Raw;
let algorithm = SECURITY_FRAMEWORK.get_sec_string_constant(algorithm_id)?;
return Ok(SignParams::RSA(algorithm, data));
}
// Otherwise, `data` should be a DigestInfo.
let (digest_oid, hash) = read_digest_info(data)?;
let algorithm_id = if digest_oid == OID_BYTES_SHA_256 {
SecStringConstant::SecKeyAlgorithmRSASignatureDigestPKCS1v15SHA256
} else if digest_oid == OID_BYTES_SHA_384 {
SecStringConstant::SecKeyAlgorithmRSASignatureDigestPKCS1v15SHA384
} else if digest_oid == OID_BYTES_SHA_512 {
SecStringConstant::SecKeyAlgorithmRSASignatureDigestPKCS1v15SHA512
} else if digest_oid == OID_BYTES_SHA_1 {
SecStringConstant::SecKeyAlgorithmRSASignatureDigestPKCS1v15SHA1
} else {
error!("unsupported digest algorithm: {:?}", digest_oid);
return Err(());
};
Ok(SignParams::RSA(algorithm))
Ok(SignParams::RSA(
SECURITY_FRAMEWORK.get_sec_string_constant(algorithm_id)?,
hash,
))
}
fn get_algorithm(&self) -> SecKeyAlgorithm {
match self {
SignParams::EC(algorithm) => algorithm.as_concrete_TypeRef(),
SignParams::RSA(algorithm) => algorithm.as_concrete_TypeRef(),
SignParams::EC(algorithm, _) => algorithm.as_concrete_TypeRef(),
SignParams::RSA(algorithm, _) => algorithm.as_concrete_TypeRef(),
}
}
fn get_data_to_sign(&self) -> &'a [u8] {
match self {
SignParams::EC(_, data_to_sign) => data_to_sign,
SignParams::RSA(_, data_to_sign) => data_to_sign,
}
}
}
@ -656,9 +704,9 @@ impl Key {
None => return Err(()),
};
match key_size_in_bits {
256 => ec_params = Some(OID_BYTES_SECP256R1.to_vec()),
384 => ec_params = Some(OID_BYTES_SECP384R1.to_vec()),
521 => ec_params = Some(OID_BYTES_SECP521R1.to_vec()),
256 => ec_params = Some(ENCODED_OID_BYTES_SECP256R1.to_vec()),
384 => ec_params = Some(ENCODED_OID_BYTES_SECP384R1.to_vec()),
521 => ec_params = Some(ENCODED_OID_BYTES_SECP521R1.to_vec()),
_ => {
error!("unsupported EC key");
return Err(());
@ -796,11 +844,11 @@ impl Key {
return Err(());
}
};
let sign_params = SignParams::new(self.key_type_enum, data.len(), params)?;
let sign_params = SignParams::new(self.key_type_enum, data, params)?;
let signing_algorithm = sign_params.get_algorithm();
let data = CFData::from_buffer(data);
let data_to_sign = CFData::from_buffer(sign_params.get_data_to_sign());
let signature =
SECURITY_FRAMEWORK.sec_key_create_signature(&key, signing_algorithm, &data)?;
SECURITY_FRAMEWORK.sec_key_create_signature(&key, signing_algorithm, &data_to_sign)?;
let signature_value = match self.key_type_enum {
KeyType::EC(coordinate_width) => {
// We need to convert the DER Ecdsa-Sig-Value to the

Просмотреть файл

@ -361,7 +361,7 @@ fn sign_cryptoapi(
// data will be an encoded DigestInfo, which specifies the hash algorithm and bytes of the hash
// to sign. However, CryptoAPI requires directly specifying the bytes of the hash, so it must
// be extracted first.
let hash_bytes = read_digest(data)?;
let (_, hash_bytes) = read_digest_info(data)?;
let hash = HCryptHash::new(hcryptprov, hash_bytes)?;
let mut signature_len = 0;
if unsafe {

Просмотреть файл

@ -19,13 +19,28 @@ macro_rules! unsafe_packed_field_access {
}};
}
// The following ENCODED_OID_BYTES_* consist of the encoded bytes of an ASN.1
// OBJECT IDENTIFIER specifying the indicated OID (in other words, the full
// tag, length, and value).
#[cfg(target_os = "macos")]
pub const OID_BYTES_SECP256R1: &[u8] =
pub const ENCODED_OID_BYTES_SECP256R1: &[u8] =
&[0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07];
#[cfg(target_os = "macos")]
pub const OID_BYTES_SECP384R1: &[u8] = &[0x06, 0x05, 0x2b, 0x81, 0x04, 0x00, 0x22];
pub const ENCODED_OID_BYTES_SECP384R1: &[u8] = &[0x06, 0x05, 0x2b, 0x81, 0x04, 0x00, 0x22];
#[cfg(target_os = "macos")]
pub const OID_BYTES_SECP521R1: &[u8] = &[0x06, 0x05, 0x2b, 0x81, 0x04, 0x00, 0x23];
pub const ENCODED_OID_BYTES_SECP521R1: &[u8] = &[0x06, 0x05, 0x2b, 0x81, 0x04, 0x00, 0x23];
// The following OID_BYTES_* consist of the contents of the bytes of an ASN.1
// OBJECT IDENTIFIER specifying the indicated OID (in other words, just the
// value, and not the tag or length).
#[cfg(target_os = "macos")]
pub const OID_BYTES_SHA_256: &[u8] = &[0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01];
#[cfg(target_os = "macos")]
pub const OID_BYTES_SHA_384: &[u8] = &[0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02];
#[cfg(target_os = "macos")]
pub const OID_BYTES_SHA_512: &[u8] = &[0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03];
#[cfg(target_os = "macos")]
pub const OID_BYTES_SHA_1: &[u8] = &[0x2b, 0x0e, 0x03, 0x02, 0x1a];
// This is a helper function to take a value and lay it out in memory how
// PKCS#11 is expecting it.
@ -57,8 +72,8 @@ pub fn read_rsa_modulus(public_key: &[u8]) -> Result<Vec<u8>, ()> {
Ok(modulus_value.to_vec())
}
/// Given a slice of DER bytes representing a DigestInfo, extracts the bytes of the digest.
///
/// Given a slice of DER bytes representing a DigestInfo, extracts the bytes of
/// the OID of the hash algorithm and the digest.
/// DigestInfo ::= SEQUENCE {
/// digestAlgorithm DigestAlgorithmIdentifier,
/// digest Digest }
@ -70,16 +85,21 @@ pub fn read_rsa_modulus(public_key: &[u8]) -> Result<Vec<u8>, ()> {
/// parameters ANY DEFINED BY algorithm OPTIONAL }
///
/// Digest ::= OCTET STRING
#[cfg(target_os = "windows")]
pub fn read_digest<'a>(digest_info: &'a [u8]) -> Result<&'a [u8], ()> {
pub fn read_digest_info<'a>(digest_info: &'a [u8]) -> Result<(&'a [u8], &'a [u8]), ()> {
let mut sequence = Sequence::new(digest_info)?;
let _ = sequence.read_sequence()?;
let mut algorithm = sequence.read_sequence()?;
let oid = algorithm.read_oid()?;
algorithm.read_null()?;
if !algorithm.at_end() {
error!("read_digest: extra input");
return Err(());
}
let digest = sequence.read_octet_string()?;
if !sequence.at_end() {
error!("read_digest: extra input");
return Err(());
}
Ok(digest)
Ok((oid, digest))
}
/// Given a slice of DER bytes representing an ECDSA signature, extracts the bytes of `r` and `s`
@ -134,8 +154,11 @@ macro_rules! try_read_bytes {
/// ASN.1 tag identifying an integer.
const INTEGER: u8 = 0x02;
/// ASN.1 tag identifying an octet string.
#[cfg(target_os = "windows")]
const OCTET_STRING: u8 = 0x04;
/// ASN.1 tag identifying a null value.
const NULL: u8 = 0x05;
/// ASN.1 tag identifying an object identifier (OID).
const OBJECT_IDENTIFIER: u8 = 0x06;
/// ASN.1 tag identifying a sequence.
const SEQUENCE: u8 = 0x10;
/// ASN.1 tag modifier identifying an item as constructed.
@ -179,12 +202,25 @@ impl<'a> Sequence<'a> {
}
}
#[cfg(target_os = "windows")]
fn read_octet_string(&mut self) -> Result<&'a [u8], ()> {
let (_, _, bytes) = self.contents.read_tlv(OCTET_STRING)?;
Ok(bytes)
}
fn read_oid(&mut self) -> Result<&'a [u8], ()> {
let (_, _, bytes) = self.contents.read_tlv(OBJECT_IDENTIFIER)?;
Ok(bytes)
}
fn read_null(&mut self) -> Result<(), ()> {
let (_, _, bytes) = self.contents.read_tlv(NULL)?;
if bytes.len() == 0 {
Ok(())
} else {
Err(())
}
}
fn read_sequence(&mut self) -> Result<Sequence<'a>, ()> {
let (_, _, sequence_bytes) = self.contents.read_tlv(SEQUENCE | CONSTRUCTED)?;
Ok(Sequence {