| Message ID | 1522049540-10042-3-git-send-email-gilad@benyossef.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
On Mon, Mar 26, 2018 at 08:32:19AM +0100, Gilad Ben-Yossef wrote: > Enable CryptoCell support for hardware keys. > > Hardware keys are regular AES keys loaded into CryptoCell internal memory > via firmware, often from secure boot ROM or hardware fuses at boot time. > > As such, they can be used for enc/dec purposes like any other key but > cannot (read: extremely hard to) be extracted since since they are not > available anywhere in RAM during runtime. > > The mechanism has some similarities to s390 secure keys although the keys > are not wrapped or sealed, but simply loaded offline. The interface was > therefore modeled based on the s390 secure keys support. > > Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com> ... > static const struct cc_alg_template skcipher_algs[] = { > { > + .name = "xts(haes)", > + .driver_name = "xts-haes-ccree", > + .blocksize = AES_BLOCK_SIZE, > + .template_skcipher = { > + .setkey = cc_cipher_sethkey, > + .encrypt = cc_cipher_encrypt, > + .decrypt = cc_cipher_decrypt, > + .min_keysize = CC_HW_KEY_SIZE, > + .max_keysize = CC_HW_KEY_SIZE, > + .ivsize = AES_BLOCK_SIZE, > + }, > + .cipher_mode = DRV_CIPHER_XTS, > + .flow_mode = S_DIN_to_AES, > + .min_hw_rev = CC_HW_REV_630, > + }, How can this possibly pass the self-test? If we want to add hardware keys we will need to figure out how to deal with it in the top-level API first. Are there other crypto drivers doing this? Thanks,
On Fri, Mar 30, 2018 at 8:26 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote: > On Mon, Mar 26, 2018 at 08:32:19AM +0100, Gilad Ben-Yossef wrote: >> Enable CryptoCell support for hardware keys. >> >> Hardware keys are regular AES keys loaded into CryptoCell internal memory >> via firmware, often from secure boot ROM or hardware fuses at boot time. >> >> As such, they can be used for enc/dec purposes like any other key but >> cannot (read: extremely hard to) be extracted since since they are not >> available anywhere in RAM during runtime. >> >> The mechanism has some similarities to s390 secure keys although the keys >> are not wrapped or sealed, but simply loaded offline. The interface was >> therefore modeled based on the s390 secure keys support. >> >> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com> > > ... > >> static const struct cc_alg_template skcipher_algs[] = { >> { >> + .name = "xts(haes)", >> + .driver_name = "xts-haes-ccree", >> + .blocksize = AES_BLOCK_SIZE, >> + .template_skcipher = { >> + .setkey = cc_cipher_sethkey, >> + .encrypt = cc_cipher_encrypt, >> + .decrypt = cc_cipher_decrypt, >> + .min_keysize = CC_HW_KEY_SIZE, >> + .max_keysize = CC_HW_KEY_SIZE, >> + .ivsize = AES_BLOCK_SIZE, >> + }, >> + .cipher_mode = DRV_CIPHER_XTS, >> + .flow_mode = S_DIN_to_AES, >> + .min_hw_rev = CC_HW_REV_630, >> + }, > > How can this possibly pass the self-test? Indeed, I could not figure a way to test the mechanism directly. However, as it uses the exact same mechanism of the regular xts-aes-ccree but takes the key from another source, I've marked it with a test of alg_test_null() on the premise that if the xts-aes-ccree works, so must this. > > If we want to add hardware keys we will need to figure out how > to deal with it in the top-level API first. > > Are there other crypto drivers doing this? I thought the exact same thing until I ran into a presentation about the s390 secure keys implementation. I basically imitated their use (or abuse?) of the Crypto API assuming it is the way to go. Take a look at arch/s390/crypto/paes_s390.c The slide for the presentation describing this is here: http://schd.ws/hosted_files/ossna2017/89/LC2017SecKeyDmCryptV5.pdf And they seem to even have support for it in the DM-Crypt tools, which at the time they claimed to be in the process of getting it up-streamed. Anyway, if this is not the way to go I'd be more than happy to do whatever work is needed to create the right interface. PS. I'd be out of the office and away from email access to the coming week, so kindly excuse any delay in response. Thanks! Gilad
On Sat, Mar 31, 2018 at 08:30:46PM +0300, Gilad Ben-Yossef wrote: > > However, as it uses the exact same mechanism of the regular xts-aes-ccree > but takes the key from another source, I've marked it with a test of > alg_test_null() on the premise that if the xts-aes-ccree works, so must this. Well it appears to be stubbed out as cc_is_hw_key always returns false. > > Are there other crypto drivers doing this? > > I thought the exact same thing until I ran into a presentation about the s390 > secure keys implementation. I basically imitated their use (or abuse?) > of the Crypto API > assuming it is the way to go. > > Take a look at arch/s390/crypto/paes_s390.c It's always nice to discover code that was never reviewed. The general approach seems sane though. > Anyway, if this is not the way to go I'd be more than happy to do whatever > work is needed to create the right interface. > > PS. I'd be out of the office and away from email access to the coming week, so > kindly excuse any delay in response. I think it's fine. However, I don't really like the idea of everyone coming up with their own "paes", i.e., your patch's use of "haes". It should be OK to just use paes for everyone, no? As to your patch specifically, there is one issue where you're directly dereferencing the key as a struct. This is a no-no because the key may have come from user-space. You must treat it as a binary blob. The s390 code seems to do this correctly. Cheers,
On 03/31/2018 07:30 PM, Gilad Ben-Yossef wrote: ... >> Are there other crypto drivers doing this? > > I thought the exact same thing until I ran into a presentation about the s390 > secure keys implementation. I basically imitated their use (or abuse?) > of the Crypto API > assuming it is the way to go. > > Take a look at arch/s390/crypto/paes_s390.c > > The slide for the presentation describing this is here: > http://schd.ws/hosted_files/ossna2017/89/LC2017SecKeyDmCryptV5.pdf > > And they seem to even have support for it in the DM-Crypt tools, which at > the time they claimed to be in the process of getting it up-streamed. It is "in the process", but definitely not accepted. We are just discussing how to integrate paes wrapped keys in cryptsetup and it will definitely not be the way presented in the slides above. If you plan more such ciphers, I would welcome some unified way in crypto API how to handle these HSM keys flavors. For kernel dm-crypt, there is no change needed (dmcrypt just treats it as a normal cipher key). (I would say that it is not the best idea either, IMHO it would be better to use kernel keyring reference instead and somehow handle hw keys through keyring.) Milan
On Tue, Apr 3, 2018 at 1:19 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote: > On Sat, Mar 31, 2018 at 08:30:46PM +0300, Gilad Ben-Yossef wrote: >> >> However, as it uses the exact same mechanism of the regular xts-aes-ccree >> but takes the key from another source, I've marked it with a test of >> alg_test_null() on the premise that if the xts-aes-ccree works, so must this. > > Well it appears to be stubbed out as cc_is_hw_key always returns > false. Please look again. The stub version of cc_is_hw_key() doing that is being replaced in this patch. >> > Are there other crypto drivers doing this? >> >> I thought the exact same thing until I ran into a presentation about the s390 >> secure keys implementation. I basically imitated their use (or abuse?) >> of the Crypto API >> assuming it is the way to go. >> >> Take a look at arch/s390/crypto/paes_s390.c > > It's always nice to discover code that was never reviewed. :-) > > The general approach seems sane though. > >> Anyway, if this is not the way to go I'd be more than happy to do whatever >> work is needed to create the right interface. >> >> PS. I'd be out of the office and away from email access to the coming week, so >> kindly excuse any delay in response. > > I think it's fine. However, I don't really like the idea of everyone > coming up with their own "paes", i.e., your patch's use of "haes". > It should be OK to just use paes for everyone, no? The s390 key and the cryptocell keys are not the same: Their is, I believe, is an AES key encrypted by some internal key/algorithm. The cryptocell "key" is a token, which is internally comprised of one or two indexes, referencing slots in the internal memory in the hardware, and a key size, that describe the size of the key. I thought it would be confusing to use "paes" to describe both, since they are not interchangeable. You would not be able to feed an paes key that works with the s390 version to cryptocell and vice verse and get it work. Having said, if you prefer to have "paes" simply designate "implementation specific token for an AES key" I'm perfectly fine with that. > > As to your patch specifically, there is one issue where you're > directly dereferencing the key as a struct. This is a no-no because > the key may have come from user-space. You must treat it as a > binary blob. The s390 code seems to do this correctly. > As noted above, the haes "key" is really a token encoding 3 different pieces of information: 1. The index of the first key slot to use 2. Potentially, the index of a 2nd key slot to use (for XTS) 3. a key size, denoting the actual key size used, not the slot number I than have to parse this information encoded in the token and feed it to the HW (via 3 different registers) Therefore, I do not see how I can avoid parsing the token. I hope I've managed to explain things better now. Thanks, Gilad
On Tue, Apr 3, 2018 at 3:22 PM, Milan Broz <gmazyland@gmail.com> wrote: > On 03/31/2018 07:30 PM, Gilad Ben-Yossef wrote: > ... >>> Are there other crypto drivers doing this? >> >> I thought the exact same thing until I ran into a presentation about the s390 >> secure keys implementation. I basically imitated their use (or abuse?) >> of the Crypto API >> assuming it is the way to go. >> >> Take a look at arch/s390/crypto/paes_s390.c >> >> The slide for the presentation describing this is here: >> http://schd.ws/hosted_files/ossna2017/89/LC2017SecKeyDmCryptV5.pdf >> >> And they seem to even have support for it in the DM-Crypt tools, which at >> the time they claimed to be in the process of getting it up-streamed. > > It is "in the process", but definitely not accepted. > > We are just discussing how to integrate paes wrapped keys in cryptsetup and > it will definitely not be the way presented in the slides above. > > If you plan more such ciphers, I would welcome some unified way in crypto API > how to handle these HSM keys flavors. That would be good. Note however the fine difference - the s390 usage is a wrapped key. Ours is a token for a key (a slot number really). Probably makes no difference for any practical sense, but I thought it is worth mentioning it. > > For kernel dm-crypt, there is no change needed (dmcrypt just treats it as a normal cipher key). > (I would say that it is not the best idea either, IMHO it would be better to use > kernel keyring reference instead and somehow handle hw keys through keyring.) > I am all for the keyring approach. In fact, that was the way I wanted to to go to introduce this feature for cryptocell when I discovered that was already upstream code using a different approach. Any suggestion how this would work vis a vis the crypto API usage? e.g. - have a parallel setkey variant added to crypto APi that takes a kernel keyring object rather than actual key? Thanks, Gilad
On 04/03/2018 12:19 PM, Herbert Xu wrote: > On Sat, Mar 31, 2018 at 08:30:46PM +0300, Gilad Ben-Yossef wrote: >> However, as it uses the exact same mechanism of the regular xts-aes-ccree >> but takes the key from another source, I've marked it with a test of >> alg_test_null() on the premise that if the xts-aes-ccree works, so must this. > Well it appears to be stubbed out as cc_is_hw_key always returns > false. > >>> Are there other crypto drivers doing this? >> I thought the exact same thing until I ran into a presentation about the s390 >> secure keys implementation. I basically imitated their use (or abuse?) >> of the Crypto API >> assuming it is the way to go. >> >> Take a look at arch/s390/crypto/paes_s390.c > It's always nice to discover code that was never reviewed. > > The general approach seems sane though. > >> Anyway, if this is not the way to go I'd be more than happy to do whatever >> work is needed to create the right interface. >> >> PS. I'd be out of the office and away from email access to the coming week, so >> kindly excuse any delay in response. > I think it's fine. However, I don't really like the idea of everyone > coming up with their own "paes", i.e., your patch's use of "haes". > It should be OK to just use paes for everyone, no? > > As to your patch specifically, there is one issue where you're > directly dereferencing the key as a struct. This is a no-no because > the key may have come from user-space. You must treat it as a > binary blob. The s390 code seems to do this correctly. > > Cheers, I would be happy to have a generic kernel interface for some kind of key token as a binary blob. I am also open to use the kernel key ring for future extensions. But please understand we needed a way to support our hardware keys and I think the chosen design isn't so bad. regards Harald Freudenberger using the kernel key ring in future extensions.
On Mon, Apr 09, 2018 at 11:42:31AM +0300, Gilad Ben-Yossef wrote: > > Please look again. The stub version of cc_is_hw_key() doing that is being > replaced in this patch. The point is that the existing mechanism was unused before and this is new code. So you can't really point to the stubbed-out function as a precedent. > The s390 key and the cryptocell keys are not the same: > > Their is, I believe, is an AES key encrypted by some internal key/algorithm. > > The cryptocell "key" is a token, which is internally comprised of one > or two indexes, referencing slots in the internal memory in the > hardware, and a key size, that describe the size of the key. > > I thought it would be confusing to use "paes" to describe both, since > they are not interchangeable. > You would not be able to feed an paes key that works with the s390 > version to cryptocell and vice verse and get it work. Thanks for the info. > Having said, if you prefer to have "paes" simply designate > "implementation specific token for an AES key" I'm perfectly fine with > that. Well by definition none of these hardware keys will be compatible with each other so I don't really see the point of using individual algorithm names such as paes or haes. This would make sense only if they were somehow compatible with each other. So instead of using algorithm names, you really want refer to the specific driver name, which means that they can all use the same algorithm name. > > As to your patch specifically, there is one issue where you're > > directly dereferencing the key as a struct. This is a no-no because > > the key may have come from user-space. You must treat it as a > > binary blob. The s390 code seems to do this correctly. > > As noted above, the haes "key" is really a token encoding 3 different > pieces of information: My point is that you should not just cast it but instead do a copy to properly aligned kernel memory. Cheers,
On Thu, Apr 19, 2018 at 6:35 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote: > On Mon, Apr 09, 2018 at 11:42:31AM +0300, Gilad Ben-Yossef wrote: >> >> Please look again. The stub version of cc_is_hw_key() doing that is being >> replaced in this patch. > > The point is that the existing mechanism was unused before and this > is new code. So you can't really point to the stubbed-out function > as a precedent. hm... I was trying to point to the s390 implementation as a precedent, not my own stub code. Sorry if I miscommunicated my intent. > >> The s390 key and the cryptocell keys are not the same: >> >> Their is, I believe, is an AES key encrypted by some internal key/algorithm. >> >> The cryptocell "key" is a token, which is internally comprised of one >> or two indexes, referencing slots in the internal memory in the >> hardware, and a key size, that describe the size of the key. >> >> I thought it would be confusing to use "paes" to describe both, since >> they are not interchangeable. >> You would not be able to feed an paes key that works with the s390 >> version to cryptocell and vice verse and get it work. > > Thanks for the info. > >> Having said, if you prefer to have "paes" simply designate >> "implementation specific token for an AES key" I'm perfectly fine with >> that. > > Well by definition none of these hardware keys will be compatible > with each other so I don't really see the point of using individual > algorithm names such as paes or haes. This would make sense only if > they were somehow compatible with each other. > > So instead of using algorithm names, you really want refer to the > specific driver name, which means that they can all use the same > algorithm name. Sounds good to me. > >> > As to your patch specifically, there is one issue where you're >> > directly dereferencing the key as a struct. This is a no-no because >> > the key may have come from user-space. You must treat it as a >> > binary blob. The s390 code seems to do this correctly. >> >> As noted above, the haes "key" is really a token encoding 3 different >> pieces of information: > > My point is that you should not just cast it but instead do a > copy to properly aligned kernel memory. That is a good point I completely missed. Thanks! A v2 will follow shortly. Thanks, Gilad
diff --git a/crypto/testmgr.c b/crypto/testmgr.c index af4a01c..8a5a60c 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -2558,6 +2558,13 @@ static const struct alg_test_desc alg_test_descs[] = { } } }, { + /* Same as cbc(aes) except the key is stored in + * hardware secure memory which we reference by index + */ + .alg = "cbc(haes)", + .test = alg_test_null, + .fips_allowed = 1, + }, { .alg = "cbc(serpent)", .test = alg_test_skcipher, .suite = { @@ -2704,6 +2711,13 @@ static const struct alg_test_desc alg_test_descs[] = { } } }, { + /* Same as ctr(aes) except the key is stored in + * hardware secure memory which we reference by index + */ + .alg = "ctr(haes)", + .test = alg_test_null, + .fips_allowed = 1, + }, { .alg = "ctr(serpent)", .test = alg_test_skcipher, .suite = { @@ -2974,6 +2988,13 @@ static const struct alg_test_desc alg_test_descs[] = { } } }, { + /* Same as ecb(aes) except the key is stored in + * hardware secure memory which we reference by index + */ + .alg = "ecb(haes)", + .test = alg_test_null, + .fips_allowed = 1, + }, { .alg = "ecb(khazad)", .test = alg_test_skcipher, .suite = { @@ -3301,6 +3322,13 @@ static const struct alg_test_desc alg_test_descs[] = { } } }, { + /* Same as ofb(aes) except the key is stored in + * hardware secure memory which we reference by index + */ + .alg = "ofb(haes)", + .test = alg_test_null, + .fips_allowed = 1, + }, { .alg = "pcbc(fcrypt)", .test = alg_test_skcipher, .suite = { @@ -3558,6 +3586,21 @@ static const struct alg_test_desc alg_test_descs[] = { } } }, { + /* Same as xts(aes) except the key is stored in + * hardware secure memory which we reference by index + */ + .alg = "xts(haes)", + .test = alg_test_null, + .fips_allowed = 1, + }, { + .alg = "xts4096(haes)", + .test = alg_test_null, + .fips_allowed = 1, + }, { + .alg = "xts512(haes)", + .test = alg_test_null, + .fips_allowed = 1, + }, { .alg = "xts(camellia)", .test = alg_test_skcipher, .suite = { diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c index df98f7a..8ccb7c4 100644 --- a/drivers/crypto/ccree/cc_cipher.c +++ b/drivers/crypto/ccree/cc_cipher.c @@ -42,6 +42,7 @@ struct cc_cipher_ctx { int cipher_mode; int flow_mode; unsigned int flags; + bool hw_key; struct cc_user_key_info user; struct cc_hw_key_info hw; struct crypto_shash *shash_tfm; @@ -49,6 +50,13 @@ struct cc_cipher_ctx { static void cc_cipher_complete(struct device *dev, void *cc_req, int err); +static inline bool cc_is_hw_key(struct crypto_tfm *tfm) +{ + struct cc_cipher_ctx *ctx_p = crypto_tfm_ctx(tfm); + + return ctx_p->hw_key; +} + static int validate_keys_sizes(struct cc_cipher_ctx *ctx_p, u32 size) { switch (ctx_p->flow_mode) { @@ -211,7 +219,7 @@ struct tdes_keys { u8 key3[DES_KEY_SIZE]; }; -static enum cc_hw_crypto_key hw_key_to_cc_hw_key(int slot_num) +static enum cc_hw_crypto_key cc_slot_to_hw_key(int slot_num) { switch (slot_num) { case 0: @@ -226,69 +234,98 @@ static enum cc_hw_crypto_key hw_key_to_cc_hw_key(int slot_num) return END_OF_KEYS; } -static int cc_cipher_setkey(struct crypto_skcipher *sktfm, const u8 *key, - unsigned int keylen) +static int cc_cipher_sethkey(struct crypto_skcipher *sktfm, const u8 *key, + unsigned int keylen) { struct crypto_tfm *tfm = crypto_skcipher_tfm(sktfm); struct cc_cipher_ctx *ctx_p = crypto_tfm_ctx(tfm); struct device *dev = drvdata_to_dev(ctx_p->drvdata); - u32 tmp[DES3_EDE_EXPKEY_WORDS]; - struct cc_crypto_alg *cc_alg = - container_of(tfm->__crt_alg, struct cc_crypto_alg, - skcipher_alg.base); - unsigned int max_key_buf_size = cc_alg->skcipher_alg.max_keysize; + struct cc_hkey_info *hki = (struct cc_hkey_info *)key; - dev_dbg(dev, "Setting key in context @%p for %s. keylen=%u\n", + dev_dbg(dev, "Setting HW key in context @%p for %s. keylen=%u\n", ctx_p, crypto_tfm_alg_name(tfm), keylen); dump_byte_array("key", (u8 *)key, keylen); /* STAT_PHASE_0: Init and sanity checks */ + /* This check the size of the hardware key token */ + if (keylen != sizeof(*hki)) { + dev_err(dev, "Unsupported HW key size %d.\n", keylen); + crypto_tfm_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN); + return -EINVAL; + } + + if (ctx_p->flow_mode != S_DIN_to_AES) { + dev_err(dev, "HW key not supported for non-AES flows\n"); + return -EINVAL; + } + + /* The real key len for crypto op is the size of the HW key + * referenced by the HW key slot, not the hardware key token + */ + keylen = hki->keylen; + if (validate_keys_sizes(ctx_p, keylen)) { dev_err(dev, "Unsupported key size %d.\n", keylen); crypto_tfm_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN); return -EINVAL; } - if (cc_is_hw_key(tfm)) { - /* setting HW key slots */ - struct arm_hw_key_info *hki = (struct arm_hw_key_info *)key; + ctx_p->hw.key1_slot = cc_slot_to_hw_key(hki->hw_key1); + if (ctx_p->hw.key1_slot == END_OF_KEYS) { + dev_err(dev, "Unsupported hw key1 number (%d)\n", hki->hw_key1); + return -EINVAL; + } - if (ctx_p->flow_mode != S_DIN_to_AES) { - dev_err(dev, "HW key not supported for non-AES flows\n"); + if (ctx_p->cipher_mode == DRV_CIPHER_XTS || + ctx_p->cipher_mode == DRV_CIPHER_ESSIV || + ctx_p->cipher_mode == DRV_CIPHER_BITLOCKER) { + if (hki->hw_key1 == hki->hw_key2) { + dev_err(dev, "Illegal hw key numbers (%d,%d)\n", + hki->hw_key1, hki->hw_key2); return -EINVAL; } - - ctx_p->hw.key1_slot = hw_key_to_cc_hw_key(hki->hw_key1); - if (ctx_p->hw.key1_slot == END_OF_KEYS) { - dev_err(dev, "Unsupported hw key1 number (%d)\n", - hki->hw_key1); + ctx_p->hw.key2_slot = cc_slot_to_hw_key(hki->hw_key2); + if (ctx_p->hw.key2_slot == END_OF_KEYS) { + dev_err(dev, "Unsupported hw key2 number (%d)\n", + hki->hw_key2); return -EINVAL; } + } - if (ctx_p->cipher_mode == DRV_CIPHER_XTS || - ctx_p->cipher_mode == DRV_CIPHER_ESSIV || - ctx_p->cipher_mode == DRV_CIPHER_BITLOCKER) { - if (hki->hw_key1 == hki->hw_key2) { - dev_err(dev, "Illegal hw key numbers (%d,%d)\n", - hki->hw_key1, hki->hw_key2); - return -EINVAL; - } - ctx_p->hw.key2_slot = - hw_key_to_cc_hw_key(hki->hw_key2); - if (ctx_p->hw.key2_slot == END_OF_KEYS) { - dev_err(dev, "Unsupported hw key2 number (%d)\n", - hki->hw_key2); - return -EINVAL; - } - } + ctx_p->keylen = keylen; + ctx_p->hw_key = true; + dev_dbg(dev, "cc_is_hw_key ret 0"); - ctx_p->keylen = keylen; - dev_dbg(dev, "cc_is_hw_key ret 0"); + return 0; +} - return 0; +static int cc_cipher_setkey(struct crypto_skcipher *sktfm, const u8 *key, + unsigned int keylen) +{ + struct crypto_tfm *tfm = crypto_skcipher_tfm(sktfm); + struct cc_cipher_ctx *ctx_p = crypto_tfm_ctx(tfm); + struct device *dev = drvdata_to_dev(ctx_p->drvdata); + u32 tmp[DES3_EDE_EXPKEY_WORDS]; + struct cc_crypto_alg *cc_alg = + container_of(tfm->__crt_alg, struct cc_crypto_alg, + skcipher_alg.base); + unsigned int max_key_buf_size = cc_alg->skcipher_alg.max_keysize; + + dev_dbg(dev, "Setting key in context @%p for %s. keylen=%u\n", + ctx_p, crypto_tfm_alg_name(tfm), keylen); + dump_byte_array("key", (u8 *)key, keylen); + + /* STAT_PHASE_0: Init and sanity checks */ + + if (validate_keys_sizes(ctx_p, keylen)) { + dev_err(dev, "Unsupported key size %d.\n", keylen); + crypto_tfm_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN); + return -EINVAL; } + ctx_p->hw_key = false; + /* * Verify DES weak keys * Note that we're dropping the expanded key since the @@ -735,6 +772,241 @@ static int cc_cipher_decrypt(struct skcipher_request *req) /* Block cipher alg */ static const struct cc_alg_template skcipher_algs[] = { { + .name = "xts(haes)", + .driver_name = "xts-haes-ccree", + .blocksize = AES_BLOCK_SIZE, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = AES_BLOCK_SIZE, + }, + .cipher_mode = DRV_CIPHER_XTS, + .flow_mode = S_DIN_to_AES, + .min_hw_rev = CC_HW_REV_630, + }, + { + .name = "xts512(haes)", + .driver_name = "xts-haes-du512-ccree", + .blocksize = AES_BLOCK_SIZE, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = AES_BLOCK_SIZE, + }, + .cipher_mode = DRV_CIPHER_XTS, + .flow_mode = S_DIN_to_AES, + .data_unit = 512, + .min_hw_rev = CC_HW_REV_712, + }, + { + .name = "xts4096(haes)", + .driver_name = "xts-haes-du4096-ccree", + .blocksize = AES_BLOCK_SIZE, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = AES_BLOCK_SIZE, + }, + .cipher_mode = DRV_CIPHER_XTS, + .flow_mode = S_DIN_to_AES, + .data_unit = 4096, + .min_hw_rev = CC_HW_REV_712, + }, + { + .name = "essiv(haes)", + .driver_name = "essiv-haes-ccree", + .blocksize = AES_BLOCK_SIZE, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = AES_BLOCK_SIZE, + }, + .cipher_mode = DRV_CIPHER_ESSIV, + .flow_mode = S_DIN_to_AES, + .min_hw_rev = CC_HW_REV_712, + }, + { + .name = "essiv512(haes)", + .driver_name = "essiv-haes-du512-ccree", + .blocksize = AES_BLOCK_SIZE, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = AES_BLOCK_SIZE, + }, + .cipher_mode = DRV_CIPHER_ESSIV, + .flow_mode = S_DIN_to_AES, + .data_unit = 512, + .min_hw_rev = CC_HW_REV_712, + }, + { + .name = "essiv4096(haes)", + .driver_name = "essiv-haes-du4096-ccree", + .blocksize = AES_BLOCK_SIZE, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = AES_BLOCK_SIZE, + }, + .cipher_mode = DRV_CIPHER_ESSIV, + .flow_mode = S_DIN_to_AES, + .data_unit = 4096, + .min_hw_rev = CC_HW_REV_712, + }, + { + .name = "bitlocker(haes)", + .driver_name = "bitlocker-haes-ccree", + .blocksize = AES_BLOCK_SIZE, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = AES_BLOCK_SIZE, + }, + .cipher_mode = DRV_CIPHER_BITLOCKER, + .flow_mode = S_DIN_to_AES, + .min_hw_rev = CC_HW_REV_712, + }, + { + .name = "bitlocker512(haes)", + .driver_name = "bitlocker-haes-du512-ccree", + .blocksize = AES_BLOCK_SIZE, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = AES_BLOCK_SIZE, + }, + .cipher_mode = DRV_CIPHER_BITLOCKER, + .flow_mode = S_DIN_to_AES, + .data_unit = 512, + .min_hw_rev = CC_HW_REV_712, + }, + { + .name = "bitlocker4096(haes)", + .driver_name = "bitlocker-haes-du4096-ccree", + .blocksize = AES_BLOCK_SIZE, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = AES_BLOCK_SIZE, + }, + .cipher_mode = DRV_CIPHER_BITLOCKER, + .flow_mode = S_DIN_to_AES, + .data_unit = 4096, + .min_hw_rev = CC_HW_REV_712, + }, + { + .name = "ecb(haes)", + .driver_name = "ecb-haes-ccree", + .blocksize = AES_BLOCK_SIZE, + .type = CRYPTO_ALG_TYPE_ABLKCIPHER, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = 0, + }, + .cipher_mode = DRV_CIPHER_ECB, + .flow_mode = S_DIN_to_AES, + .min_hw_rev = CC_HW_REV_712, + }, + { + .name = "cbc(haes)", + .driver_name = "cbc-haes-ccree", + .blocksize = AES_BLOCK_SIZE, + .type = CRYPTO_ALG_TYPE_ABLKCIPHER, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = AES_BLOCK_SIZE, + }, + .cipher_mode = DRV_CIPHER_CBC, + .flow_mode = S_DIN_to_AES, + .min_hw_rev = CC_HW_REV_712, + }, + { + .name = "ofb(haes)", + .driver_name = "ofb-haes-ccree", + .blocksize = AES_BLOCK_SIZE, + .type = CRYPTO_ALG_TYPE_ABLKCIPHER, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = AES_BLOCK_SIZE, + }, + .cipher_mode = DRV_CIPHER_OFB, + .flow_mode = S_DIN_to_AES, + .min_hw_rev = CC_HW_REV_712, + }, + { + .name = "cts1(cbc(haes))", + .driver_name = "cts1-cbc-haes-ccree", + .blocksize = AES_BLOCK_SIZE, + .type = CRYPTO_ALG_TYPE_ABLKCIPHER, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = AES_BLOCK_SIZE, + }, + .cipher_mode = DRV_CIPHER_CBC_CTS, + .flow_mode = S_DIN_to_AES, + .min_hw_rev = CC_HW_REV_712, + }, + { + .name = "ctr(haes)", + .driver_name = "ctr-haes-ccree", + .blocksize = 1, + .type = CRYPTO_ALG_TYPE_ABLKCIPHER, + .template_skcipher = { + .setkey = cc_cipher_sethkey, + .encrypt = cc_cipher_encrypt, + .decrypt = cc_cipher_decrypt, + .min_keysize = CC_HW_KEY_SIZE, + .max_keysize = CC_HW_KEY_SIZE, + .ivsize = AES_BLOCK_SIZE, + }, + .cipher_mode = DRV_CIPHER_CTR, + .flow_mode = S_DIN_to_AES, + .min_hw_rev = CC_HW_REV_712, + }, + { .name = "xts(aes)", .driver_name = "xts-aes-ccree", .blocksize = AES_BLOCK_SIZE, diff --git a/drivers/crypto/ccree/cc_cipher.h b/drivers/crypto/ccree/cc_cipher.h index 2a2a6f4..68444cf 100644 --- a/drivers/crypto/ccree/cc_cipher.h +++ b/drivers/crypto/ccree/cc_cipher.h @@ -13,18 +13,6 @@ #include "cc_driver.h" #include "cc_buffer_mgr.h" -/* Crypto cipher flags */ -#define CC_CRYPTO_CIPHER_KEY_KFDE0 BIT(0) -#define CC_CRYPTO_CIPHER_KEY_KFDE1 BIT(1) -#define CC_CRYPTO_CIPHER_KEY_KFDE2 BIT(2) -#define CC_CRYPTO_CIPHER_KEY_KFDE3 BIT(3) -#define CC_CRYPTO_CIPHER_DU_SIZE_512B BIT(4) - -#define CC_CRYPTO_CIPHER_KEY_KFDE_MASK (CC_CRYPTO_CIPHER_KEY_KFDE0 | \ - CC_CRYPTO_CIPHER_KEY_KFDE1 | \ - CC_CRYPTO_CIPHER_KEY_KFDE2 | \ - CC_CRYPTO_CIPHER_KEY_KFDE3) - struct cipher_req_ctx { struct async_gen_req_ctx gen_ctx; enum cc_req_dma_buf_type dma_buf_type; @@ -42,18 +30,12 @@ int cc_cipher_alloc(struct cc_drvdata *drvdata); int cc_cipher_free(struct cc_drvdata *drvdata); -struct arm_hw_key_info { - int hw_key1; - int hw_key2; -}; +struct cc_hkey_info { + u16 keylen; + u8 hw_key1; + u8 hw_key2; +} __packed; -/* - * This is a stub function that will replaced when we - * implement secure keys - */ -static inline bool cc_is_hw_key(struct crypto_tfm *tfm) -{ - return false; -} +#define CC_HW_KEY_SIZE sizeof(struct cc_hkey_info) #endif /*__CC_CIPHER_H__*/
Enable CryptoCell support for hardware keys. Hardware keys are regular AES keys loaded into CryptoCell internal memory via firmware, often from secure boot ROM or hardware fuses at boot time. As such, they can be used for enc/dec purposes like any other key but cannot (read: extremely hard to) be extracted since since they are not available anywhere in RAM during runtime. The mechanism has some similarities to s390 secure keys although the keys are not wrapped or sealed, but simply loaded offline. The interface was therefore modeled based on the s390 secure keys support. Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com> --- crypto/testmgr.c | 43 +++++ drivers/crypto/ccree/cc_cipher.c | 348 ++++++++++++++++++++++++++++++++++----- drivers/crypto/ccree/cc_cipher.h | 30 +--- 3 files changed, 359 insertions(+), 62 deletions(-)