Merge "Use cert hash not public key for APK authority" into main
diff --git a/libs/apkverify/Android.bp b/libs/apkverify/Android.bp
index 1c18d2d..4c5a622 100644
--- a/libs/apkverify/Android.bp
+++ b/libs/apkverify/Android.bp
@@ -48,10 +48,12 @@
test_suites: ["general-tests"],
rustlibs: [
"libandroid_logger",
+ "libanyhow",
"libapkverify",
"libapkzip",
"libbyteorder",
"liblog_rust",
+ "libopenssl",
"libzip",
],
data: ["tests/data/*"],
diff --git a/libs/apkverify/src/lib.rs b/libs/apkverify/src/lib.rs
index 6af8122..1f3c74f 100644
--- a/libs/apkverify/src/lib.rs
+++ b/libs/apkverify/src/lib.rs
@@ -26,5 +26,5 @@
mod v4;
pub use algorithms::{HashAlgorithm, SignatureAlgorithmID};
-pub use v3::{get_public_key_der, verify};
+pub use v3::{extract_signed_data, verify, SignedData};
pub use v4::{get_apk_digest, V4Signature};
diff --git a/libs/apkverify/src/v3.rs b/libs/apkverify/src/v3.rs
index 8a8ad73..88644c7 100644
--- a/libs/apkverify/src/v3.rs
+++ b/libs/apkverify/src/v3.rs
@@ -44,13 +44,9 @@
public_key: PKey<pkey::Public>,
}
-impl Signer {
- fn sdk_range(&self) -> RangeInclusive<u32> {
- self.min_sdk..=self.max_sdk
- }
-}
-
-struct SignedData {
+/// Contains the signed data part of an APK v3 signature.
+#[derive(Debug)]
+pub struct SignedData {
digests: LengthPrefixed<Vec<LengthPrefixed<Digest>>>,
certificates: LengthPrefixed<Vec<LengthPrefixed<X509Certificate>>>,
min_sdk: u32,
@@ -59,20 +55,6 @@
additional_attributes: LengthPrefixed<Vec<LengthPrefixed<AdditionalAttributes>>>,
}
-impl SignedData {
- fn sdk_range(&self) -> RangeInclusive<u32> {
- self.min_sdk..=self.max_sdk
- }
-
- fn find_digest_by_algorithm(&self, algorithm_id: SignatureAlgorithmID) -> Result<&Digest> {
- Ok(self
- .digests
- .iter()
- .find(|&dig| dig.signature_algorithm_id == Some(algorithm_id))
- .context(format!("Digest not found for algorithm: {:?}", algorithm_id))?)
- }
-}
-
#[derive(Debug)]
pub(crate) struct Signature {
/// Option is used here to allow us to ignore unsupported algorithm.
@@ -80,6 +62,7 @@
signature: LengthPrefixed<Bytes>,
}
+#[derive(Debug)]
struct Digest {
signature_algorithm_id: Option<SignatureAlgorithmID>,
digest: LengthPrefixed<Bytes>,
@@ -88,19 +71,19 @@
type X509Certificate = Bytes;
type AdditionalAttributes = Bytes;
-/// 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, current_sdk: u32) -> Result<Box<[u8]>> {
+/// Verifies APK Signature Scheme v3 signatures of the provided APK and returns the SignedData from
+/// the signature.
+pub fn verify<P: AsRef<Path>>(apk_path: P, current_sdk: u32) -> Result<SignedData> {
let apk = File::open(apk_path.as_ref())?;
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, current_sdk: u32) -> Result<Box<[u8]>> {
+/// Extracts the SignedData from the signature of the given APK. (The signature is not verified.)
+pub fn extract_signed_data<P: AsRef<Path>>(apk_path: P, current_sdk: u32) -> Result<SignedData> {
let apk = File::open(apk_path.as_ref())?;
let (signer, _) = extract_signer_and_apk_sections(apk, current_sdk)?;
- Ok(signer.public_key.public_key_to_der()?.into_boxed_slice())
+ signer.parse_signed_data()
}
pub(crate) fn extract_signer_and_apk_sections<R: Read + Seek>(
@@ -123,6 +106,10 @@
}
impl Signer {
+ fn sdk_range(&self) -> RangeInclusive<u32> {
+ self.min_sdk..=self.max_sdk
+ }
+
/// Selects the signature that has the strongest supported `SignatureAlgorithmID`.
/// The strongest signature is used in both v3 verification and v4 apk digest computation.
pub(crate) fn strongest_signature(&self) -> Result<&Signature> {
@@ -143,27 +130,33 @@
Ok(digest.digest.as_ref().to_vec().into_boxed_slice())
}
- /// Verifies the strongest signature from signatures against signed data using public key.
- /// Returns the verified signed data.
- fn verify_signature(&self, strongest: &Signature) -> Result<SignedData> {
- let mut verifier = strongest
+ /// Verifies a signature over the signed data using the public key.
+ fn verify_signature(&self, signature: &Signature) -> Result<()> {
+ let mut verifier = signature
.signature_algorithm_id
.context("Unsupported algorithm")?
.new_verifier(&self.public_key)?;
verifier.update(&self.signed_data)?;
- ensure!(verifier.verify(&strongest.signature)?, "Signature is invalid.");
- // It is now safe to parse signed data.
+ ensure!(verifier.verify(&signature.signature)?, "Signature is invalid.");
+ Ok(())
+ }
+
+ /// Returns the signed data, converted from bytes.
+ fn parse_signed_data(&self) -> Result<SignedData> {
self.signed_data.slice(..).read()
}
/// The steps in this method implements APK Signature Scheme v3 verification step 3.
- fn verify<R: Read + Seek>(&self, sections: &mut ApkSections<R>) -> Result<Box<[u8]>> {
+ fn verify<R: Read + Seek>(&self, sections: &mut ApkSections<R>) -> Result<SignedData> {
// 1. Choose the strongest supported signature algorithm ID from signatures.
let strongest = self.strongest_signature()?;
// 2. Verify the corresponding signature from signatures against signed data using public
// key.
- let verified_signed_data = self.verify_signature(strongest)?;
+ self.verify_signature(strongest)?;
+
+ // It is now safe to parse signed data.
+ let verified_signed_data = self.parse_signed_data()?;
// 3. Verify the min and max SDK versions in the signed data match those specified for the
// signer.
@@ -199,8 +192,7 @@
// 7. Verify that public key of the first certificate of certificates is identical to public
// key.
- let cert = verified_signed_data.certificates.first().context("No certificates listed")?;
- let cert = X509::from_der(cert.as_ref())?;
+ let cert = X509::from_der(verified_signed_data.first_certificate_der()?)?;
ensure!(
cert.public_key()?.public_eq(&self.public_key),
"Public key mismatch between certificate and signature record"
@@ -209,7 +201,29 @@
// TODO(b/245914104)
// 8. If the proof-of-rotation attribute exists for the signer verify that the
// struct is valid and this signer is the last certificate in the list.
- Ok(self.public_key.public_key_to_der()?.into_boxed_slice())
+
+ Ok(verified_signed_data)
+ }
+}
+
+impl SignedData {
+ /// Returns the first X.509 certificate in the signed data, encoded in DER form. (All other
+ /// certificates are ignored for v3; this certificate describes the public key that was actually
+ /// used to sign the APK.)
+ pub fn first_certificate_der(&self) -> Result<&[u8]> {
+ Ok(self.certificates.first().context("No certificates listed")?)
+ }
+
+ fn sdk_range(&self) -> RangeInclusive<u32> {
+ self.min_sdk..=self.max_sdk
+ }
+
+ fn find_digest_by_algorithm(&self, algorithm_id: SignatureAlgorithmID) -> Result<&Digest> {
+ Ok(self
+ .digests
+ .iter()
+ .find(|&dig| dig.signature_algorithm_id == Some(algorithm_id))
+ .context(format!("Digest not found for algorithm: {:?}", algorithm_id))?)
}
}
diff --git a/libs/apkverify/tests/apkverify_test.rs b/libs/apkverify/tests/apkverify_test.rs
index 0d8e020..441b708 100644
--- a/libs/apkverify/tests/apkverify_test.rs
+++ b/libs/apkverify/tests/apkverify_test.rs
@@ -14,12 +14,14 @@
* limitations under the License.
*/
+use anyhow::Result;
use apkverify::{
- get_apk_digest, get_public_key_der, testing::assert_contains, verify, SignatureAlgorithmID,
+ extract_signed_data, get_apk_digest, testing::assert_contains, verify, SignatureAlgorithmID,
};
use apkzip::zip_sections;
use byteorder::{LittleEndian, ReadBytesExt};
use log::info;
+use openssl::x509::X509;
use std::fmt::Write;
use std::io::{Seek, SeekFrom};
use std::{fs, matches, path::Path};
@@ -286,22 +288,30 @@
/// * 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, SDK_INT);
- let public_key_from_verification =
- public_key_from_verification.expect("Error in verification result");
+ let signed_data_from_verification =
+ verify(&apk_path, SDK_INT).expect("Error in verification result");
+ let cert_from_verification = signed_data_from_verification.first_certificate_der().unwrap();
+ let public_key_from_verification = public_key_der_from_cert(cert_from_verification).unwrap();
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, SDK_INT);
- let public_key_from_apk =
- public_key_from_apk.expect("Error when extracting public key from apk");
+ let signed_data_from_apk = extract_signed_data(&apk_path, SDK_INT)
+ .expect("Error when extracting signed data from apk");
+ let cert_from_apk = signed_data_from_apk.first_certificate_der().unwrap();
+ // If the two certficiates are byte for byte identical (which they should be), then so are
+ // the public keys embedded in them.
assert_eq!(
- public_key_from_verification, public_key_from_apk,
- "Public key extracted directly from apk does not match the public key from verification."
+ cert_from_verification, cert_from_apk,
+ "Certificate extracted directly from apk does not match the certificate from verification."
);
}
+fn public_key_der_from_cert(cert_der: &[u8]) -> Result<Vec<u8>> {
+ let cert = X509::from_der(cert_der)?;
+ Ok(cert.public_key()?.public_key_to_der()?)
+}
+
/// Validates that the following apk_digest are equal:
/// * apk_digest directly extracted from apk without computation
/// * computed apk_digest
diff --git a/microdroid_manager/src/dice.rs b/microdroid_manager/src/dice.rs
index e6ddfb9..0cf7013 100644
--- a/microdroid_manager/src/dice.rs
+++ b/microdroid_manager/src/dice.rs
@@ -30,17 +30,17 @@
payload_metadata: &PayloadMetadata,
) -> Result<OwnedDiceArtifacts> {
let subcomponents = build_subcomponent_list(instance_data);
- let config_descriptor = format_payload_config_descriptor(payload_metadata, &subcomponents)
+ let config_descriptor = format_payload_config_descriptor(payload_metadata, subcomponents)
.context("Building config descriptor")?;
// Calculate compound digests of code and authorities
let mut code_hash_ctx = Sha512::new();
let mut authority_hash_ctx = Sha512::new();
code_hash_ctx.update(instance_data.apk_data.root_hash.as_ref());
- authority_hash_ctx.update(instance_data.apk_data.pubkey.as_ref());
+ authority_hash_ctx.update(instance_data.apk_data.cert_hash.as_ref());
for extra_apk in &instance_data.extra_apks_data {
code_hash_ctx.update(extra_apk.root_hash.as_ref());
- authority_hash_ctx.update(extra_apk.pubkey.as_ref());
+ authority_hash_ctx.update(extra_apk.cert_hash.as_ref());
}
for apex in &instance_data.apex_data {
code_hash_ctx.update(apex.root_digest.as_ref());
@@ -57,42 +57,40 @@
dice.derive(code_hash, &config_descriptor, authority_hash, debuggable, hidden)
}
-struct Subcomponent<'a> {
+struct Subcomponent {
name: String,
version: u64,
- code_hash: &'a [u8],
- authority_hash: Box<[u8]>,
+ code_hash: Vec<u8>,
+ authority_hash: Vec<u8>,
}
-impl<'a> Subcomponent<'a> {
- fn to_value(&self) -> Result<Value> {
+impl Subcomponent {
+ fn into_value(self) -> Result<Value> {
Ok(cbor!({
1 => self.name,
2 => self.version,
- 3 => self.code_hash,
- 4 => self.authority_hash
+ 3 => Value::Bytes(self.code_hash),
+ 4 => Value::Bytes(self.authority_hash),
})?)
}
- fn for_apk(apk: &'a ApkData) -> Self {
+ fn for_apk(apk: &ApkData) -> Self {
Self {
name: format!("apk:{}", apk.package_name),
version: apk.version_code,
- code_hash: &apk.root_hash,
- authority_hash:
- // TODO(b/305925597): Hash the certificate not the pubkey
- Box::new(sha512(&apk.pubkey)),
+ code_hash: apk.root_hash.clone(),
+ authority_hash: apk.cert_hash.clone(),
}
}
- fn for_apex(apex: &'a ApexData) -> Self {
+ fn for_apex(apex: &ApexData) -> Self {
// Note that this is only reachable if the dice_changes flag is on, in which case
// the manifest data will always be present.
Self {
name: format!("apex:{}", apex.manifest_name.as_ref().unwrap()),
version: apex.manifest_version.unwrap() as u64,
- code_hash: &apex.root_digest,
- authority_hash: Box::new(sha512(&apex.public_key)),
+ code_hash: apex.root_digest.clone(),
+ authority_hash: sha512(&apex.public_key).to_vec(),
}
}
}
@@ -113,7 +111,7 @@
// of the format.
fn format_payload_config_descriptor(
payload: &PayloadMetadata,
- subcomponents: &[Subcomponent],
+ subcomponents: Vec<Subcomponent>,
) -> Result<Vec<u8>> {
let mut map = Vec::new();
map.push((cbor!(-70002)?, cbor!("Microdroid payload")?));
@@ -129,7 +127,7 @@
if !subcomponents.is_empty() {
let values =
- subcomponents.iter().map(Subcomponent::to_value).collect::<Result<Vec<_>>>()?;
+ subcomponents.into_iter().map(Subcomponent::into_value).collect::<Result<Vec<_>>>()?;
map.push((cbor!(-71002)?, cbor!(values)?));
}
@@ -141,7 +139,7 @@
use super::*;
use microdroid_metadata::PayloadConfig;
- const NO_SUBCOMPONENTS: [Subcomponent; 0] = [];
+ const NO_SUBCOMPONENTS: Vec<Subcomponent> = Vec::new();
fn assert_eq_bytes(expected: &[u8], actual: &[u8]) {
assert_eq!(
@@ -157,7 +155,7 @@
fn payload_metadata_with_path_formats_correctly() -> Result<()> {
let payload_metadata = PayloadMetadata::ConfigPath("/config_path".to_string());
let config_descriptor =
- format_payload_config_descriptor(&payload_metadata, &NO_SUBCOMPONENTS)?;
+ format_payload_config_descriptor(&payload_metadata, NO_SUBCOMPONENTS)?;
static EXPECTED_CONFIG_DESCRIPTOR: &[u8] = &[
0xa2, 0x3a, 0x00, 0x01, 0x11, 0x71, 0x72, 0x4d, 0x69, 0x63, 0x72, 0x6f, 0x64, 0x72,
0x6f, 0x69, 0x64, 0x20, 0x70, 0x61, 0x79, 0x6c, 0x6f, 0x61, 0x64, 0x3a, 0x00, 0x01,
@@ -176,7 +174,7 @@
};
let payload_metadata = PayloadMetadata::Config(payload_config);
let config_descriptor =
- format_payload_config_descriptor(&payload_metadata, &NO_SUBCOMPONENTS)?;
+ format_payload_config_descriptor(&payload_metadata, NO_SUBCOMPONENTS)?;
static EXPECTED_CONFIG_DESCRIPTOR: &[u8] = &[
0xa2, 0x3a, 0x00, 0x01, 0x11, 0x71, 0x72, 0x4d, 0x69, 0x63, 0x72, 0x6f, 0x64, 0x72,
0x6f, 0x69, 0x64, 0x20, 0x70, 0x61, 0x79, 0x6c, 0x6f, 0x61, 0x64, 0x3a, 0x00, 0x01,
@@ -190,31 +188,30 @@
#[test]
fn payload_metadata_with_subcomponents_formats_correctly() -> Result<()> {
let payload_metadata = PayloadMetadata::ConfigPath("/config_path".to_string());
- let subcomponents = [
+ let subcomponents = vec![
Subcomponent {
name: "apk1".to_string(),
version: 1,
- code_hash: &[42u8],
- authority_hash: Box::new([17u8]),
+ code_hash: vec![42, 43],
+ authority_hash: vec![17],
},
Subcomponent {
name: "apk2".to_string(),
version: 0x1000_0000_0001,
- code_hash: &[43u8],
- authority_hash: Box::new([19u8]),
+ code_hash: vec![43],
+ authority_hash: vec![19, 20],
},
];
- let config_descriptor =
- format_payload_config_descriptor(&payload_metadata, &subcomponents)?;
+ let config_descriptor = format_payload_config_descriptor(&payload_metadata, subcomponents)?;
// Verified using cbor.me.
static EXPECTED_CONFIG_DESCRIPTOR: &[u8] = &[
0xa3, 0x3a, 0x00, 0x01, 0x11, 0x71, 0x72, 0x4d, 0x69, 0x63, 0x72, 0x6f, 0x64, 0x72,
0x6f, 0x69, 0x64, 0x20, 0x70, 0x61, 0x79, 0x6c, 0x6f, 0x61, 0x64, 0x3a, 0x00, 0x01,
0x15, 0x57, 0x6c, 0x2f, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x5f, 0x70, 0x61, 0x74,
0x68, 0x3a, 0x00, 0x01, 0x15, 0x59, 0x82, 0xa4, 0x01, 0x64, 0x61, 0x70, 0x6b, 0x31,
- 0x02, 0x01, 0x03, 0x81, 0x18, 0x2a, 0x04, 0x81, 0x11, 0xa4, 0x01, 0x64, 0x61, 0x70,
- 0x6b, 0x32, 0x02, 0x1b, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03, 0x81,
- 0x18, 0x2b, 0x04, 0x81, 0x13,
+ 0x02, 0x01, 0x03, 0x42, 0x2a, 0x2b, 0x04, 0x41, 0x11, 0xa4, 0x01, 0x64, 0x61, 0x70,
+ 0x6b, 0x32, 0x02, 0x1b, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03, 0x41,
+ 0x2b, 0x04, 0x42, 0x13, 0x14,
];
assert_eq_bytes(EXPECTED_CONFIG_DESCRIPTOR, &config_descriptor);
Ok(())
diff --git a/microdroid_manager/src/instance.rs b/microdroid_manager/src/instance.rs
index b0fc03d..7a9f0e0 100644
--- a/microdroid_manager/src/instance.rs
+++ b/microdroid_manager/src/instance.rs
@@ -287,20 +287,18 @@
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
pub struct ApkData {
- pub root_hash: Box<RootHash>,
- pub pubkey: Box<[u8]>,
+ pub root_hash: Vec<u8>,
+ pub cert_hash: Vec<u8>,
pub package_name: String,
pub version_code: u64,
}
impl ApkData {
pub fn root_hash_eq(&self, root_hash: &[u8]) -> bool {
- self.root_hash.as_ref() == root_hash
+ self.root_hash == root_hash
}
}
-pub type RootHash = [u8];
-
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
pub struct ApexData {
pub name: String,
diff --git a/microdroid_manager/src/verify.rs b/microdroid_manager/src/verify.rs
index e63530b..445c1ae 100644
--- a/microdroid_manager/src/verify.rs
+++ b/microdroid_manager/src/verify.rs
@@ -12,16 +12,17 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-use crate::instance::{ApexData, ApkData, MicrodroidData, RootHash};
+use crate::instance::{ApexData, ApkData, MicrodroidData};
use crate::payload::{get_apex_data_from_payload, to_metadata};
use crate::{is_strict_boot, is_verified_boot, MicrodroidError};
use anyhow::{anyhow, ensure, Context, Result};
use apkmanifest::get_manifest_info;
-use apkverify::{get_public_key_der, verify, V4Signature};
+use apkverify::{extract_signed_data, verify, V4Signature};
use glob::glob;
use itertools::sorted;
use log::{info, warn};
use microdroid_metadata::{write_metadata, Metadata};
+use openssl::sha::sha512;
use rand::Fill;
use rustutils::system_properties;
use std::fs::OpenOptions;
@@ -190,10 +191,10 @@
fn get_data_from_apk(
apk_path: &str,
- root_hash: Box<RootHash>,
+ root_hash: Box<[u8]>,
root_hash_trustful: bool,
) -> Result<ApkData> {
- let pubkey = get_public_key_from_apk(apk_path, root_hash_trustful)?;
+ let cert_hash = get_cert_hash_from_apk(apk_path, root_hash_trustful)?.to_vec();
// Read package name etc from the APK manifest. In the unlikely event that they aren't present
// we use the default values. We simply put these values in the DICE node for the payload, and
// users of that can decide how to handle blank information - there's no reason for us
@@ -203,8 +204,8 @@
.unwrap_or_default();
Ok(ApkData {
- root_hash,
- pubkey,
+ root_hash: root_hash.into(),
+ cert_hash,
package_name: manifest_info.package,
version_code: manifest_info.version_code,
})
@@ -233,21 +234,22 @@
Ok(())
}
-fn get_apk_root_hash_from_idsig<P: AsRef<Path>>(idsig_path: P) -> Result<Box<RootHash>> {
+fn get_apk_root_hash_from_idsig<P: AsRef<Path>>(idsig_path: P) -> Result<Box<[u8]>> {
Ok(V4Signature::from_idsig_path(idsig_path)?.hashing_info.raw_root_hash)
}
-fn get_public_key_from_apk(apk: &str, root_hash_trustful: bool) -> Result<Box<[u8]>> {
+fn get_cert_hash_from_apk(apk: &str, root_hash_trustful: bool) -> Result<[u8; 64]> {
let current_sdk = get_current_sdk()?;
- if !root_hash_trustful {
+ let signed_data = if !root_hash_trustful {
verify(apk, current_sdk).context(MicrodroidError::PayloadVerificationFailed(format!(
"failed to verify {}",
apk
)))
} else {
- get_public_key_der(apk, current_sdk)
- }
+ extract_signed_data(apk, current_sdk)
+ }?;
+ Ok(sha512(signed_data.first_certificate_der()?))
}
fn get_current_sdk() -> Result<u32> {
@@ -260,7 +262,7 @@
apk: &'a str,
idsig: &'a str,
name: &'a str,
- saved_root_hash: Option<&'a RootHash>,
+ saved_root_hash: Option<&'a [u8]>,
}
fn run_apkdmverity(args: &[ApkDmverityArgument]) -> Result<Child> {