Clean up imports, formatting and tests.
Bug: n/a
Test: atest libapkverify.test libapkverify.integration_test
Change-Id: I39686d17927741297aa3feb02ed6043ee770933b
diff --git a/apkverify/src/bytes_ext.rs b/apkverify/src/bytes_ext.rs
index 1b8d6b6..22a3085 100644
--- a/apkverify/src/bytes_ext.rs
+++ b/apkverify/src/bytes_ext.rs
@@ -98,7 +98,9 @@
#[cfg(test)]
mod tests {
+ use super::*;
use bytes::{BufMut, BytesMut};
+
#[test]
fn test_read_length_prefixed_slice() {
let data = b"hello world";
@@ -106,7 +108,7 @@
b.put_u32_le(data.len() as u32);
b.put_slice(data);
let mut slice = b.freeze();
- let res = super::read_length_prefixed_slice(&mut slice);
+ let res = read_length_prefixed_slice(&mut slice);
assert!(res.is_ok());
assert_eq!(data, res.ok().unwrap().as_ref());
}
diff --git a/apkverify/src/lib.rs b/apkverify/src/lib.rs
index 869431e..f75913c 100644
--- a/apkverify/src/lib.rs
+++ b/apkverify/src/lib.rs
@@ -18,7 +18,8 @@
mod bytes_ext;
mod sigutil;
-mod testing;
+#[allow(dead_code)]
+pub mod testing;
mod v3;
mod ziputil;
diff --git a/apkverify/src/sigutil.rs b/apkverify/src/sigutil.rs
index 06645fe..fc92898 100644
--- a/apkverify/src/sigutil.rs
+++ b/apkverify/src/sigutil.rs
@@ -18,7 +18,7 @@
use anyhow::{anyhow, bail, Result};
use byteorder::{LittleEndian, ReadBytesExt};
-use bytes::{Buf, BufMut, Bytes};
+use bytes::{Buf, BufMut, Bytes, BytesMut};
use ring::digest;
use std::cmp::min;
use std::io::{Cursor, Read, Seek, SeekFrom, Take};
@@ -61,11 +61,11 @@
impl<R: Read + Seek> ApkSections<R> {
pub fn new(reader: R) -> Result<ApkSections<R>> {
- let (mut f, zip_sections) = zip_sections(reader)?;
+ let (mut reader, zip_sections) = zip_sections(reader)?;
let (signing_block_offset, signing_block_size) =
- find_signing_block(&mut f, zip_sections.central_directory_offset)?;
+ find_signing_block(&mut reader, zip_sections.central_directory_offset)?;
Ok(ApkSections {
- inner: f,
+ inner: reader,
signing_block_offset,
signing_block_size,
central_directory_offset: zip_sections.central_directory_offset,
@@ -93,7 +93,7 @@
pub fn compute_digest(&mut self, signature_algorithm_id: u32) -> Result<Vec<u8>> {
let digester = Digester::new(signature_algorithm_id)?;
- let mut digests_of_chunks = bytes::BytesMut::new();
+ let mut digests_of_chunks = BytesMut::new();
let mut chunk_count = 0u32;
let mut chunk = vec![0u8; CHUNK_SIZE_BYTES as usize];
for data in &[
@@ -118,6 +118,7 @@
fn zip_entries(&mut self) -> Result<Take<Box<dyn Read + '_>>> {
scoped_read(&mut self.inner, 0, self.signing_block_offset as u64)
}
+
fn central_directory(&mut self) -> Result<Take<Box<dyn Read + '_>>> {
scoped_read(
&mut self.inner,
@@ -125,6 +126,7 @@
self.central_directory_size as u64,
)
}
+
fn eocd_for_verification(&mut self) -> Result<Take<Box<dyn Read + '_>>> {
let mut eocd = self.bytes(self.eocd_offset, self.eocd_size)?;
// Protection of section 4 (ZIP End of Central Directory) is complicated by the section
@@ -136,6 +138,7 @@
set_central_directory_offset(&mut eocd, self.signing_block_offset)?;
Ok(Read::take(Box::new(Cursor::new(eocd)), self.eocd_size as u64))
}
+
fn bytes(&mut self, offset: u32, size: u32) -> Result<Vec<u8>> {
self.inner.seek(SeekFrom::Start(offset as u64))?;
let mut buf = vec![0u8; size as usize];
@@ -159,6 +162,7 @@
const CHUNK_HEADER_TOP: &[u8] = &[0x5a];
const CHUNK_HEADER_MID: &[u8] = &[0xa5];
+
impl Digester {
fn new(signature_algorithm_id: u32) -> Result<Digester> {
let digest_algorithm_id = to_content_digest_algorithm(signature_algorithm_id)?;
@@ -173,6 +177,7 @@
};
Ok(Digester { algorithm })
}
+
// v2/v3 digests are computed after prepending "header" byte and "size" info.
fn digest(&self, data: &[u8], header: &[u8], size: u32) -> digest::Digest {
let mut ctx = digest::Context::new(self.algorithm);
diff --git a/apkverify/src/testing.rs b/apkverify/src/testing.rs
index 777afb8..301a9c3 100644
--- a/apkverify/src/testing.rs
+++ b/apkverify/src/testing.rs
@@ -16,19 +16,7 @@
//! A collection of utilities for testing
-/// Asserts if `haystack.contains(needed)`
-#[macro_export]
-macro_rules! assert_contains {
- ($haystack:expr,$needle:expr $(,)?) => {
- match (&$haystack, &$needle) {
- (haystack_value, needle_value) => {
- assert!(
- haystack_value.contains(needle_value),
- "{} is not found in {}",
- needle_value,
- haystack_value
- );
- }
- }
- };
+/// Asserts if `haystack.contains(needle)`
+pub fn assert_contains(haystack: &str, needle: &str) {
+ assert!(haystack.contains(needle), "{} is not found in {}", needle, haystack);
}
diff --git a/apkverify/src/v3.rs b/apkverify/src/v3.rs
index d1f59c6..2f9ce94 100644
--- a/apkverify/src/v3.rs
+++ b/apkverify/src/v3.rs
@@ -21,11 +21,15 @@
use anyhow::{anyhow, bail, Context, Result};
use bytes::Bytes;
+use ring::signature::{
+ UnparsedPublicKey, VerificationAlgorithm, ECDSA_P256_SHA256_ASN1, RSA_PKCS1_2048_8192_SHA256,
+ RSA_PKCS1_2048_8192_SHA512, RSA_PSS_2048_8192_SHA256, RSA_PSS_2048_8192_SHA512,
+};
use std::fs::File;
use std::io::{Read, Seek};
use std::ops::Range;
use std::path::Path;
-use x509_parser::{prelude::FromDer, x509};
+use x509_parser::{parse_x509_certificate, prelude::FromDer, x509::SubjectPublicKeyInfo};
use crate::bytes_ext::{BytesExt, LengthPrefixed, ReadFromBytes};
use crate::sigutil::*;
@@ -45,7 +49,7 @@
min_sdk: u32,
max_sdk: u32,
signatures: LengthPrefixed<Vec<LengthPrefixed<Signature>>>,
- public_key: LengthPrefixed<SubjectPublicKeyInfo>,
+ public_key: LengthPrefixed<Bytes>,
}
impl Signer {
@@ -79,7 +83,6 @@
digest: LengthPrefixed<Bytes>,
}
-type SubjectPublicKeyInfo = Bytes;
type X509Certificate = Bytes;
type AdditionalAttributes = Bytes;
@@ -128,7 +131,7 @@
// 2. Verify the corresponding signature from signatures against signed data using public key.
// (It is now safe to parse signed data.)
- let (_, key_info) = x509::SubjectPublicKeyInfo::from_der(self.public_key.as_ref())?;
+ let (_, key_info) = SubjectPublicKeyInfo::from_der(self.public_key.as_ref())?;
verify_signed_data(&self.signed_data, strongest, &key_info)?;
// It is now safe to parse signed data.
@@ -172,7 +175,7 @@
// 7. Verify that SubjectPublicKeyInfo of the first certificate of certificates is identical
// to public key.
let cert = signed_data.certificates.first().context("No certificates listed")?;
- let (_, cert) = x509_parser::parse_x509_certificate(cert.as_ref())?;
+ let (_, cert) = parse_x509_certificate(cert.as_ref())?;
if cert.tbs_certificate.subject_pki != key_info {
bail!("Public key mismatch between certificate and signature record");
}
@@ -185,32 +188,28 @@
fn verify_signed_data(
data: &Bytes,
signature: &Signature,
- key_info: &x509::SubjectPublicKeyInfo,
+ key_info: &SubjectPublicKeyInfo,
) -> Result<()> {
- use ring::signature;
- let verification_alg: &dyn signature::VerificationAlgorithm =
- match signature.signature_algorithm_id {
- SIGNATURE_RSA_PSS_WITH_SHA256 => &signature::RSA_PSS_2048_8192_SHA256,
- SIGNATURE_RSA_PSS_WITH_SHA512 => &signature::RSA_PSS_2048_8192_SHA512,
- SIGNATURE_RSA_PKCS1_V1_5_WITH_SHA256 | SIGNATURE_VERITY_RSA_PKCS1_V1_5_WITH_SHA256 => {
- &signature::RSA_PKCS1_2048_8192_SHA256
- }
- SIGNATURE_RSA_PKCS1_V1_5_WITH_SHA512 => &signature::RSA_PKCS1_2048_8192_SHA512,
- SIGNATURE_ECDSA_WITH_SHA256 | SIGNATURE_VERITY_ECDSA_WITH_SHA256 => {
- &signature::ECDSA_P256_SHA256_ASN1
- }
- // TODO(b/190343842) not implemented signature algorithm
- SIGNATURE_ECDSA_WITH_SHA512
- | SIGNATURE_DSA_WITH_SHA256
- | SIGNATURE_VERITY_DSA_WITH_SHA256 => {
- bail!(
- "TODO(b/190343842) not implemented signature algorithm: {:#x}",
- signature.signature_algorithm_id
- );
- }
- _ => bail!("Unsupported signature algorithm: {:#x}", signature.signature_algorithm_id),
- };
- let key = signature::UnparsedPublicKey::new(verification_alg, &key_info.subject_public_key);
+ let verification_alg: &dyn VerificationAlgorithm = match signature.signature_algorithm_id {
+ SIGNATURE_RSA_PSS_WITH_SHA256 => &RSA_PSS_2048_8192_SHA256,
+ SIGNATURE_RSA_PSS_WITH_SHA512 => &RSA_PSS_2048_8192_SHA512,
+ SIGNATURE_RSA_PKCS1_V1_5_WITH_SHA256 | SIGNATURE_VERITY_RSA_PKCS1_V1_5_WITH_SHA256 => {
+ &RSA_PKCS1_2048_8192_SHA256
+ }
+ SIGNATURE_RSA_PKCS1_V1_5_WITH_SHA512 => &RSA_PKCS1_2048_8192_SHA512,
+ SIGNATURE_ECDSA_WITH_SHA256 | SIGNATURE_VERITY_ECDSA_WITH_SHA256 => &ECDSA_P256_SHA256_ASN1,
+ // TODO(b/190343842) not implemented signature algorithm
+ SIGNATURE_ECDSA_WITH_SHA512
+ | SIGNATURE_DSA_WITH_SHA256
+ | SIGNATURE_VERITY_DSA_WITH_SHA256 => {
+ bail!(
+ "TODO(b/190343842) not implemented signature algorithm: {:#x}",
+ signature.signature_algorithm_id
+ );
+ }
+ _ => bail!("Unsupported signature algorithm: {:#x}", signature.signature_algorithm_id),
+ };
+ let key = UnparsedPublicKey::new(verification_alg, &key_info.subject_public_key);
key.verify(data.as_ref(), signature.signature.as_ref())?;
Ok(())
}
diff --git a/apkverify/src/ziputil.rs b/apkverify/src/ziputil.rs
index bfb1c01..f18a38a 100644
--- a/apkverify/src/ziputil.rs
+++ b/apkverify/src/ziputil.rs
@@ -93,7 +93,7 @@
#[cfg(test)]
mod tests {
use super::*;
- use crate::assert_contains;
+ use crate::testing::assert_contains;
use std::io::{Cursor, Write};
use zip::{write::FileOptions, ZipWriter};
@@ -125,6 +125,6 @@
// ZipArchive::new() succeeds, but we should reject
let res = zip_sections(Cursor::new([pre_eocd, cd, eocd].concat()));
assert!(res.is_err());
- assert_contains!(res.err().unwrap().to_string(), "Invalid ZIP: offset should be 0");
+ assert_contains(&res.err().unwrap().to_string(), "Invalid ZIP: offset should be 0");
}
}
diff --git a/apkverify/tests/apkverify_test.rs b/apkverify/tests/apkverify_test.rs
index 3366524..ade4468 100644
--- a/apkverify/tests/apkverify_test.rs
+++ b/apkverify/tests/apkverify_test.rs
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-use apkverify::{assert_contains, verify};
+use apkverify::{testing::assert_contains, verify};
use std::matches;
#[test]
@@ -26,14 +26,14 @@
fn test_verify_v3_digest_mismatch() {
let res = verify("tests/data/v3-only-with-rsa-pkcs1-sha512-8192-digest-mismatch.apk");
assert!(res.is_err());
- assert_contains!(res.err().unwrap().to_string(), "Digest mismatch");
+ assert_contains(&res.unwrap_err().to_string(), "Digest mismatch");
}
#[test]
-fn test_verify_v3_cert_and_publick_key_mismatch() {
+fn test_verify_v3_cert_and_public_key_mismatch() {
let res = verify("tests/data/v3-only-cert-and-public-key-mismatch.apk");
assert!(res.is_err());
- assert_contains!(res.err().unwrap().to_string(), "Public key mismatch");
+ assert_contains(&res.unwrap_err().to_string(), "Public key mismatch");
}
#[test]
@@ -42,7 +42,7 @@
let res = verify("tests/data/v2-only-truncated-cd.apk");
// TODO(jooyung): consider making a helper for err assertion
assert!(matches!(
- res.err().unwrap().root_cause().downcast_ref::<ZipError>().unwrap(),
+ res.unwrap_err().root_cause().downcast_ref::<ZipError>().unwrap(),
ZipError::InvalidArchive(_),
));
}
diff --git a/idsig/src/apksigv4.rs b/idsig/src/apksigv4.rs
index 6f4603d..5f4e92a 100644
--- a/idsig/src/apksigv4.rs
+++ b/idsig/src/apksigv4.rs
@@ -183,7 +183,7 @@
#[cfg(test)]
mod tests {
- use crate::apksigv4::*;
+ use super::*;
use std::io::Cursor;
fn hexstring_from(s: &[u8]) -> String {
diff --git a/idsig/src/hashtree.rs b/idsig/src/hashtree.rs
index a4727a9..b91698b 100644
--- a/idsig/src/hashtree.rs
+++ b/idsig/src/hashtree.rs
@@ -191,7 +191,7 @@
#[cfg(test)]
mod tests {
- use crate::hashtree::*;
+ use super::*;
use ring::digest;
use std::fs::{self, File};