Add support for signing with an external program

By default, the helper program is invoked in a way that is compatible with avbtool's --signing_helper. However, the arguments have been extended slightly to allow passing in the passphrase file or environment variable for non-interactive use. Fixes: #310 Signed-off-by: Andrew Gunnerson <[email protected]>
chenxiaolong · Jun 25, 2024 · 62c33bf · 62c33bf
1 parent bf885e4
commit 62c33bf
Show file tree

Hide file tree

Showing 13 changed files with 452 additions and 160 deletions.
diff --git a/README.md b/README.md
@@ -426,6 +426,28 @@ avbroot ota extract \
     --all
 ```
 
+### Signing with an external program
+
+avbroot supports delegating all RSA signing operations to an external program with the `--signing-helper` option. When using this option, the `--key-avb` and `--key-ota` options should be given a public key instead of a private key.
+
+For each signing operation, avbroot will invoke the program with:
+
+```bash
+<helper> <algorithm> <public key>
+```
+
+The algorithm is one of `SHA{256,512}_RSA{2048,4096}` and the public key is what was passed to avbroot. The program can use the public key to find the corresponding private key (eg. on a hardware security module). avbroot will write the digest to be signed to `stdin` in PKCS#1 v1.5 encoding. The helper program is expected to write the raw signature to `stdout` with no special encoding.
+
+By default, this behavior is compatible with the `--signing_helper` option in AOSP's avbtool. However, avbroot additionally extends the arguments to support non-interactive use. If `--pass-{avb,ota}-file` or `--pass-{avb,ota}-env-var` are used, then the helper program will be invoked with two additional arguments that point to the password file or environment variable.
+
+```bash
+<helper> <algorithm> <public key> file <pass file>
+# or
+<helper> <algorithm> <public key> env <env file>
+```
+
+Note that avbroot will verify the signature returned by helper program against the public key. This ensures that the patching process will fail appropriately if the wrong private key was used.
+
 ## Building from source
 
 Make sure the [Rust toolchain](https://www.rust-lang.org/) is installed. Then run:

diff --git a/avbroot/src/cli/avb.rs b/avbroot/src/cli/avb.rs
@@ -25,7 +25,7 @@ use serde::{Deserialize, Serialize};
 use tracing::{debug_span, info, warn, Span};
 
 use crate::{
-    crypto::{self, PassphraseSource},
+    crypto::{self, PassphraseSource, RsaSigningKey},
     format::avb::{
         self, AlgorithmType, AppendedDescriptorMut, AppendedDescriptorRef, Descriptor, Footer,
         HashTreeDescriptor, Header, KernelCmdlineDescriptor,
@@ -383,12 +383,26 @@ fn sign_or_clear(info: &mut AvbInfo, orig_header: &Header, key_group: &KeyGroup)
                 key_group.pass_file.as_deref(),
                 key_group.pass_env_var.as_deref(),
             );
-            let private_key = crypto::read_pem_key_file(key_path, &source)
-                .with_context(|| format!("Failed to load key: {key_path:?}"))?;
+            let signing_key = if let Some(helper) = &key_group.signing_helper {
+                let public_key = crypto::read_pem_public_key_file(key_path)
+                    .with_context(|| format!("Failed to load key: {key_path:?}"))?;
+
+                RsaSigningKey::External {
+                    program: helper.clone(),
+                    public_key_file: key_path.clone(),
+                    public_key,
+                    passphrase_source: source,
+                }
+            } else {
+                let private_key = crypto::read_pem_key_file(key_path, &source)
+                    .with_context(|| format!("Failed to load key: {key_path:?}"))?;
+
+                RsaSigningKey::Internal(private_key)
+            };
 
-            info.header.set_algo_for_key(&private_key)?;
+            info.header.set_algo_for_key(&signing_key)?;
             info.header
-                .sign(&private_key)
+                .sign(&signing_key)
                 .context("Failed to sign new AVB header")?;
         }
         SignAction::Clear => {
@@ -743,12 +757,15 @@ struct DisplayGroup {
 
 #[derive(Debug, Args)]
 struct KeyGroup {
-    /// Path to private key for signing.
+    /// Path to signing key.
     ///
-    /// A private key is needed if packing an image where the original header
+    /// A signing key is needed if packing an image where the original header
     /// was signed and the header needs to be modified (eg. for a new checksum).
-    /// If the header was originally not signed, then the private key is not
+    /// If the header was originally not signed, then the signing key is not
     /// used, unless --force is specified.
+    ///
+    /// This should normally be a private key. However, if --signing-helper is
+    /// used, then it should be a public key instead.
     #[arg(short, long, value_name = "FILE", value_parser)]
     key: Option<PathBuf>,
 
@@ -768,6 +785,15 @@ struct KeyGroup {
     /// File containing private key passphrase.
     #[arg(long, value_name = "FILE", value_parser, group = "pass")]
     pass_file: Option<PathBuf>,
+
+    /// External program for signing.
+    ///
+    /// If this option is specified, then --key must refer to a public key. The
+    /// program will be invoked as:
+    ///
+    /// <program> <algo> <public key> [file <pass file>|env <pass env>]
+    #[arg(long, value_name = "PROGRAM", value_parser)]
+    signing_helper: Option<PathBuf>,
 }
 
 /// Unpack an AVB image.

diff --git a/avbroot/src/cli/ota.rs b/avbroot/src/cli/ota.rs
@@ -21,7 +21,6 @@ use cap_std::{ambient_authority, fs::Dir};
 use cap_tempfile::TempDir;
 use clap::{value_parser, ArgAction, Args, Parser, Subcommand};
 use rayon::{iter::IntoParallelRefIterator, prelude::ParallelIterator};
-use rsa::RsaPrivateKey;
 use tempfile::NamedTempFile;
 use topological_sort::TopologicalSort;
 use tracing::{debug_span, info, warn};
@@ -30,7 +29,7 @@ use zip::{write::FileOptions, CompressionMethod, ZipArchive, ZipWriter};
 
 use crate::{
     cli,
-    crypto::{self, PassphraseSource},
+    crypto::{self, PassphraseSource, RsaSigningKey},
     format::{
         avb::{self, Descriptor, Header},
         ota::{self, SigningWriter, ZipEntry},
@@ -197,7 +196,7 @@ fn patch_boot_images<'a, 'b: 'a>(
     required_images: &'b RequiredImages,
     input_files: &mut HashMap<String, InputFile>,
     boot_patchers: Vec<Box<dyn BootImagePatch + Sync>>,
-    key_avb: &RsaPrivateKey,
+    key_avb: &RsaSigningKey,
     cancel_signal: &AtomicBool,
 ) -> Result<()> {
     let input_files = Mutex::new(input_files);
@@ -241,7 +240,7 @@ fn patch_system_image<'a, 'b: 'a>(
     required_images: &'b RequiredImages,
     input_files: &mut HashMap<String, InputFile>,
     cert_ota: &Certificate,
-    key_avb: &RsaPrivateKey,
+    key_avb: &RsaSigningKey,
     cancel_signal: &AtomicBool,
 ) -> Result<(&'b str, Vec<Range<u64>>)> {
     let Some(target) = required_images.iter_system().next() else {
@@ -565,7 +564,7 @@ fn update_vbmeta_headers(
     headers: &mut HashMap<String, Header>,
     order: &mut [(String, HashSet<String>)],
     clear_vbmeta_flags: bool,
-    key: &RsaPrivateKey,
+    key: &RsaSigningKey,
     block_size: u64,
 ) -> Result<()> {
     for (name, deps) in order {
@@ -722,8 +721,8 @@ fn patch_ota_payload(
     external_images: &HashMap<String, PathBuf>,
     boot_patchers: Vec<Box<dyn BootImagePatch + Sync>>,
     clear_vbmeta_flags: bool,
-    key_avb: &RsaPrivateKey,
-    key_ota: &RsaPrivateKey,
+    key_avb: &RsaSigningKey,
+    key_ota: &RsaSigningKey,
     cert_ota: &Certificate,
     cancel_signal: &AtomicBool,
 ) -> Result<(String, u64)> {
@@ -916,8 +915,8 @@ fn patch_ota_zip(
     external_images: &HashMap<String, PathBuf>,
     mut boot_patchers: Vec<Box<dyn BootImagePatch + Sync>>,
     clear_vbmeta_flags: bool,
-    key_avb: &RsaPrivateKey,
-    key_ota: &RsaPrivateKey,
+    key_avb: &RsaSigningKey,
+    key_ota: &RsaSigningKey,
     cert_ota: &Certificate,
     cancel_signal: &AtomicBool,
 ) -> Result<(OtaMetadata, u64)> {
@@ -1218,10 +1217,38 @@ pub fn patch_subcommand(cli: &PatchCli, cancel_signal: &AtomicBool) -> Result<()
         cli.pass_ota_env_var.as_deref(),
     );
 
-    let key_avb = crypto::read_pem_key_file(&cli.key_avb, &source_avb)
-        .with_context(|| format!("Failed to load key: {:?}", cli.key_avb))?;
-    let key_ota = crypto::read_pem_key_file(&cli.key_ota, &source_ota)
-        .with_context(|| format!("Failed to load key: {:?}", cli.key_ota))?;
+    let (key_avb, key_ota) = if let Some(helper) = &cli.signing_helper {
+        let public_key_avb = crypto::read_pem_public_key_file(&cli.key_avb)
+            .with_context(|| format!("Failed to load key: {:?}", cli.key_avb))?;
+        let public_key_ota = crypto::read_pem_public_key_file(&cli.key_ota)
+            .with_context(|| format!("Failed to load key: {:?}", cli.key_ota))?;
+
+        let key_avb = RsaSigningKey::External {
+            program: helper.clone(),
+            public_key_file: cli.key_avb.clone(),
+            public_key: public_key_avb,
+            passphrase_source: source_avb,
+        };
+        let key_ota = RsaSigningKey::External {
+            program: helper.clone(),
+            public_key_file: cli.key_ota.clone(),
+            public_key: public_key_ota,
+            passphrase_source: source_ota,
+        };
+
+        (key_avb, key_ota)
+    } else {
+        let private_key_avb = crypto::read_pem_key_file(&cli.key_avb, &source_avb)
+            .with_context(|| format!("Failed to load key: {:?}", cli.key_avb))?;
+        let private_key_ota = crypto::read_pem_key_file(&cli.key_ota, &source_ota)
+            .with_context(|| format!("Failed to load key: {:?}", cli.key_ota))?;
+
+        let key_avb = RsaSigningKey::Internal(private_key_avb);
+        let key_ota = RsaSigningKey::Internal(private_key_ota);
+
+        (key_avb, key_ota)
+    };
+
     let cert_ota = crypto::read_pem_cert_file(&cli.cert_ota)
         .with_context(|| format!("Failed to load certificate: {:?}", cli.cert_ota))?;
 
@@ -1748,7 +1775,10 @@ pub struct PatchCli {
     #[arg(short, long, value_name = "FILE", value_parser, help_heading = HEADING_PATH)]
     pub output: Option<PathBuf>,
 
-    /// Private key for signing vbmeta images.
+    /// Signing key for vbmeta headers.
+    ///
+    /// This should normally be a private key. However, if --signing-helper is
+    /// used, then it should be a public key instead.
     #[arg(
         long,
         alias = "privkey-avb",
@@ -1758,7 +1788,10 @@ pub struct PatchCli {
     )]
     pub key_avb: PathBuf,
 
-    /// Private key for signing the OTA.
+    /// Signing key for the OTA.
+    ///
+    /// This should normally be a private key. However, if --signing-helper is
+    /// used, then it should be a public key instead.
     #[arg(
         long,
         alias = "privkey-ota",
@@ -1816,6 +1849,15 @@ pub struct PatchCli {
     )]
     pub pass_ota_file: Option<PathBuf>,
 
+    /// External program for signing.
+    ///
+    /// If this option is specified, then --key-avb and --key-ota must refer to
+    /// public keys. The program will be invoked as:
+    ///
+    /// <program> <algo> <public key> [file <pass file>|env <pass env>]
+    #[arg(long, value_name = "PROGRAM", value_parser, help_heading = HEADING_KEY)]
+    pub signing_helper: Option<PathBuf>,
+
     /// Use partition image from a file instead of the original payload.
     #[arg(
         long,