Use cert hash not public key for APK authority
Previously we were using the public key of an APK as the input to the
authority hash for the VM, and as the authority hash in its
Subcomponent - as we do for an APEX.
Instead, use a hash of the certificate. Android has always required
the certificate to be consistent over versions of an APK, not just the
public key, and a hash of the certificate (with the package name) is
widely used to uniquely identify an APK.
This triggered slightly more refactoring than was perhaps strictly
necessary. I didn't want libapkverify to force a choice of what the
relevant data was; instead we return the SignedData and let the client
request what they want.
I removed the RootHash typdef, as it seemed to me it was hiding
information rather than making it clear.
Bug: 305925597
Test: atest libapkverify.test libapkverify.integration_test
Test: atest microdroid_manager_test
Test: atest MicrodroidTests
Change-Id: I7669fc468802d25a422e81d344e6655df5b0e636
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> {