From ab1851e9f2c40942fff243504788795aeaf89961 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Tue, 14 Dec 2021 14:32:51 +0000 Subject: [PATCH] Alter spec text for RSA-PSS to match reality The Key{Mint,Master} spec previously said that RSA-PSS mode should use SHA-1 for the MGF1 digest, separately from whatever Tag::DIGEST gets specified as the main digest. However, both the reference implementation and the VTS/CTS tests use BoringSSL's defaults, which is to re-use the main digest as the MGF1 digest if none is separately specified. Given that this behaviour is embedded in many implementations over several years (and given that there isn't a security implication), change the spec to match this behaviour. Also update the VTS test code to make this clear/obvious. Test: VtsAidlKeyMintTargetTest, VtsHalKeymasterV4_0TargetTest Bug: 210424594 Change-Id: I4303f28d094ef4d4b9dc931d6728b1fa040de20d Ignore-AOSP-First: target internal master first due to merge conflict --- current.txt | 1 + keymaster/4.0/IKeymasterDevice.hal | 3 ++- keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp | 1 + .../android/hardware/security/keymint/IKeyMintOperation.aidl | 3 ++- security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp | 1 + 5 files changed, 7 insertions(+), 2 deletions(-) diff --git a/current.txt b/current.txt index 1dbf5ab1be..1fedaa0d6e 100644 --- a/current.txt +++ b/current.txt @@ -907,6 +907,7 @@ c8a57364f6ad20842be14f4db284df5304f7521ca8eac6bcc1fa6c5b466fb8a6 android.hardwar # ABI preserving changes to HALs during Android T 62ace52d9c3ff1f60f94118557a2aaf0b953513e59dcd34d5f94ae28d4c7e780 android.hardware.fastboot@1.0::IFastboot f767a132ef28275294db15353f14f3876a4048770751931a77d038d4228f2cb7 android.hardware.graphics.composer@2.4::IComposerClient +d0fb32f3ddeb9af7115ab32905225ea69b930d2472be8e9610f0cf136c15aefb android.hardware.keymaster@4.0::IKeymasterDevice # b/210424594 ca62a2a95d173ed323309e5e00f653ad3cceec82a6e5e4976a249cb5aafe2515 android.hardware.neuralnetworks@1.2::types fa76bced6b1b71c40fc706c508a9011284c57f57831cd0cf5f45653ed4ea463e android.hardware.neuralnetworks@1.3::types diff --git a/keymaster/4.0/IKeymasterDevice.hal b/keymaster/4.0/IKeymasterDevice.hal index dfde060e3f..1c6ae478f4 100644 --- a/keymaster/4.0/IKeymasterDevice.hal +++ b/keymaster/4.0/IKeymasterDevice.hal @@ -1254,7 +1254,8 @@ interface IKeymasterDevice { * o PaddingMode::RSA_PSS. For PSS-padded signature operations, the PSS salt length must match * the size of the PSS digest selected. The digest specified with Tag::DIGEST in inputParams * on begin() must be used as the PSS digest algorithm, MGF1 must be used as the mask - * generation function and SHA1 must be used as the MGF1 digest algorithm. + * generation function and the digest specified with Tag:DIGEST in inputParams must also be + * used as the MGF1 digest algorithm. * * o PaddingMode::RSA_OAEP. The digest specified with Tag::DIGEST in inputParams on begin is * used as the OAEP digest algorithm, MGF1 must be used as the mask generation function and diff --git a/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp b/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp index 767614754b..6412f3a06b 100644 --- a/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp +++ b/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp @@ -1712,6 +1712,7 @@ TEST_P(VerificationOperationsTest, RsaAllPaddingsAndDigests) { case PaddingMode::RSA_PSS: EXPECT_GT(EVP_PKEY_CTX_set_rsa_padding(pkey_ctx, RSA_PKCS1_PSS_PADDING), 0); EXPECT_GT(EVP_PKEY_CTX_set_rsa_pss_saltlen(pkey_ctx, EVP_MD_size(md)), 0); + EXPECT_GT(EVP_PKEY_CTX_set_rsa_mgf1_md(pkey_ctx, md), 0); break; case PaddingMode::RSA_PKCS1_1_5_SIGN: // PKCS1 is the default; don't need to set anything. diff --git a/security/keymint/aidl/android/hardware/security/keymint/IKeyMintOperation.aidl b/security/keymint/aidl/android/hardware/security/keymint/IKeyMintOperation.aidl index ce83044d0e..ca8955552e 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/IKeyMintOperation.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/IKeyMintOperation.aidl @@ -227,7 +227,8 @@ interface IKeyMintOperation { * o PaddingMode::RSA_PSS. For PSS-padded signature operations, the PSS salt length must match * the size of the PSS digest selected. The digest specified with Tag::DIGEST in params * on begin() must be used as the PSS digest algorithm, MGF1 must be used as the mask - * generation function and SHA1 must be used as the MGF1 digest algorithm. + * generation function and the digest specified with Tag:DIGEST in params on begin() must also + * be used as the MGF1 digest algorithm. * * -- ECDSA keys -- * diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index 02462fce3a..374f2da7a8 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -812,6 +812,7 @@ void KeyMintAidlTestBase::LocalVerifyMessage(const string& message, const string if (padding == PaddingMode::RSA_PSS) { EXPECT_GT(EVP_PKEY_CTX_set_rsa_padding(pkey_ctx, RSA_PKCS1_PSS_PADDING), 0); EXPECT_GT(EVP_PKEY_CTX_set_rsa_pss_saltlen(pkey_ctx, EVP_MD_size(md)), 0); + EXPECT_GT(EVP_PKEY_CTX_set_rsa_mgf1_md(pkey_ctx, md), 0); } ASSERT_EQ(1, EVP_DigestVerifyUpdate(&digest_ctx,