| Message ID | 20240127232436.2632187-1-quic_gaurkash@quicinc.com |
|---|---|
| Headers | show |
| Series | Hardware wrapped key support for qcom ice and ufs | expand |
On 28/01/2024 00:14, Gaurav Kashyap wrote: > The Inline Crypto Engine (ICE) for UFS/EMMC supports the > Hardware Key Manager (HWKM) to securely manage storage > keys. Enable using this hardware on sm8650. > > This requires two changes: > 1. Register size increase: HWKM is an additional piece of hardware > sitting alongside ICE, and extends the old ICE's register space. > 2. Explicitly tell the ICE driver to use HWKM with ICE so that > wrapped keys are used in sm8650. > > Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com> > Reviewed-by: Om Prakash Singh <quic_omprsing@quicinc.com> > Tested-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > arch/arm64/boot/dts/qcom/sm8650.dtsi | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi > index 2df77123a8c7..c27daf576af5 100644 > --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi > @@ -2524,7 +2524,8 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, > ice: crypto@1d88000 { > compatible = "qcom,sm8650-inline-crypto-engine", > "qcom,inline-crypto-engine"; > - reg = <0 0x01d88000 0 0x8000>; > + reg = <0 0x01d88000 0 0x10000>; > + qcom,ice-use-hwkm; Vendor properties go to the end. Please consult DTS coding style. Best regards, Krzysztof
On 28/01/2024 00:14, Gaurav Kashyap wrote: > When Qualcomm's Inline Crypto Engine (ICE) contains Hardware > Key Manager (HWKM), and the 'HWKM' mode is enabled, it > supports wrapped keys. However, this also requires firmware > support in Trustzone to work correctly, which may not be available > on all chipsets. In the above scenario, ICE needs to support standard > keys even though HWKM is integrated from a hardware perspective. > > Introducing this property so that Hardware wrapped key support > can be enabled/disabled from software based on chipset firmware, > and not just based on hardware version. > > Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com> > Tested-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > .../bindings/crypto/qcom,inline-crypto-engine.yaml | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml > index 09e43157cc71..6415d7be9b73 100644 > --- a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml > +++ b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml > @@ -25,6 +25,16 @@ properties: > clocks: > maxItems: 1 > > + qcom,ice-use-hwkm: > + type: boolean > + description: > + Use the supported Hardware Key Manager (HWKM) in Qualcomm ICE > + to support wrapped keys. Having this entry helps scenarios where > + the ICE hardware supports HWKM, but the Trustzone firmware does > + not have the full capability to use this HWKM and support wrapped How does it help in this scenario? You enable this property, Trustzone does not support it, so what happens? Also, which SoCs have incomplete Trustzone support? I expect this to be a quirk, thus limited to specific SoCs with issues. Best regards, Krzysztof
On Sat, Jan 27, 2024 at 03:14:00PM -0800, Gaurav Kashyap wrote: The subject prefix does not match other changes to this file. > Inline storage encryption may require deriving a software > secret from storage keys added to the kernel. > > For non-wrapped keys, this can be directly done in the kernel as > keys are in the clear. > > However, hardware wrapped keys can only be unwrapped by the wrapping > entity. In case of Qualcomm's wrapped key solution, this is done by > the Hardware Key Manager (HWKM) from Trustzone. > Hence, adding a new SCM call which in the end provides a hook > to the software secret crypto profile API provided by the block > layer. > > Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com> > Tested-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > drivers/firmware/qcom/qcom_scm.c | 65 ++++++++++++++++++++++++++ > drivers/firmware/qcom/qcom_scm.h | 1 + > include/linux/firmware/qcom/qcom_scm.h | 2 + > 3 files changed, 68 insertions(+) > > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c > index 7e17fd662bda..4882f8a36453 100644 > --- a/drivers/firmware/qcom/qcom_scm.c > +++ b/drivers/firmware/qcom/qcom_scm.c > @@ -1220,6 +1220,71 @@ int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size, > } > EXPORT_SYMBOL_GPL(qcom_scm_ice_set_key); > > +/** > + * qcom_scm_derive_sw_secret() - Derive software secret from wrapped key > + * @wkey: the hardware wrapped key inaccessible to software > + * @wkey_size: size of the wrapped key > + * @sw_secret: the secret to be derived which is exactly the secret size > + * @sw_secret_size: size of the sw_secret > + * > + * Derive a software secret from a hardware wrapped key for software crypto > + * operations. > + * For wrapped keys, the key needs to be unwrapped, in order to derive a > + * software secret, which can be done in the hardware from a secure execution > + * environment. > + * > + * For more information on sw secret, please refer to "Hardware-wrapped keys" > + * section of Documentation/block/inline-encryption.rst. > + * > + * Return: 0 on success; -errno on failure. > + */ > +int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size, > + u8 *sw_secret, size_t sw_secret_size) > +{ > + struct qcom_scm_desc desc = { > + .svc = QCOM_SCM_SVC_ES, > + .cmd = QCOM_SCM_ES_DERIVE_SW_SECRET, > + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RW, > + QCOM_SCM_VAL, QCOM_SCM_RW, > + QCOM_SCM_VAL), > + .args[1] = wkey_size, > + .args[3] = sw_secret_size, > + .owner = ARM_SMCCC_OWNER_SIP, > + }; > + > + void *secret_buf; > + void *wkey_buf; > + int ret; > + > + wkey_buf = qcom_tzmem_alloc(__scm->mempool, wkey_size, GFP_KERNEL); > + if (!wkey_buf) > + return -ENOMEM; > + > + secret_buf = qcom_tzmem_alloc(__scm->mempool, sw_secret_size, GFP_KERNEL); > + if (!secret_buf) { > + ret = -ENOMEM; > + goto err_free_wrapped; > + } > + > + memcpy(wkey_buf, wkey, wkey_size); > + desc.args[0] = qcom_tzmem_to_phys(wkey_buf); > + desc.args[2] = qcom_tzmem_to_phys(secret_buf); > + > + ret = qcom_scm_call(__scm->dev, &desc, NULL); > + if (!ret) > + memcpy(sw_secret, secret_buf, sw_secret_size); > + > + memzero_explicit(secret_buf, sw_secret_size); > + qcom_tzmem_free(secret_buf); > + > +err_free_wrapped: This code path is shared between error path and normal path, prefixing it "err_" is not helpful to the reader. Please change this to out_free_wrapped: The rest of the patch looks good to me. Regards, Bjorn > + memzero_explicit(wkey_buf, wkey_size); > + qcom_tzmem_free(wkey_buf); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(qcom_scm_derive_sw_secret); > + > /** > * qcom_scm_hdcp_available() - Check if secure environment supports HDCP. > * > diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h > index cb7273aa0a5e..56ff0806f5d2 100644 > --- a/drivers/firmware/qcom/qcom_scm.h > +++ b/drivers/firmware/qcom/qcom_scm.h > @@ -127,6 +127,7 @@ struct qcom_tzmem_pool *qcom_scm_get_tzmem_pool(void); > #define QCOM_SCM_SVC_ES 0x10 /* Enterprise Security */ > #define QCOM_SCM_ES_INVALIDATE_ICE_KEY 0x03 > #define QCOM_SCM_ES_CONFIG_SET_ICE_KEY 0x04 > +#define QCOM_SCM_ES_DERIVE_SW_SECRET 0x07 > > #define QCOM_SCM_SVC_HDCP 0x11 > #define QCOM_SCM_HDCP_INVOKE 0x01 > diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h > index 9b6054813f59..89358478ac67 100644 > --- a/include/linux/firmware/qcom/qcom_scm.h > +++ b/include/linux/firmware/qcom/qcom_scm.h > @@ -103,6 +103,8 @@ bool qcom_scm_ice_available(void); > int qcom_scm_ice_invalidate_key(u32 index); > int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size, > enum qcom_scm_ice_cipher cipher, u32 data_unit_size); > +int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size, > + u8 *sw_secret, size_t sw_secret_size); > > bool qcom_scm_hdcp_available(void); > int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp); > -- > 2.43.0 >
On 28.01.2024 00:14, Gaurav Kashyap wrote: > Inline storage encryption may require deriving a software > secret from storage keys added to the kernel. > > For non-wrapped keys, this can be directly done in the kernel as > keys are in the clear. > > However, hardware wrapped keys can only be unwrapped by the wrapping > entity. In case of Qualcomm's wrapped key solution, this is done by > the Hardware Key Manager (HWKM) from Trustzone. > Hence, adding a new SCM call which in the end provides a hook > to the software secret crypto profile API provided by the block > layer. > > Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com> > Tested-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > drivers/firmware/qcom/qcom_scm.c | 65 ++++++++++++++++++++++++++ > drivers/firmware/qcom/qcom_scm.h | 1 + > include/linux/firmware/qcom/qcom_scm.h | 2 + > 3 files changed, 68 insertions(+) > > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c > index 7e17fd662bda..4882f8a36453 100644 > --- a/drivers/firmware/qcom/qcom_scm.c > +++ b/drivers/firmware/qcom/qcom_scm.c > @@ -1220,6 +1220,71 @@ int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size, > } > EXPORT_SYMBOL_GPL(qcom_scm_ice_set_key); > > +/** > + * qcom_scm_derive_sw_secret() - Derive software secret from wrapped key > + * @wkey: the hardware wrapped key inaccessible to software > + * @wkey_size: size of the wrapped key > + * @sw_secret: the secret to be derived which is exactly the secret size > + * @sw_secret_size: size of the sw_secret > + * > + * Derive a software secret from a hardware wrapped key for software crypto > + * operations. > + * For wrapped keys, the key needs to be unwrapped, in order to derive a > + * software secret, which can be done in the hardware from a secure execution > + * environment. > + * > + * For more information on sw secret, please refer to "Hardware-wrapped keys" > + * section of Documentation/block/inline-encryption.rst. > + * > + * Return: 0 on success; -errno on failure. > + */ > +int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size, > + u8 *sw_secret, size_t sw_secret_size) > +{ > + struct qcom_scm_desc desc = { > + .svc = QCOM_SCM_SVC_ES, > + .cmd = QCOM_SCM_ES_DERIVE_SW_SECRET, > + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RW, > + QCOM_SCM_VAL, QCOM_SCM_RW, > + QCOM_SCM_VAL), > + .args[1] = wkey_size, > + .args[3] = sw_secret_size, > + .owner = ARM_SMCCC_OWNER_SIP, > + }; > + > + void *secret_buf; > + void *wkey_buf; > + int ret; > + > + wkey_buf = qcom_tzmem_alloc(__scm->mempool, wkey_size, GFP_KERNEL); > + if (!wkey_buf) > + return -ENOMEM; > + > + secret_buf = qcom_tzmem_alloc(__scm->mempool, sw_secret_size, GFP_KERNEL); > + if (!secret_buf) { > + ret = -ENOMEM; > + goto err_free_wrapped; > + } > + > + memcpy(wkey_buf, wkey, wkey_size); > + desc.args[0] = qcom_tzmem_to_phys(wkey_buf); > + desc.args[2] = qcom_tzmem_to_phys(secret_buf); > + > + ret = qcom_scm_call(__scm->dev, &desc, NULL); > + if (!ret) > + memcpy(sw_secret, secret_buf, sw_secret_size); > + > + memzero_explicit(secret_buf, sw_secret_size); > + qcom_tzmem_free(secret_buf); > + > +err_free_wrapped: > + memzero_explicit(wkey_buf, wkey_size); > + qcom_tzmem_free(wkey_buf); __free(qcom_tzmem) attribute instead? Konrad
On 29.01.2024 09:18, Krzysztof Kozlowski wrote: > On 28/01/2024 00:14, Gaurav Kashyap wrote: >> When Qualcomm's Inline Crypto Engine (ICE) contains Hardware >> Key Manager (HWKM), and the 'HWKM' mode is enabled, it >> supports wrapped keys. However, this also requires firmware >> support in Trustzone to work correctly, which may not be available >> on all chipsets. In the above scenario, ICE needs to support standard >> keys even though HWKM is integrated from a hardware perspective. >> >> Introducing this property so that Hardware wrapped key support >> can be enabled/disabled from software based on chipset firmware, >> and not just based on hardware version. >> >> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com> >> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> .../bindings/crypto/qcom,inline-crypto-engine.yaml | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml >> index 09e43157cc71..6415d7be9b73 100644 >> --- a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml >> +++ b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml >> @@ -25,6 +25,16 @@ properties: >> clocks: >> maxItems: 1 >> >> + qcom,ice-use-hwkm: >> + type: boolean >> + description: >> + Use the supported Hardware Key Manager (HWKM) in Qualcomm ICE >> + to support wrapped keys. Having this entry helps scenarios where >> + the ICE hardware supports HWKM, but the Trustzone firmware does >> + not have the full capability to use this HWKM and support wrapped > > How does it help in this scenario? You enable this property, Trustzone > does not support it, so what happens? > > Also, which SoCs have incomplete Trustzone support? I expect this to be > a quirk, thus limited to specific SoCs with issues. Can we simply evaluate the return value of the secure calls? Konrad
On Thu, 1 Feb 2024 at 17:11, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > On 28.01.2024 00:14, Gaurav Kashyap wrote: > > Inline storage encryption may require deriving a software > > secret from storage keys added to the kernel. > > > > For non-wrapped keys, this can be directly done in the kernel as > > keys are in the clear. > > > > However, hardware wrapped keys can only be unwrapped by the wrapping > > entity. In case of Qualcomm's wrapped key solution, this is done by > > the Hardware Key Manager (HWKM) from Trustzone. > > Hence, adding a new SCM call which in the end provides a hook > > to the software secret crypto profile API provided by the block > > layer. > > > > Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com> > > Tested-by: Neil Armstrong <neil.armstrong@linaro.org> > > --- > > drivers/firmware/qcom/qcom_scm.c | 65 ++++++++++++++++++++++++++ > > drivers/firmware/qcom/qcom_scm.h | 1 + > > include/linux/firmware/qcom/qcom_scm.h | 2 + > > 3 files changed, 68 insertions(+) > > > > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c > > index 7e17fd662bda..4882f8a36453 100644 > > --- a/drivers/firmware/qcom/qcom_scm.c > > +++ b/drivers/firmware/qcom/qcom_scm.c > > @@ -1220,6 +1220,71 @@ int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size, > > } > > EXPORT_SYMBOL_GPL(qcom_scm_ice_set_key); > > > > +/** > > + * qcom_scm_derive_sw_secret() - Derive software secret from wrapped key > > + * @wkey: the hardware wrapped key inaccessible to software > > + * @wkey_size: size of the wrapped key > > + * @sw_secret: the secret to be derived which is exactly the secret size > > + * @sw_secret_size: size of the sw_secret > > + * > > + * Derive a software secret from a hardware wrapped key for software crypto > > + * operations. > > + * For wrapped keys, the key needs to be unwrapped, in order to derive a > > + * software secret, which can be done in the hardware from a secure execution > > + * environment. > > + * > > + * For more information on sw secret, please refer to "Hardware-wrapped keys" > > + * section of Documentation/block/inline-encryption.rst. > > + * > > + * Return: 0 on success; -errno on failure. > > + */ > > +int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size, > > + u8 *sw_secret, size_t sw_secret_size) > > +{ > > + struct qcom_scm_desc desc = { > > + .svc = QCOM_SCM_SVC_ES, > > + .cmd = QCOM_SCM_ES_DERIVE_SW_SECRET, > > + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RW, > > + QCOM_SCM_VAL, QCOM_SCM_RW, > > + QCOM_SCM_VAL), > > + .args[1] = wkey_size, > > + .args[3] = sw_secret_size, > > + .owner = ARM_SMCCC_OWNER_SIP, > > + }; > > + > > + void *secret_buf; > > + void *wkey_buf; > > + int ret; > > + > > + wkey_buf = qcom_tzmem_alloc(__scm->mempool, wkey_size, GFP_KERNEL); > > + if (!wkey_buf) > > + return -ENOMEM; > > + > > + secret_buf = qcom_tzmem_alloc(__scm->mempool, sw_secret_size, GFP_KERNEL); > > + if (!secret_buf) { > > + ret = -ENOMEM; > > + goto err_free_wrapped; > > + } > > + > > + memcpy(wkey_buf, wkey, wkey_size); > > + desc.args[0] = qcom_tzmem_to_phys(wkey_buf); > > + desc.args[2] = qcom_tzmem_to_phys(secret_buf); > > + > > + ret = qcom_scm_call(__scm->dev, &desc, NULL); > > + if (!ret) > > + memcpy(sw_secret, secret_buf, sw_secret_size); > > + > > + memzero_explicit(secret_buf, sw_secret_size); > > + qcom_tzmem_free(secret_buf); > > + > > +err_free_wrapped: > > + memzero_explicit(wkey_buf, wkey_size); > > + qcom_tzmem_free(wkey_buf); > __free(qcom_tzmem) attribute instead? > I second this. Please look at other implementations. Bart > Konrad >
Hello Konrad and Krzysztof On 02/01/2024 11:14, Konrad Dybcio wrote > On 29.01.2024 09:18, Krzysztof Kozlowski wrote: > > On 28/01/2024 00:14, Gaurav Kashyap wrote: > >> When Qualcomm's Inline Crypto Engine (ICE) contains Hardware Key > >> Manager (HWKM), and the 'HWKM' mode is enabled, it supports wrapped > >> keys. However, this also requires firmware support in Trustzone to > >> work correctly, which may not be available on all chipsets. In the > >> above scenario, ICE needs to support standard keys even though HWKM > >> is integrated from a hardware perspective. > >> > >> Introducing this property so that Hardware wrapped key support can be > >> enabled/disabled from software based on chipset firmware, and not > >> just based on hardware version. > >> > >> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com> > >> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> > >> --- > >> .../bindings/crypto/qcom,inline-crypto-engine.yaml | 10 ++++++++++ > >> 1 file changed, 10 insertions(+) > >> > >> diff --git > >> a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto- > engine. > >> yaml > >> b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto- > engine. > >> yaml index 09e43157cc71..6415d7be9b73 100644 > >> --- > >> a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto- > engine. > >> yaml > >> +++ b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto- > eng > >> +++ ine.yaml > >> @@ -25,6 +25,16 @@ properties: > >> clocks: > >> maxItems: 1 > >> > >> + qcom,ice-use-hwkm: > >> + type: boolean > >> + description: > >> + Use the supported Hardware Key Manager (HWKM) in Qualcomm ICE > >> + to support wrapped keys. Having this entry helps scenarios where > >> + the ICE hardware supports HWKM, but the Trustzone firmware does > >> + not have the full capability to use this HWKM and support > >> + wrapped > > > > How does it help in this scenario? You enable this property, Trustzone > > does not support it, so what happens? > > > > Also, which SoCs have incomplete Trustzone support? I expect this to > > be a quirk, thus limited to specific SoCs with issues. Apologies for not addressing this earlier, we can perhaps continue this discussion in the new patch thread. I will link to this there. SM8450 and SM8350 QCOM ICE both support HWKM in their ICE hardware. However, wrapped keys can not be enabled on those targets due to certain missing trustzone support. If we solely rely on hardware version to decide if ICE has to use wrapped keys for data encryption, then it becomes untestable on those chipsets. So, we want another way to distinguish this scenario, and hence I chose a DT vendor property to explicitly mention if we have to use the supported HWKM. If there is another way, I am open to exploring that as well. > > Can we simply evaluate the return value of the secure calls? > This might not work as UFS crypto needs this information much earlier, and based on that , it would need to register with the keyslot manager (and block crypto), on whether wrapped keys are supported. https://lore.kernel.org/all/20231104211259.17448-2-ebiggers@kernel.org/ > Konrad