* Early init for few crypto modules for Secure Guests @ 2022-10-04 4:41 Nikunj A. Dadhania 2022-10-04 8:24 ` Ard Biesheuvel 0 siblings, 1 reply; 7+ messages in thread From: Nikunj A. Dadhania @ 2022-10-04 4:41 UTC (permalink / raw) To: herbert, davem; +Cc: linux-crypto, Tom Lendacky, ketanch Hi! We are trying to implement Secure TSC feature for AMD SNP guests [1]. During the boot-up of the secondary cpus, SecureTSC enabled guests need to query TSC info from Security processor (PSP). This communication channel is encrypted between the security processor and the guest, hypervisor is just the conduit to deliver the guest messages to the security processor. Each message is protected with an AEAD (AES-256 GCM). As the TSC info is needed during the smpboot phase, few crypto modules need to be loaded early to use the crypto api for encryption/decryption of SNP Guest messages. I was able to get the SNP Guest messages working with initializing few crypto modules using early_initcall() instead of subsys_initcall(). Require suggestion/inputs if this is acceptable. List of modules that was changed to early_initcall: early_initcall(aes_init); early_initcall(cryptomgr_init); early_initcall(crypto_ctr_module_init); early_initcall(crypto_gcm_module_init); early_initcall(ghash_mod_init); Thanks, Nikunj 1. AMD APM Vol-2 (15.36.18 Secure TSC): https://www.amd.com/system/files/TechDocs/24593.pdf 2. SNP ABI Spec (7.9 TSC Info): https://www.amd.com/system/files/TechDocs/56860.pdf ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Early init for few crypto modules for Secure Guests 2022-10-04 4:41 Early init for few crypto modules for Secure Guests Nikunj A. Dadhania @ 2022-10-04 8:24 ` Ard Biesheuvel 2022-10-04 8:28 ` Ard Biesheuvel 0 siblings, 1 reply; 7+ messages in thread From: Ard Biesheuvel @ 2022-10-04 8:24 UTC (permalink / raw) To: Nikunj A. Dadhania; +Cc: herbert, davem, linux-crypto, Tom Lendacky, ketanch On Tue, 4 Oct 2022 at 06:41, Nikunj A. Dadhania <nikunj@amd.com> wrote: > > Hi! > > We are trying to implement Secure TSC feature for AMD SNP guests [1]. During the boot-up of the > secondary cpus, SecureTSC enabled guests need to query TSC info from Security processor (PSP). > This communication channel is encrypted between the security processor and the guest, > hypervisor is just the conduit to deliver the guest messages to the security processor. > Each message is protected with an AEAD (AES-256 GCM). > > As the TSC info is needed during the smpboot phase, few crypto modules need to be loaded early > to use the crypto api for encryption/decryption of SNP Guest messages. > > I was able to get the SNP Guest messages working with initializing few crypto modules using > early_initcall() instead of subsys_initcall(). > > Require suggestion/inputs if this is acceptable. List of modules that was changed > to early_initcall: > > early_initcall(aes_init); > early_initcall(cryptomgr_init); > early_initcall(crypto_ctr_module_init); > early_initcall(crypto_gcm_module_init); > early_initcall(ghash_mod_init); > I understand the need for this, but I think it is a bad idea. These will run even before SMP bringup, and before pure initcalls, which are documented as ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Early init for few crypto modules for Secure Guests 2022-10-04 8:24 ` Ard Biesheuvel @ 2022-10-04 8:28 ` Ard Biesheuvel 2022-10-04 9:50 ` Nikunj A. Dadhania 0 siblings, 1 reply; 7+ messages in thread From: Ard Biesheuvel @ 2022-10-04 8:28 UTC (permalink / raw) To: Nikunj A. Dadhania; +Cc: herbert, davem, linux-crypto, Tom Lendacky, ketanch On Tue, 4 Oct 2022 at 10:24, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Tue, 4 Oct 2022 at 06:41, Nikunj A. Dadhania <nikunj@amd.com> wrote: > > > > Hi! > > > > We are trying to implement Secure TSC feature for AMD SNP guests [1]. During the boot-up of the > > secondary cpus, SecureTSC enabled guests need to query TSC info from Security processor (PSP). > > This communication channel is encrypted between the security processor and the guest, > > hypervisor is just the conduit to deliver the guest messages to the security processor. > > Each message is protected with an AEAD (AES-256 GCM). > > > > As the TSC info is needed during the smpboot phase, few crypto modules need to be loaded early > > to use the crypto api for encryption/decryption of SNP Guest messages. > > > > I was able to get the SNP Guest messages working with initializing few crypto modules using > > early_initcall() instead of subsys_initcall(). > > > > Require suggestion/inputs if this is acceptable. List of modules that was changed > > to early_initcall: > > > > early_initcall(aes_init); > > early_initcall(cryptomgr_init); > > early_initcall(crypto_ctr_module_init); > > early_initcall(crypto_gcm_module_init); > > early_initcall(ghash_mod_init); > > > > I understand the need for this, but I think it is a bad idea. These > will run even before SMP bringup, and before pure initcalls, which are > documented as /* * A "pure" initcall has no dependencies on anything else, and purely * initializes variables that couldn't be statically initialized. */ So basically, you should not be relying on any global infrastructure to have been initialized. This is also something that may cause different problems on different architectures, and doing this only for x86 seems like a problem as well. Can you elaborate a bit on the use case? AES in GCM mode seems like a thing that we might be able to add to the crypto library API without much hassle (which already has a minimal implementation of AES) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Early init for few crypto modules for Secure Guests 2022-10-04 8:28 ` Ard Biesheuvel @ 2022-10-04 9:50 ` Nikunj A. Dadhania 2022-10-04 17:17 ` Ard Biesheuvel 0 siblings, 1 reply; 7+ messages in thread From: Nikunj A. Dadhania @ 2022-10-04 9:50 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: herbert, davem, linux-crypto, Tom Lendacky, ketanch On 04/10/22 13:58, Ard Biesheuvel wrote: > On Tue, 4 Oct 2022 at 10:24, Ard Biesheuvel <ardb@kernel.org> wrote: >> >> On Tue, 4 Oct 2022 at 06:41, Nikunj A. Dadhania <nikunj@amd.com> wrote: >>> >>> Hi! >>> >>> We are trying to implement Secure TSC feature for AMD SNP guests [1]. During the boot-up of the >>> secondary cpus, SecureTSC enabled guests need to query TSC info from Security processor (PSP). >>> This communication channel is encrypted between the security processor and the guest, >>> hypervisor is just the conduit to deliver the guest messages to the security processor. >>> Each message is protected with an AEAD (AES-256 GCM). >>> >>> As the TSC info is needed during the smpboot phase, few crypto modules need to be loaded early >>> to use the crypto api for encryption/decryption of SNP Guest messages. >>> >>> I was able to get the SNP Guest messages working with initializing few crypto modules using >>> early_initcall() instead of subsys_initcall(). >>> >>> Require suggestion/inputs if this is acceptable. List of modules that was changed >>> to early_initcall: >>> >>> early_initcall(aes_init); >>> early_initcall(cryptomgr_init); >>> early_initcall(crypto_ctr_module_init); >>> early_initcall(crypto_gcm_module_init); >>> early_initcall(ghash_mod_init); >>> >> >> I understand the need for this, but I think it is a bad idea. These >> will run even before SMP bringup, and before pure initcalls, which are >> documented as > > /* > * A "pure" initcall has no dependencies on anything else, and purely > * initializes variables that couldn't be statically initialized. > */> > So basically, you should not be relying on any global infrastructure > to have been initialized. This is also something that may cause > different problems on different architectures, and doing this only for > x86 seems like a problem as well. > > Can you elaborate a bit on the use case? Parameters used in TSC value calculation is controlled by the hypervisor and a malicious hypervisor can prevent guest from moving forward. Secure TSC allows guest to securely use rdtsc/rdtscp as the parameters being used now cannot be changed by hypervisor once the guest is launched. For the boot-cpu, TSC_SCALE/OFFSET is initialized as part of the guest launch process. During the secure guest boot, boot cpu will start bringing up the secondary CPUs. While creation of secondary CPU, TSC_SCALE/OFFSET field needs to be initialized appropriately. SNP Guest messages are the mechanism to communicate with the PSP to prevent risks from a malicious hypervisor snooping. The PSP firmware provides each guests with four Virtual Machine Platform Communication key(VMPCK) and is passed to the guest using a special secrets page as part of the guest launch process. The key is either with the guest or the PSP firmware. The messages exchanged between the guest and the PSP firmware is encrypted/decrypted using this key. > AES in GCM mode seems like a > thing that we might be able to add to the crypto library API without > much hassle (which already has a minimal implementation of AES) That will be great ! Regards, Nikunj ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Early init for few crypto modules for Secure Guests 2022-10-04 9:50 ` Nikunj A. Dadhania @ 2022-10-04 17:17 ` Ard Biesheuvel 2022-10-06 11:50 ` Nikunj A. Dadhania 0 siblings, 1 reply; 7+ messages in thread From: Ard Biesheuvel @ 2022-10-04 17:17 UTC (permalink / raw) To: Nikunj A. Dadhania; +Cc: herbert, davem, linux-crypto, Tom Lendacky, ketanch On Tue, 4 Oct 2022 at 11:51, Nikunj A. Dadhania <nikunj@amd.com> wrote: > > On 04/10/22 13:58, Ard Biesheuvel wrote: > > On Tue, 4 Oct 2022 at 10:24, Ard Biesheuvel <ardb@kernel.org> wrote: > >> > >> On Tue, 4 Oct 2022 at 06:41, Nikunj A. Dadhania <nikunj@amd.com> wrote: > >>> > >>> Hi! > >>> > >>> We are trying to implement Secure TSC feature for AMD SNP guests [1]. During the boot-up of the > >>> secondary cpus, SecureTSC enabled guests need to query TSC info from Security processor (PSP). > >>> This communication channel is encrypted between the security processor and the guest, > >>> hypervisor is just the conduit to deliver the guest messages to the security processor. > >>> Each message is protected with an AEAD (AES-256 GCM). > >>> > >>> As the TSC info is needed during the smpboot phase, few crypto modules need to be loaded early > >>> to use the crypto api for encryption/decryption of SNP Guest messages. > >>> > >>> I was able to get the SNP Guest messages working with initializing few crypto modules using > >>> early_initcall() instead of subsys_initcall(). > >>> > >>> Require suggestion/inputs if this is acceptable. List of modules that was changed > >>> to early_initcall: > >>> > >>> early_initcall(aes_init); > >>> early_initcall(cryptomgr_init); > >>> early_initcall(crypto_ctr_module_init); > >>> early_initcall(crypto_gcm_module_init); > >>> early_initcall(ghash_mod_init); > >>> > >> > >> I understand the need for this, but I think it is a bad idea. These > >> will run even before SMP bringup, and before pure initcalls, which are > >> documented as > > > > /* > > * A "pure" initcall has no dependencies on anything else, and purely > > * initializes variables that couldn't be statically initialized. > > */> > > So basically, you should not be relying on any global infrastructure > > to have been initialized. This is also something that may cause > > different problems on different architectures, and doing this only for > > x86 seems like a problem as well. > > > > Can you elaborate a bit on the use case? > > Parameters used in TSC value calculation is controlled by > the hypervisor and a malicious hypervisor can prevent guest from > moving forward. Secure TSC allows guest to securely use rdtsc/rdtscp > as the parameters being used now cannot be changed by hypervisor once > the guest is launched. > > For the boot-cpu, TSC_SCALE/OFFSET is initialized as part of the guest > launch process. During the secure guest boot, boot cpu will start bringing > up the secondary CPUs. While creation of secondary CPU, TSC_SCALE/OFFSET > field needs to be initialized appropriately. SNP Guest messages are the > mechanism to communicate with the PSP to prevent risks from a malicious > hypervisor snooping. > > The PSP firmware provides each guests with four Virtual Machine Platform > Communication key(VMPCK) and is passed to the guest using a special secrets page > as part of the guest launch process. The key is either with the guest or the > PSP firmware. > > The messages exchanged between the guest and the PSP firmware is > encrypted/decrypted using this key. > > > AES in GCM mode seems like a > > thing that we might be able to add to the crypto library API without > > much hassle (which already has a minimal implementation of AES) > > That will be great ! > Try this branch and see if it works for you https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=libgcm ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Early init for few crypto modules for Secure Guests 2022-10-04 17:17 ` Ard Biesheuvel @ 2022-10-06 11:50 ` Nikunj A. Dadhania 2022-10-06 12:10 ` Ard Biesheuvel 0 siblings, 1 reply; 7+ messages in thread From: Nikunj A. Dadhania @ 2022-10-06 11:50 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: herbert, davem, linux-crypto, Tom Lendacky, ketanch On 04/10/22 22:47, Ard Biesheuvel wrote: > On Tue, 4 Oct 2022 at 11:51, Nikunj A. Dadhania <nikunj@amd.com> wrote: >> >>> AES in GCM mode seems like a >>> thing that we might be able to add to the crypto library API without >>> much hassle (which already has a minimal implementation of AES) >> >> That will be great ! >> > > Try this branch and see if it works for you > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=libgcm Thanks Ard, I had to make few changes to the api to get it working for my usecase. The ghash is store/retrieved from the AUTHTAG field of message header as per "Table 97. Message Header Format" in the SNP ABI document: https://www.amd.com/system/files/TechDocs/56860.pdf Below are the changes I had made in my tree. --- diff --git a/include/crypto/gcm.h b/include/crypto/gcm.h index bab85df6df7a..838d1b4e25c3 100644 --- a/include/crypto/gcm.h +++ b/include/crypto/gcm.h @@ -74,9 +74,11 @@ int gcm_setkey(struct gcm_ctx *ctx, const u8 *key, unsigned int keysize, unsigned int authsize); void gcm_encrypt(const struct gcm_ctx *ctx, u8 *dst, const u8 *src, - int src_len, const u8 *assoc, int assoc_len, const u8 *iv); + int src_len, const u8 *assoc, int assoc_len, const u8 *iv, + u8 *authtag); int gcm_decrypt(const struct gcm_ctx *ctx, u8 *dst, const u8 *src, - int src_len, const u8 *assoc, int assoc_len, const u8 *iv); + int src_len, const u8 *assoc, int assoc_len, const u8 *iv, + u8 *authtag); #endif diff --git a/lib/crypto/gcm.c b/lib/crypto/gcm.c index d80e430505ee..34c86b2ea2aa 100644 --- a/lib/crypto/gcm.c +++ b/lib/crypto/gcm.c @@ -30,7 +30,7 @@ int gcm_setkey(struct gcm_ctx *ctx, const u8 *key, static int gcm_crypt(const struct gcm_ctx *ctx, u8 *dst, const u8 *src, int crypt_len, const u8 *assoc, int assoc_len, - const u8 *iv, bool encrypt) + const u8 *iv, bool encrypt, u8 *authtag) { be128 tail = { cpu_to_be64(assoc_len * 8), cpu_to_be64(crypt_len * 8) }; u8 ctr[AES_BLOCK_SIZE], buf[AES_BLOCK_SIZE]; @@ -76,8 +76,8 @@ static int gcm_crypt(const struct gcm_ctx *ctx, u8 *dst, const u8 *src, put_unaligned_be32(1, ctr + GCM_AES_IV_SIZE); aes_encrypt(&ctx->aes_ctx, buf, ctr); - crypto_xor_cpy(encrypt ? dst : buf, buf, (u8 *)&ghash, ctx->authsize); - if (!encrypt && crypto_memneq(src, buf, ctx->authsize)) + crypto_xor_cpy(encrypt ? authtag : buf, buf, (u8 *)&ghash, ctx->authsize); + if (!encrypt && crypto_memneq(authtag, buf, ctx->authsize)) ret = -EBADMSG; memzero_explicit(&ghash, sizeof(ghash)); @@ -87,16 +87,18 @@ static int gcm_crypt(const struct gcm_ctx *ctx, u8 *dst, const u8 *src, } void gcm_encrypt(const struct gcm_ctx *ctx, u8 *dst, const u8 *src, - int src_len, const u8 *assoc, int assoc_len, const u8 *iv) + int src_len, const u8 *assoc, int assoc_len, const u8 *iv, + u8 *authtag) { - gcm_crypt(ctx, dst, src, src_len, assoc, assoc_len, iv, true); + gcm_crypt(ctx, dst, src, src_len, assoc, assoc_len, iv, true, authtag); } int gcm_decrypt(const struct gcm_ctx *ctx, u8 *dst, const u8 *src, - int src_len, const u8 *assoc, int assoc_len, const u8 *iv) + int src_len, const u8 *assoc, int assoc_len, const u8 *iv, + u8 *authtag) { return gcm_crypt(ctx, dst, src, src_len - ctx->authsize, assoc, - assoc_len, iv, false); + assoc_len, iv, false, authtag); } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: Early init for few crypto modules for Secure Guests 2022-10-06 11:50 ` Nikunj A. Dadhania @ 2022-10-06 12:10 ` Ard Biesheuvel 0 siblings, 0 replies; 7+ messages in thread From: Ard Biesheuvel @ 2022-10-06 12:10 UTC (permalink / raw) To: Nikunj A. Dadhania; +Cc: herbert, davem, linux-crypto, Tom Lendacky, ketanch On Thu, 6 Oct 2022 at 13:50, Nikunj A. Dadhania <nikunj@amd.com> wrote: > > > > On 04/10/22 22:47, Ard Biesheuvel wrote: > > On Tue, 4 Oct 2022 at 11:51, Nikunj A. Dadhania <nikunj@amd.com> wrote: > >> > >>> AES in GCM mode seems like a > >>> thing that we might be able to add to the crypto library API without > >>> much hassle (which already has a minimal implementation of AES) > >> > >> That will be great ! > >> > > > > Try this branch and see if it works for you > > > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=libgcm > > Thanks Ard, I had to make few changes to the api to get it working for my usecase. Excellent > The ghash is store/retrieved from the AUTHTAG field of message header as per > "Table 97. Message Header Format" in the SNP ABI document: > https://www.amd.com/system/files/TechDocs/56860.pdf > > Below are the changes I had made in my tree. > > --- > > diff --git a/include/crypto/gcm.h b/include/crypto/gcm.h > index bab85df6df7a..838d1b4e25c3 100644 > --- a/include/crypto/gcm.h > +++ b/include/crypto/gcm.h > @@ -74,9 +74,11 @@ int gcm_setkey(struct gcm_ctx *ctx, const u8 *key, > unsigned int keysize, unsigned int authsize); > > void gcm_encrypt(const struct gcm_ctx *ctx, u8 *dst, const u8 *src, > - int src_len, const u8 *assoc, int assoc_len, const u8 *iv); > + int src_len, const u8 *assoc, int assoc_len, const u8 *iv, > + u8 *authtag); > > int gcm_decrypt(const struct gcm_ctx *ctx, u8 *dst, const u8 *src, > - int src_len, const u8 *assoc, int assoc_len, const u8 *iv); > + int src_len, const u8 *assoc, int assoc_len, const u8 *iv, > + u8 *authtag); This should really be 'const u8 *authtag'. Which means that the encrypt/decrypt path should be split somewhat differently, i.e., something like void gcm_encrypt(const struct gcm_ctx *ctx, u8 *dst, const u8 *src, int crypt_len, const u8 *assoc, int assoc_len, const u8 iv[GCM_AES_IV_SIZE], u8 *authtag) { gcm_crypt(ctx, dst, src, crypt_len, assoc, assoc_len, iv, authtag, true); } int gcm_decrypt(const struct gcm_ctx *ctx, u8 *dst, const u8 *src, int crypt_len, const u8 *assoc, int assoc_len, const u8 iv[GCM_AES_IV_SIZE], const u8 *authtag) { u8 tagbuf[AES_BLOCK_SIZE]; gcm_crypt(ctx, dst, src, crypt_len - ctx->authsize, assoc, assoc_len, iv, tagbuf, false); if (crypto_memneq(authtag, tagbuf, ctx->authsize)) { memzero_explicit(tagbuf, sizeof(tagbuf)); return -EBADMSG; } return 0; } I've updated my branch with these (and some other changes). Now we just need to add some comment blocks to describe the API. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-10-06 12:10 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-04 4:41 Early init for few crypto modules for Secure Guests Nikunj A. Dadhania 2022-10-04 8:24 ` Ard Biesheuvel 2022-10-04 8:28 ` Ard Biesheuvel 2022-10-04 9:50 ` Nikunj A. Dadhania 2022-10-04 17:17 ` Ard Biesheuvel 2022-10-06 11:50 ` Nikunj A. Dadhania 2022-10-06 12:10 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).