Improve APK signature handling
There are two changes here:
- The maxSdk property of a signature is inclusive not exclusive
(https://source.android.com/docs/security/features/apksigning/v3),
so our mapping to a Rust Range was wrong.
- We hard-wired the platform SDK as 31; but really we should use the
version of the platform we're running on.
I thought the APK Verify library shouldn't really be reading
properties, so instead I changed the function signatures to accept the
current SDK version as a parameter. That does mean duplicating the
code to read it in virtualization manager & microdroid manager, but I
can live with that for now.
Bug: 271500509
Bug: 190343842
Test: atest MicrodroidTests
Test: run a VM from an APK signed with minSdk = 33
Change-Id: I7ec7a201d05a6545847eb9b264abe642de883e0b
diff --git a/libs/apkverify/src/v3.rs b/libs/apkverify/src/v3.rs
index e1b728d..6082422 100644
--- a/libs/apkverify/src/v3.rs
+++ b/libs/apkverify/src/v3.rs
@@ -24,7 +24,7 @@
use openssl::x509::X509;
use std::fs::File;
use std::io::{Read, Seek};
-use std::ops::Range;
+use std::ops::RangeInclusive;
use std::path::Path;
use crate::algorithms::SignatureAlgorithmID;
@@ -33,11 +33,9 @@
pub const APK_SIGNATURE_SCHEME_V3_BLOCK_ID: u32 = 0xf05368c0;
-// TODO(b/190343842): get "ro.build.version.sdk"
-const SDK_INT: u32 = 31;
-
type Signers = LengthPrefixed<Vec<LengthPrefixed<Signer>>>;
+#[derive(Debug)]
pub(crate) struct Signer {
signed_data: LengthPrefixed<Bytes>, // not verified yet
min_sdk: u32,
@@ -47,8 +45,8 @@
}
impl Signer {
- fn sdk_range(&self) -> Range<u32> {
- self.min_sdk..self.max_sdk
+ fn sdk_range(&self) -> RangeInclusive<u32> {
+ self.min_sdk..=self.max_sdk
}
}
@@ -62,8 +60,8 @@
}
impl SignedData {
- fn sdk_range(&self) -> Range<u32> {
- self.min_sdk..self.max_sdk
+ fn sdk_range(&self) -> RangeInclusive<u32> {
+ self.min_sdk..=self.max_sdk
}
fn find_digest_by_algorithm(&self, algorithm_id: SignatureAlgorithmID) -> Result<&Digest> {
@@ -92,32 +90,30 @@
/// Verifies APK Signature Scheme v3 signatures of the provided APK and returns the public key
/// associated with the signer in DER format.
-pub fn verify<P: AsRef<Path>>(apk_path: P) -> Result<Box<[u8]>> {
+pub fn verify<P: AsRef<Path>>(apk_path: P, current_sdk: u32) -> Result<Box<[u8]>> {
let apk = File::open(apk_path.as_ref())?;
- let (signer, mut sections) = extract_signer_and_apk_sections(apk)?;
+ let (signer, mut sections) = extract_signer_and_apk_sections(apk, current_sdk)?;
signer.verify(&mut sections)
}
/// Gets the public key (in DER format) that was used to sign the given APK/APEX file
-pub fn get_public_key_der<P: AsRef<Path>>(apk_path: P) -> Result<Box<[u8]>> {
+pub fn get_public_key_der<P: AsRef<Path>>(apk_path: P, current_sdk: u32) -> Result<Box<[u8]>> {
let apk = File::open(apk_path.as_ref())?;
- let (signer, _) = extract_signer_and_apk_sections(apk)?;
+ let (signer, _) = extract_signer_and_apk_sections(apk, current_sdk)?;
Ok(signer.public_key.public_key_to_der()?.into_boxed_slice())
}
pub(crate) fn extract_signer_and_apk_sections<R: Read + Seek>(
apk: R,
+ current_sdk: u32,
) -> Result<(Signer, ApkSections<R>)> {
let mut sections = ApkSections::new(apk)?;
let mut block = sections.find_signature(APK_SIGNATURE_SCHEME_V3_BLOCK_ID).context(
"Fallback to v2 when v3 block not found is not yet implemented.", // b/197052981
)?;
- let mut supported = block
- .read::<Signers>()?
- .into_inner()
- .into_iter()
- .filter(|s| s.sdk_range().contains(&SDK_INT))
- .collect::<Vec<_>>();
+ let signers = block.read::<Signers>()?.into_inner();
+ let mut supported =
+ signers.into_iter().filter(|s| s.sdk_range().contains(¤t_sdk)).collect::<Vec<_>>();
ensure!(
supported.len() == 1,
"APK Signature Scheme V3 only supports one signer: {} signers found.",
diff --git a/libs/apkverify/src/v4.rs b/libs/apkverify/src/v4.rs
index e77ad77..045f4af 100644
--- a/libs/apkverify/src/v4.rs
+++ b/libs/apkverify/src/v4.rs
@@ -37,9 +37,10 @@
/// [apk_digest]: https://source.android.com/docs/security/apksigning/v4#apk-digest
pub fn get_apk_digest<R: Read + Seek>(
apk: R,
+ current_sdk: u32,
verify: bool,
) -> Result<(SignatureAlgorithmID, Box<[u8]>)> {
- let (signer, mut sections) = extract_signer_and_apk_sections(apk)?;
+ let (signer, mut sections) = extract_signer_and_apk_sections(apk, current_sdk)?;
let strongest_algorithm_id = signer
.strongest_signature()?
.signature_algorithm_id
@@ -148,6 +149,7 @@
/// function OOMing.
pub fn create(
mut apk: &mut R,
+ current_sdk: u32,
block_size: usize,
salt: &[u8],
algorithm: HashAlgorithm,
@@ -175,7 +177,8 @@
ret.hashing_info.log2_blocksize = log2(block_size);
apk.seek(SeekFrom::Start(start))?;
- let (signature_algorithm_id, apk_digest) = get_apk_digest(apk, /*verify=*/ false)?;
+ let (signature_algorithm_id, apk_digest) =
+ get_apk_digest(apk, current_sdk, /*verify=*/ false)?;
ret.signing_info.signature_algorithm_id = signature_algorithm_id;
ret.signing_info.apk_digest = apk_digest;
// TODO(jiyong): add a signature to the signing_info struct
@@ -362,8 +365,9 @@
#[test]
fn digest_from_apk() {
let mut input = Cursor::new(include_bytes!("../tests/data/v4-digest-v3-Sha256withEC.apk"));
+ let current_sdk = 31;
let mut created =
- V4Signature::create(&mut input, 4096, &[], HashAlgorithm::SHA256).unwrap();
+ V4Signature::create(&mut input, current_sdk, 4096, &[], HashAlgorithm::SHA256).unwrap();
let mut golden = V4Signature::from_idsig_path(format!("{}.idsig", TEST_APK_PATH)).unwrap();
diff --git a/libs/apkverify/tests/apkverify_test.rs b/libs/apkverify/tests/apkverify_test.rs
index baf7c42..f08b357 100644
--- a/libs/apkverify/tests/apkverify_test.rs
+++ b/libs/apkverify/tests/apkverify_test.rs
@@ -23,10 +23,12 @@
const KEY_NAMES_ECDSA: &[&str] = &["p256", "p384", "p521"];
const KEY_NAMES_RSA: &[&str] = &["1024", "2048", "3072", "4096", "8192", "16384"];
+const SDK_INT: u32 = 31;
+
#[test]
fn test_verify_truncated_cd() {
use zip::result::ZipError;
- let res = verify("tests/data/v2-only-truncated-cd.apk");
+ let res = verify("tests/data/v2-only-truncated-cd.apk", SDK_INT);
// TODO(b/190343842): consider making a helper for err assertion
assert!(matches!(
res.unwrap_err().root_cause().downcast_ref::<ZipError>().unwrap(),
@@ -42,7 +44,7 @@
#[test]
fn apks_signed_with_v3_dsa_sha256_are_not_supported() {
for key_name in KEY_NAMES_DSA.iter() {
- let res = verify(format!("tests/data/v3-only-with-dsa-sha256-{}.apk", key_name));
+ let res = verify(format!("tests/data/v3-only-with-dsa-sha256-{}.apk", key_name), SDK_INT);
assert!(res.is_err(), "DSA algorithm is not supported for verification. See b/197052981.");
assert_contains(&res.unwrap_err().to_string(), "No supported APK signatures found");
}
@@ -95,7 +97,7 @@
"tests/data/v3-only-with-rsa-pkcs1-sha256-3072-sig-does-not-verify.apk",
];
for path in path_list.iter() {
- let res = verify(path);
+ let res = verify(path, SDK_INT);
assert!(res.is_err());
assert_contains(&res.unwrap_err().to_string(), "Signature is invalid");
}
@@ -103,22 +105,25 @@
#[test]
fn test_verify_v3_digest_mismatch() {
- let res = verify("tests/data/v3-only-with-rsa-pkcs1-sha512-8192-digest-mismatch.apk");
+ let res = verify("tests/data/v3-only-with-rsa-pkcs1-sha512-8192-digest-mismatch.apk", SDK_INT);
assert!(res.is_err());
assert_contains(&res.unwrap_err().to_string(), "Digest mismatch");
}
#[test]
fn test_verify_v3_wrong_apk_sig_block_magic() {
- let res = verify("tests/data/v3-only-with-ecdsa-sha512-p384-wrong-apk-sig-block-magic.apk");
+ let res =
+ verify("tests/data/v3-only-with-ecdsa-sha512-p384-wrong-apk-sig-block-magic.apk", SDK_INT);
assert!(res.is_err());
assert_contains(&res.unwrap_err().to_string(), "No APK Signing Block");
}
#[test]
fn test_verify_v3_apk_sig_block_size_mismatch() {
- let res =
- verify("tests/data/v3-only-with-rsa-pkcs1-sha512-4096-apk-sig-block-size-mismatch.apk");
+ let res = verify(
+ "tests/data/v3-only-with-rsa-pkcs1-sha512-4096-apk-sig-block-size-mismatch.apk",
+ SDK_INT,
+ );
assert!(res.is_err());
assert_contains(
&res.unwrap_err().to_string(),
@@ -128,35 +133,35 @@
#[test]
fn test_verify_v3_cert_and_public_key_mismatch() {
- let res = verify("tests/data/v3-only-cert-and-public-key-mismatch.apk");
+ let res = verify("tests/data/v3-only-cert-and-public-key-mismatch.apk", SDK_INT);
assert!(res.is_err());
assert_contains(&res.unwrap_err().to_string(), "Public key mismatch");
}
#[test]
fn test_verify_v3_empty() {
- let res = verify("tests/data/v3-only-empty.apk");
+ let res = verify("tests/data/v3-only-empty.apk", SDK_INT);
assert!(res.is_err());
assert_contains(&res.unwrap_err().to_string(), "APK too small for APK Signing Block");
}
#[test]
fn test_verify_v3_no_certs_in_sig() {
- let res = verify("tests/data/v3-only-no-certs-in-sig.apk");
+ let res = verify("tests/data/v3-only-no-certs-in-sig.apk", SDK_INT);
assert!(res.is_err());
assert_contains(&res.unwrap_err().to_string(), "No certificates listed");
}
#[test]
fn test_verify_v3_no_supported_sig_algs() {
- let res = verify("tests/data/v3-only-no-supported-sig-algs.apk");
+ let res = verify("tests/data/v3-only-no-supported-sig-algs.apk", SDK_INT);
assert!(res.is_err());
assert_contains(&res.unwrap_err().to_string(), "No supported APK signatures found");
}
#[test]
fn test_verify_v3_signatures_and_digests_block_mismatch() {
- let res = verify("tests/data/v3-only-signatures-and-digests-block-mismatch.apk");
+ let res = verify("tests/data/v3-only-signatures-and-digests-block-mismatch.apk", SDK_INT);
assert!(res.is_err());
assert_contains(
&res.unwrap_err().to_string(),
@@ -203,14 +208,14 @@
/// * public key extracted from apk without verification
/// * expected public key from the corresponding .der file
fn validate_apk_public_key<P: AsRef<Path>>(apk_path: P) {
- let public_key_from_verification = verify(&apk_path);
+ let public_key_from_verification = verify(&apk_path, SDK_INT);
let public_key_from_verification =
public_key_from_verification.expect("Error in verification result");
let expected_public_key_path = format!("{}.der", apk_path.as_ref().to_str().unwrap());
assert_bytes_eq_to_data_in_file(&public_key_from_verification, expected_public_key_path);
- let public_key_from_apk = get_public_key_der(&apk_path);
+ let public_key_from_apk = get_public_key_der(&apk_path, SDK_INT);
let public_key_from_apk =
public_key_from_apk.expect("Error when extracting public key from apk");
assert_eq!(
@@ -226,15 +231,17 @@
fn validate_apk_digest<P: AsRef<Path>>(apk_path: P, expected_algorithm_id: SignatureAlgorithmID) {
let apk = fs::File::open(&apk_path).expect("Unabled to open apk file");
- let (verified_algorithm_id, verified_digest) = get_apk_digest(&apk, /*verify=*/ true)
- .expect("Error when extracting apk digest with verification.");
+ let (verified_algorithm_id, verified_digest) =
+ get_apk_digest(&apk, SDK_INT, /*verify=*/ true)
+ .expect("Error when extracting apk digest with verification.");
assert_eq!(expected_algorithm_id, verified_algorithm_id);
let expected_digest_path = format!("{}.apk_digest", apk_path.as_ref().to_str().unwrap());
assert_bytes_eq_to_data_in_file(&verified_digest, expected_digest_path);
- let (unverified_algorithm_id, unverified_digest) = get_apk_digest(&apk, /*verify=*/ false)
- .expect("Error when extracting apk digest without verification.");
+ let (unverified_algorithm_id, unverified_digest) =
+ get_apk_digest(&apk, SDK_INT, /*verify=*/ false)
+ .expect("Error when extracting apk digest without verification.");
assert_eq!(expected_algorithm_id, unverified_algorithm_id);
assert_eq!(verified_digest, unverified_digest);
}