| Message ID | 20211103231840.115521-3-quic_gaurkash@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Adds wrapped key support for inline storage encryption | expand |
On Wed, Nov 03, 2021 at 04:18:38PM -0700, Gaurav Kashyap wrote: > > However, when keys are hardware wrapped, it can be only unwrapped > by Qualcomm Trustzone. Qualcomm Trustzone is software. There is a mode where it passes the key to the actual HWKM hardware as intended, right? > +/** > + * qcom_scm_derive_sw_secret() - Derive SW secret from wrapped encryption key > + * @wrapped_key: the wrapped key used for inline encryption > + * @wrapped_key_size: size of the wrapped key > + * @sw_secret: the secret to be derived > + * @secret_size: size of the secret derived Please make the semantics of the @secret_size parameter clear. Will the output be at least @secret_size, exactly @secret_size, or at most @secret_size? > + * > + * Derive a SW secret to be used for inline encryption using Qualcomm ICE. > + * > + * Generally, for non-wrapped keys, fscrypt can derive a sw secret from the > + * key in the clear in the linux keyring. > + * > + * However, for wrapped keys, the key needs to be unwrapped, which can be done > + * only by the secure EE. So, it makes sense for the secure EE to derive the > + * sw secret and return to the kernel when wrapped keys are used. It's sort of a layering violation to mention fscrypt here, as this is low-level driver code. fscrypt is just an example user. I recommend documenting this in more general terms, and maybe referring to the "Hardware-wrapped keys" section of Documentation/block/inline-encryption.rst (which my patchset adds) as that is intended to explain derive_sw_secret already. > +int qcom_scm_derive_sw_secret(const u8* wrapped_key, u32 wrapped_key_size, > + u8 *sw_secret, u32 secret_size) > +{ > + struct qcom_scm_desc desc = { > + .svc = QCOM_SCM_SVC_ES, > + .cmd = QCOM_SCM_ES_DERIVE_RAW_SECRET, > + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RW, wrapped_key is const. Should it use QCOM_SCM_RO instead of QCOM_SCM_RW? > + keybuf = dma_alloc_coherent(__scm->dev, wrapped_key_size, &key_phys, > + GFP_KERNEL); > + if (!keybuf) > + return -ENOMEM; > + secretbuf = dma_alloc_coherent(__scm->dev, secret_size, &secret_phys, > + GFP_KERNEL); > + if (!secretbuf) > + return -ENOMEM; In the '!secretbuf' case, this leaks 'keybuf'. Also, my understanding is that the use of dma_alloc_coherent() here is a bit unusual. It would be helpful to leave a comment like: /* * Like qcom_scm_ice_set_key(), we use dma_alloc_coherent() to properly * get a physical address, while guaranteeing that we can zeroize the * key material later using memzero_explicit(). */ > + ret = qcom_scm_call(__scm->dev, &desc, NULL); > + memcpy(sw_secret, secretbuf, secret_size); This is copying out the data even if the SCM call failed. > diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h > index d92156ceb3ac..de5d4f8fd20d 100644 > --- a/drivers/firmware/qcom_scm.h > +++ b/drivers/firmware/qcom_scm.h > @@ -110,6 +110,7 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc, > #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_RAW_SECRET 0x07 Can this be renamed to DERIVE_SW_SECRET? If not, then you probably should call the function qcom_scm_derive_raw_secret() instead of qcom_scm_derive_sw_secret(), since the functions in qcom_scm.c are intended to be thin wrappers around the SCM calls. The naming difference can be dealt with at a higher level. - Eric
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index 2ee97bab7440..cf62e8056c17 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -1062,6 +1062,67 @@ int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size, } EXPORT_SYMBOL(qcom_scm_ice_set_key); +/** + * qcom_scm_derive_sw_secret() - Derive SW secret from wrapped encryption key + * @wrapped_key: the wrapped key used for inline encryption + * @wrapped_key_size: size of the wrapped key + * @sw_secret: the secret to be derived + * @secret_size: size of the secret derived + * + * Derive a SW secret to be used for inline encryption using Qualcomm ICE. + * + * Generally, for non-wrapped keys, fscrypt can derive a sw secret from the + * key in the clear in the linux keyring. + * + * However, for wrapped keys, the key needs to be unwrapped, which can be done + * only by the secure EE. So, it makes sense for the secure EE to derive the + * sw secret and return to the kernel when wrapped keys are used. + * + * Return: 0 on success; -errno on failure. + */ +int qcom_scm_derive_sw_secret(const u8* wrapped_key, u32 wrapped_key_size, + u8 *sw_secret, u32 secret_size) +{ + struct qcom_scm_desc desc = { + .svc = QCOM_SCM_SVC_ES, + .cmd = QCOM_SCM_ES_DERIVE_RAW_SECRET, + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RW, + QCOM_SCM_VAL, QCOM_SCM_RW, + QCOM_SCM_VAL), + .args[1] = wrapped_key_size, + .args[3] = secret_size, + .owner = ARM_SMCCC_OWNER_SIP, + }; + + void *keybuf, *secretbuf; + dma_addr_t key_phys, secret_phys; + int ret; + + keybuf = dma_alloc_coherent(__scm->dev, wrapped_key_size, &key_phys, + GFP_KERNEL); + if (!keybuf) + return -ENOMEM; + secretbuf = dma_alloc_coherent(__scm->dev, secret_size, &secret_phys, + GFP_KERNEL); + if (!secretbuf) + return -ENOMEM; + + memcpy(keybuf, wrapped_key, wrapped_key_size); + desc.args[0] = key_phys; + desc.args[2] = secret_phys; + + ret = qcom_scm_call(__scm->dev, &desc, NULL); + memcpy(sw_secret, secretbuf, secret_size); + + memzero_explicit(keybuf, wrapped_key_size); + dma_free_coherent(__scm->dev, wrapped_key_size, keybuf, key_phys); + memzero_explicit(secretbuf, secret_size); + dma_free_coherent(__scm->dev, secret_size, secretbuf, secret_phys); + + return ret; +} +EXPORT_SYMBOL(qcom_scm_derive_sw_secret); + /** * qcom_scm_hdcp_available() - Check if secure environment supports HDCP. * diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h index d92156ceb3ac..de5d4f8fd20d 100644 --- a/drivers/firmware/qcom_scm.h +++ b/drivers/firmware/qcom_scm.h @@ -110,6 +110,7 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc, #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_RAW_SECRET 0x07 #define QCOM_SCM_SVC_HDCP 0x11 #define QCOM_SCM_HDCP_INVOKE 0x01 diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h index c0475d1c9885..e1f645d714f9 100644 --- a/include/linux/qcom_scm.h +++ b/include/linux/qcom_scm.h @@ -103,6 +103,8 @@ extern int qcom_scm_ice_invalidate_key(u32 index); extern int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size, enum qcom_scm_ice_cipher cipher, u32 data_unit_size); +extern int qcom_scm_derive_sw_secret(const u8* wrapped_key, + u32 wrapped_key_size, u8 *sw_secret, u32 secret_size); extern bool qcom_scm_hdcp_available(void); extern int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, @@ -169,6 +171,9 @@ static inline int qcom_scm_ice_invalidate_key(u32 index) { return -ENODEV; } static inline int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size, enum qcom_scm_ice_cipher cipher, u32 data_unit_size) { return -ENODEV; } +static inline int qcom_scm_derive_sw_secret(const u8* wrapped_key, + u32 wrapped_key_size, u8 *sw_secret, + u32 secret_size) { return -ENODEV; } static inline bool qcom_scm_hdcp_available(void) { return false; } static inline int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
Storage encryption requires fscrypt deriving a sw secret from the keys inserted into the linux keyring. For non-wrapped keys, this can be directly done as keys are in the clear. However, when keys are hardware wrapped, it can be only unwrapped by Qualcomm Trustzone. Hence, it also makes sense that the software secret is also derived there and returned to the linux kernel for wrapped keys. Fscrypt invokes this functionality using the crypto profile APIs provided by the block layer. Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com> --- drivers/firmware/qcom_scm.c | 61 +++++++++++++++++++++++++++++++++++++ drivers/firmware/qcom_scm.h | 1 + include/linux/qcom_scm.h | 5 +++ 3 files changed, 67 insertions(+)