| Message ID | 20240523212515.4875-1-jarkko@kernel.org |
|---|---|
| Headers | show |
| Series | KEYS: asymmetric: tpm2_key_rsa | expand |
On Fri May 24, 2024 at 12:25 AM EEST, Jarkko Sakkinen wrote: > + /* > + * ABI requires this according include/crypto/akcipher.h, which says > + * that there is epilogue with algorithm OID and parameters length. > + * Neither size nor semantics is documented *anywhere*, and there's no > + * struct to hold them. > + * > + * So zeroing out the last eight bytes after the key blob seems like the > + * best bet, given no better (or any) information. The size of the > + * parameters (two u32's) was found from crypto/asymmetric/public_key.c. > + */ > + memset(work, 0, 8); This is a mystery (or nightmare). BR, Jarkko
On Fri May 24, 2024 at 12:52 AM EEST, Jarkko Sakkinen wrote: > On Fri May 24, 2024 at 12:39 AM EEST, Jarkko Sakkinen wrote: > > On Fri May 24, 2024 at 12:25 AM EEST, Jarkko Sakkinen wrote: > > > + /* > > > + * ABI requires this according include/crypto/akcipher.h, which says > > > + * that there is epilogue with algorithm OID and parameters length. > > > + * Neither size nor semantics is documented *anywhere*, and there's no > > > + * struct to hold them. > > > + * > > > + * So zeroing out the last eight bytes after the key blob seems like the > > > + * best bet, given no better (or any) information. The size of the > > > + * parameters (two u32's) was found from crypto/asymmetric/public_key.c. > > > + */ > > > + memset(work, 0, 8); > > > > This is a mystery (or nightmare). > > This is from akchiper_alg documentation: > > * @set_pub_key: Function invokes the algorithm specific set public key > * function, which knows how to decode and interpret > * the BER encoded public key and parameters > > No struct, no size information and no description what they are used for. > > Can we get these properly documented? My documentation at the moment > is grep and kprobes, literally. That said, zero issues with the interface, just pointing out the part that is not right, and should be fixed. I mean I have three layers: this, rsa-pcks1 and rsa. How I can be sure that either of two layers below never ever up until sun melts will do any changes that would break, with the data that I put there? Is this a contract that will hold forever? This is concerning so I have to point this out. BR, Jarkko
On Fri May 24, 2024 at 12:25 AM EEST, Jarkko Sakkinen wrote: > ## Overview > > Introduce tpm2_key_rsa module, which implements asymmetric TPM2 RSA key. > The feature can be enabled with the CONFIG_ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE > kconfig option. This feature allows the private key to be uploaded to > the TPM2 for signing, and software can use the public key to verify > the signatures. Since barely v6.9 is out I wrote over night also tpm2_key_ecdsa i.e. ECC/ECDSA based module :-) It was a good idea. I realized e.g. actually documented in the API fact that I should return -EBADMSG as legit undetected. Also found a memory corruption bugs. I renamed extract_pub to probe because that made me sort of realized the role better too. Some of the code could later on put to up-level struct tpm2_key but it is not a functional requirement. I.e. top-level does raw parsing and then these modules check each that if this is for them (e.g. ECDSA) then eat it. Otherwise, pass over. I did do some rudimentary testing and it seems to be quite good, and my pattern seems to work. I.e. different modules for RSA and ECDSA fit well how asymmetric keys are probed and allows to do as a sysadmin appropriate configuration for the use case. My biggest concern is undocumented parameters API in akcipher. BR, Jarkko
## Overview Introduce tpm2_key_rsa module, which implements asymmetric TPM2 RSA key. The feature can be enabled with the CONFIG_ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE kconfig option. This feature allows the private key to be uploaded to the TPM2 for signing, and software can use the public key to verify the signatures. The idea in the design is to over time to have submodule per key type For instance, tpm2_key_ecdsa could be one potential future addition in the future. Perhaps, it might sense to consider at that point also a top-level tpm2_key module. The gist is that the naming convention is free from potential future bottlencks. ### Testing tpm2_createprimary --hierarchy o -G rsa2048 -c owner.txt tpm2_evictcontrol -c owner.txt 0x81000001 tpm2_getcap handles-persistent openssl genrsa -out private.pem 2048 tpm2_import -C 0x81000001 -G rsa -i private.pem -u key.pub -r key.priv tpm2_encodeobject -C 0x81000001 -u key.pub -r key.priv -o key.priv.pem openssl asn1parse -inform pem -in key.priv.pem -noout -out key.priv.der serial=`cat key.priv.der | keyctl padd asymmetric tpm @u` echo "abcdefg" > plaintext.txt keyctl pkey_encrypt $serial 0 plaintext.txt enc=pkcs1 > encrypted.dat keyctl pkey_decrypt $serial 0 encrypted.dat enc=pkcs1 > decrypted.dat keyctl pkey_sign $serial 0 plaintext.txt enc=pkcs1 hash=sha256 > signed.dat keyctl pkey_verify $serial 0 plaintext.txt signed.dat enc=pkcs1 hash=sha256 ## References - v4: https://lore.kernel.org/linux-integrity/20240522005252.17841-1-jarkko@kernel.org/ - v3: https://lore.kernel.org/linux-integrity/20240521152659.26438-1-jarkko@kernel.org/ - v2: https://lore.kernel.org/linux-integrity/336755.1716327854@warthog.procyon.org.uk/ - v1: https://lore.kernel.org/linux-integrity/20240520184727.22038-1-jarkko@kernel.org/ - Derived from https://lore.kernel.org/all/20200518172704.29608-1-prestwoj@gmail.com/ James Prestwood (1): keys: asymmetric: ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE Jarkko Sakkinen (4): crypto: rsa-pkcs1pad: export rsa1_asn_lookup() KEYS: trusted: Change -EINVAL to -E2BIG KEYS: trusted: Move tpm2_key_decode() to the TPM driver tpm: tpm2_key: Extend parser to TPM_LoadableKey crypto/asymmetric_keys/Kconfig | 15 + crypto/asymmetric_keys/Makefile | 1 + crypto/asymmetric_keys/tpm2_key_rsa.c | 680 ++++++++++++++++++ crypto/rsa-pkcs1pad.c | 16 +- drivers/char/tpm/Kconfig | 1 + drivers/char/tpm/Makefile | 5 + drivers/char/tpm/tpm2_key.c | 118 +++ .../char/tpm}/tpm2key.asn1 | 0 include/crypto/rsa-pkcs1pad.h | 20 + include/crypto/tpm2_key.h | 35 + include/linux/tpm.h | 2 + security/keys/trusted-keys/Makefile | 2 - security/keys/trusted-keys/trusted_tpm2.c | 131 +--- 13 files changed, 908 insertions(+), 118 deletions(-) create mode 100644 crypto/asymmetric_keys/tpm2_key_rsa.c create mode 100644 drivers/char/tpm/tpm2_key.c rename {security/keys/trusted-keys => drivers/char/tpm}/tpm2key.asn1 (100%) create mode 100644 include/crypto/rsa-pkcs1pad.h create mode 100644 include/crypto/tpm2_key.h