Closed szescxz closed 1 week ago
chenxiaolong
commented
1 week ago avbroot currently only supports using private keys from a PEM-encoded file.
I'll have to do some investigation to see how feasible it is to add support for other signing methods. Can you provide an example of a command you would use to sign with a hardware key with --signing_helper?
szescxz
commented
1 week ago Let me use the example from the README - the original command for patching the OTA zip is
avbroot ota patch --input /path/to/ota.zip --key-avb /path/to/avb.key --key-ota /path/to/ota.key --cert-ota /path/to/ota.crtNow, suppose that
avbroot/path/to/avb.key and /path/to/ota.key are PEM-encoded public keys/path/to/helper; this allows user to fully customize the signing method without putting too much burden on avbroot's developmentSo the command goes like
avbroot ota patch --input /path/to/ota.zip --key-avb /path/to/avb.key --key-ota /path/to/ota.key --cert-ota /path/to/ota.crt --signing_helper /path/to/helperThen, whenever avbroot wants to sign a payload, just like that in the avbtool docs, it invokes the helper script, specifying the required algorithm and the corresponding public key:
/path/to/helper SHA256_RSA4096 /path/to/avb.keyMy helper script will be responsible for locating the correct private key slot according to the given public key file, and ask the hardware key to use that slot to perform the signing operation.
Then, avbroot feeds the input to STDIN, checks the return code of the helper, and gets the signature from STDOUT. avbroot may also double check the signature is indeed valid.
chenxiaolong
commented
1 week ago Thanks for the explanation! I was originally thinking of implementing PKCS#11 (or whatever hardware interface) directly, but invoking an external command is more flexible as you said.
I'd like to try to maintain compatibility with avbtool. So by default, it will invoke:
<helper> SHA{256,512}_RSA{2048,4096} public.keywrite a PKCS#1 v1.5 encoded digest to stdin and read back a raw RSA signature (no encoding) from stdout.
However, I would like to extend it a little bit to support avbroot's non-interactive options.
If --pass-avb-file <file> or --pass-ota-file <file> are used, then the command is invoked as:
<helper> SHA256_RSA4096 public.key file <file>or if --pass-avb-env-var <var> or --pass-ota-env-var <var> are used, then the command is invoked as:
<helper> SHA256_RSA4096 public.key env <var>
avbrootmay also double check the signature is indeed valid.
Yeah, good idea. I'll make avbroot verify the signature against the public key. It'll help protect against scenarios where the helper command signed with the wrong private key.
chenxiaolong
commented
1 week ago This has been implemented in #312. Would you mind giving that a try? There are precompiled binaries in the Github Actions job if you prefer not to compile avbroot from source.
I'm not super familiar with signing things with my Yubikey, so I tested it with openssl using this helper script:
#!/bin/bash
set -euo pipefail
algorithm=${1}
public_key=${2}
pass_type=${3:-}
pass_source=${4:-}
cmd=(
openssl pkeyutl
-sign
-inkey "${public_key/.public/}"
)
case "${algorithm}" in
SHA256_*)
cmd+=(-pkeyopt digest:sha256)
;;
SHA512_*)
cmd+=(-pkeyopt digest:sha512)
;;
*)
echo >&2 "Unknown algorithm: ${algorithm}"
exit 1
;;
esac
case "${pass_type}" in
file|env)
cmd+=(-passin "${pass_type}:${pass_source}")
;;
*)
echo >&2 "Unknown password type: ${pass_type}"
exit 1
;;
esac
exec "${cmd[@]}"Thank you so much! Just finished testing with an actual YubiKey on my side (although RSA 2048 only), and left some comments in the PR.
chenxiaolong
commented
1 week ago Thanks for the yubikey example! The firmware on mine is too old and it's not upgradable, so I'm stuck with RSA2048. I generated a key with:
ykman piv keys generate --algorithm RSA2048 9c yubikey.public.key
ykman piv certificates generate --subject avbroot 9c yubikey.public.keyHowever, I wasn't able to figure out how get PivSession.sign() to accept a pre-padded digest. Do you know if that's possible? Setting padding=None results in:
Traceback (most recent call last):
File "/home/chenxiaolong/git/github/avbroot/./old/issue310_helper.py", line 165, in <module>
piv.sign(
File "/usr/lib/python3.12/site-packages/yubikit/piv.py", line 826, in sign
padded = _pad_message(key_type, message, hash_algorithm, padding)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.12/site-packages/yubikit/piv.py", line 399, in _pad_message
signature = dummy.sign(message, padding, hash_algorithm)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.12/site-packages/cryptography/hazmat/backends/openssl/rsa.py", line 512, in sign
data, algorithm = _calculate_digest_and_algorithm(data, algorithm)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.12/site-packages/cryptography/hazmat/backends/openssl/utils.py", line 58, in _calculate_digest_and_algorithm
raise ValueError(
ValueError: The provided data must be the same length as the hash algorithm's digest size.Any thoughts on whether passing a padded digest to the helper script is too painful of an API? With openssl, openssl rsautl -raw works, but it's deprecated and the openssl pkeyutl replacement also doesn't support signing a pre-padded digest.
(If using a raw digest is a better idea, I don't mind making the API incompatible with avbtool's --signing_helper. But if we do that, is there anything else in the API that we should change too?)
szescxz
commented
1 week ago However, I wasn't able to figure out how get PivSession.sign() to accept a pre-padded digest. Do you know if that's possible?
Did some digging on yubikey-manager source code, the padding is actually not generated on the YubiKey, so I modified my script to skip to the internal sign function directly. Usage of such hacks in a user-supplied script sounds totally acceptable to me.
Any thoughts on whether passing a padded digest to the helper script is too painful of an API?
My personal opinion is that while AVB does use PKCS1 currently, we don't know if Google will ever switch to a non-standard padding algorithm. Also just like the conclusion above, at least for YubiKeys it only accepts pre-hashed and pre-padded data as the input.
Here goes my updated script, now with proper argparse -
#!/usr/bin/env python3
import argparse
import io
import sys
from datetime import datetime, timedelta
from typing import Mapping, Optional
from ykman.device import list_all_devices, YkmanDevice
from ykman.piv import generate_self_signed_certificate
from yubikit.core.smartcard import ApduError, SmartCardConnection, SW
from yubikit.piv import ALGORITHM, DEFAULT_MANAGEMENT_KEY, KEY_TYPE, PIN_POLICY, PivSession, SLOT, SlotMetadata
from cryptography import x509
from cryptography.hazmat.primitives import hashes
from cryptography.hazmat.primitives.asymmetric import rsa
from cryptography.hazmat.primitives.serialization import Encoding, PublicFormat, load_pem_public_key
SUPPORTED_KEY_TYPE = [
KEY_TYPE.RSA2048,
KEY_TYPE.RSA4096
]
def eprint(*args, **kwargs):
return print(*args, file=sys.stderr, **kwargs)
def parse_slot(string: str) -> SLOT:
return SLOT(int(string.strip().lower(), 16))
def parse_hash_algorithm(hash_name: str) -> hashes.HashAlgorithm:
hash_name = hash_name.upper()
if hash_name == "SHA256":
return hashes.SHA256
elif hash_name == "SHA512":
return hashes.SHA512
else:
raise NotImplementedError
def parse_key_type(string: str) -> KEY_TYPE:
return KEY_TYPE[string]
def get_device() -> YkmanDevice:
try:
dev, info = list_all_devices()[0]
except IndexError:
eprint("No device detected")
exit(-1)
return dev
def list_keys(session: PivSession) -> Mapping[SLOT, Optional[SlotMetadata]]:
keys = {}
for slot in set(SLOT) - {SLOT.ATTESTATION}:
try:
keys[slot] = session.get_slot_metadata(slot=slot)
except ApduError as e:
if e.sw != SW.REFERENCE_DATA_NOT_FOUND:
raise
return keys
def auth_with_management_key(piv: PivSession, management_key: Optional[bytes]=None):
metadata = piv.get_management_key_metadata()
if metadata.default_value:
eprint(f"Authenticating with default management key")
management_key = DEFAULT_MANAGEMENT_KEY
elif management_key is None:
raise NotImplementedError(f"You are not using the default management key which is currently unsupported by this script. Please modify the script and supply the PIN via environment variable or a file.")
try:
piv.authenticate(key_type=metadata.key_type, management_key=management_key)
except ApduError as e:
if e.sw == SW.SECURITY_CONDITION_NOT_SATISFIED:
eprint(f"Incorrect management key")
exit(-1)
else:
raise
def auth_with_pin(piv: PivSession, pin: Optional[str]=None):
if piv.get_pin_metadata().default_value:
eprint(f"Authenticating with default PIN")
pin = "123456"
elif pin is None:
raise NotImplementedError(f"You are not using the default PIN which is currently unsupported by this script. Please modify the script and supply the PIN via environment variable or a file.")
try:
piv.verify_pin(pin=pin)
except ApduError as e:
if e.sw == SW.SECURITY_CONDITION_NOT_SATISFIED:
eprint(f"Incorrect PIN")
exit(-1)
else:
raise
def export_public_key(public_key: rsa.RSAPublicKey, output_file: io.FileIO):
output_file.write(public_key.public_bytes(Encoding.PEM, PublicFormat.SubjectPublicKeyInfo))
def export_certificate(certificate: x509.Certificate, output_file: io.FileIO):
output_file.write(certificate.public_bytes(Encoding.PEM))
def cmd_list_keys(args):
with get_device().open_connection(SmartCardConnection) as connection:
piv = PivSession(connection)
keys = [(s, m) for (s, m) in list_keys(piv).items() if m.key_type in SUPPORTED_KEY_TYPE]
if len(keys) == 0:
eprint("No supported keys available")
exit(-1)
eprint("Keys available:")
for slot, metadata in keys:
eprint(f"slot: {slot}, key_type: {metadata.key_type}")
def cmd_generate_key(args):
with get_device().open_connection(SmartCardConnection) as connection:
piv = PivSession(connection)
auth_with_management_key(piv)
public_key = piv.generate_key(slot=args.slot, key_type=args.algorithm)
export_public_key(public_key, args.output)
def cmd_generate_cert(args):
with get_device().open_connection(SmartCardConnection) as connection:
piv = PivSession(connection)
auth_with_pin(piv)
now = datetime.now()
certificate = generate_self_signed_certificate(
session=piv,
slot=args.slot,
public_key=piv.get_slot_metadata(args.slot).public_key,
subject_str=args.subject,
valid_from=now,
valid_to=now + timedelta(days=args.validity),
hash_algorithm=hashes.SHA256
)
export_certificate(certificate, args.output)
def cmd_export_key(args):
with get_device().open_connection(SmartCardConnection) as connection:
piv = PivSession(connection)
try:
metadata = piv.get_slot_metadata(args.slot)
except ApduError as e:
if e.sw == SW.REFERENCE_DATA_NOT_FOUND:
eprint(f"Specified slot {args.slot:x} is empty")
exit(-1)
else:
raise
export_public_key(metadata.public_key, args.output)
def cmd_export_cert(args):
with get_device().open_connection(SmartCardConnection) as connection:
piv = PivSession(connection)
try:
certificate = piv.get_certificate(args.slot)
except ApduError as e:
if e.sw == SW.FILE_NOT_FOUND:
eprint(f"Specified slot {args.slot:x} does not have a certificate")
exit(-1)
else:
raise
export_certificate(certificate, args.output)
def run_signing_helper():
if len(sys.argv) < 3:
parser.print_usage(sys.stderr)
exit(-1)
try:
hash_name, key_type_str = sys.argv[1].upper().split("_")
except ValueError:
parser.print_usage(sys.stderr)
exit(-1)
parse_hash_algorithm(hash_name) # just for validating the input; we don't really care since the input shall be pre-hashed anyway
key_type = parse_key_type(key_type_str)
path_to_public_key = sys.argv[2]
with open(path_to_public_key, "rb") as f:
public_key = load_pem_public_key(f.read())
padding_and_hash = sys.stdin.buffer.read()
# check the STDIN is indeed padded
try:
if key_type.algorithm == ALGORITHM.RSA:
assert len(padding_and_hash) * 8 == key_type.bit_len
else:
raise NotImplementedError
except AssertionError:
eprint(f"STDIN is not padded")
exit(-1)
with get_device().open_connection(SmartCardConnection) as connection:
piv = PivSession(connection)
for slot, metadata in list_keys(piv).items():
if metadata.public_key == public_key and metadata.key_type == key_type:
break
if metadata.public_key != public_key:
eprint(f"Specified key not available on device")
exit(-1)
eprint(f"Using slot {slot}")
if metadata.pin_policy != PIN_POLICY.NEVER:
auth_with_pin(piv)
# HACK: use internal method to skip hashing and padding
signature = piv._use_private_key(
slot=slot,
key_type=key_type,
message=padding_and_hash,
exponentiation=False
)
sys.stdout.buffer.write(signature)
if __name__ == "__main__":
parser = argparse.ArgumentParser(exit_on_error=False)
subparsers = parser.add_subparsers(title="subcommands")
sub_parser = subparsers.add_parser(
"generate-key",
help="generate a new key pair and export the public key"
)
sub_parser.add_argument(
"-S", "--slot",
help="key slot to operate on",
type=parse_slot,
required=True
)
sub_parser.add_argument(
"-A", "--algorithm",
help="algorithm of the key pair",
type=parse_key_type,
choices=SUPPORTED_KEY_TYPE,
default=KEY_TYPE.RSA2048
)
sub_parser.add_argument(
"-o", "--output",
help="path to output the public key",
type=argparse.FileType("wb"),
required=True
)
sub_parser.set_defaults(func=cmd_generate_key)
sub_parser = subparsers.add_parser(
"generate-cert",
help="generate a new certificate (NOT saved on device)"
)
sub_parser.add_argument(
"-S", "--slot",
help="key slot to operate on",
type=parse_slot,
required=True
)
sub_parser.add_argument(
"-o", "--output",
help="path to output the certificate",
type=argparse.FileType("wb"),
required=True
)
sub_parser.add_argument(
"-s", "--subject",
help="certificate subject with comma-separated components",
type=str,
default="CN=avbroot"
)
sub_parser.add_argument(
"-v", "--validity",
help="certificate validity in days",
type=int,
default=10000
)
sub_parser.set_defaults(func=cmd_generate_cert)
sub_parser = subparsers.add_parser(
"list-keys",
help="list keys in the device that can be used for signing vbmeta images or OTAs"
)
sub_parser.set_defaults(func=cmd_list_keys)
sub_parser = subparsers.add_parser(
"export-key",
help="export the public key"
)
sub_parser.add_argument(
"-S", "--slot",
help="key slot to operate on",
type=parse_slot,
required=True
)
sub_parser.add_argument(
"-o", "--output",
help="path to output the public key",
type=argparse.FileType("wb"),
required=True
)
sub_parser.set_defaults(func=cmd_export_key)
sub_parser = subparsers.add_parser(
"export-cert",
help="export the certificate"
)
sub_parser.add_argument(
"-S", "--slot",
help="key slot to operate on",
type=parse_slot,
required=True
)
sub_parser.add_argument(
"-o", "--output",
help="path to output the certificate",
type=argparse.FileType("wb"),
required=True
)
sub_parser.set_defaults(func=cmd_export_cert)
try:
args = parser.parse_args(sys.argv[1:])
except argparse.ArgumentError:
run_signing_helper()
exit(0)
try:
args.func(args)
except AttributeError:
parser.print_usage(sys.stderr)
exit(-1)
New usage: slots 0x82 ~ 0x95 are retired slots (20 in total), good enough for experiments unless you actually have old keys there
# Generate AVB stuff
$ ./script.py generate-key -S 82 -o avb.key && avbroot key extract-avb -p avb.key -o avb_pkmd.bin
# Generate OTA stuff
$ ./script.py generate-key -S 83 -o ota.key && ./script.py generate-cert -S 83 -o ota.crt
# Patching
$ avbroot ota patch -i original_OTA.zip --key-avb avb.key --key-ota ota.key --cert-ota ota.crt --signing-helper ./script.py --rootlessDid some digging on
yubikey-managersource code, the padding is actually not generated on the YubiKey, so I modified my script to skip to the internal sign function directly. Usage of such hacks in a user-supplied script sounds totally acceptable to me.
Nice! The updated script works for me too.
My personal opinion is that while AVB does use PKCS1 currently, we don't know if Google will ever switch to a non-standard padding algorithm. Also just like the conclusion above, at least for YubiKeys it only accepts pre-hashed and pre-padded data as the input.
If Google ever changes the padding, I think they'll have to give the algorithm a new name (instead of eg. SHA256_RSA4096). Using different padding for the input would result in a different signature and none of the existing bootloaders would be able to verify it.
Anyway, I'll just stick with passing pre-padded data as the input for now. If people find it too painful to use, then I can add an option like --signing-helper-input raw later.
avbtoolsupports the--signing_helperoption which allows users to use external programs for signing. Is this feasible withavbroot?The main use case here is that I would like to sign my repacks with a hardware-based key.