qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] crypto/gcrypt: prefer kernel as direct source of entropy
@ 2024-01-19 20:39 Cristian Rodríguez
  2024-01-22 14:48 ` Daniel P. Berrangé
  0 siblings, 1 reply; 5+ messages in thread
From: Cristian Rodríguez @ 2024-01-19 20:39 UTC (permalink / raw)
  Cc: Cristian Rodríguez, Daniel P. Berrangé,
	open list:All patches CC here

gcrypt by default uses an userspace RNG, which cannot know
when it is time to discard/invalidate its buffer
(suspend, resume, vm forks, other corner cases)
as a "when to discard" event is unavailable to userspace.

Set GCRYCTL_SET_PREFERRED_RNG_TYPE to GCRY_RNG_TYPE_SYSTEM
which must be done before the first call to gcry_check_version()

Signed-off-by: Cristian Rodríguez <cristian@rodriguez.im>
---
 crypto/init.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/crypto/init.c b/crypto/init.c
index fb7f1bff10..0c3fe6a841 100644
--- a/crypto/init.c
+++ b/crypto/init.c
@@ -60,6 +60,7 @@ int qcrypto_init(Error **errp)
 #endif
 
 #ifdef CONFIG_GCRYPT
+    gcry_control(GCRYCTL_SET_PREFERRED_RNG_TYPE, GCRY_RNG_TYPE_SYSTEM);
     if (!gcry_check_version(NULL)) {
         error_setg(errp, "Unable to initialize gcrypt");
         return -1;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] crypto/gcrypt: prefer kernel as direct source of entropy
  2024-01-19 20:39 [PATCH] crypto/gcrypt: prefer kernel as direct source of entropy Cristian Rodríguez
@ 2024-01-22 14:48 ` Daniel P. Berrangé
  2024-01-22 20:08   ` Cristian Rodríguez
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel P. Berrangé @ 2024-01-22 14:48 UTC (permalink / raw)
  To: Cristian Rodríguez; +Cc: open list:All patches CC here

On Fri, Jan 19, 2024 at 05:39:40PM -0300, Cristian Rodríguez wrote:
> gcrypt by default uses an userspace RNG, which cannot know
> when it is time to discard/invalidate its buffer
> (suspend, resume, vm forks, other corner cases)
> as a "when to discard" event is unavailable to userspace.

So in this scenario QEMU is impacted when QEMU is running inside
another VM. ie the L0 QEMU "forks" the guest, and the L1 QEMU
needs to re-init its RNG.

> Set GCRYCTL_SET_PREFERRED_RNG_TYPE to GCRY_RNG_TYPE_SYSTEM
> which must be done before the first call to gcry_check_version()

QEMU is just one out of many applications that use libgcrypt and
I see no reason why QEMU should be special cased in this respect.

Updating each application to hardcode use of GCRY_RNG_TYPE_SYSTEM
does not feel like a good solution. If this was a good default
to use everywhere, then gcrypt should have set this default
already, rather than requiring every app to solve the forking
problem itself.

Updating every app that uses gcrypt is not really practical
in terms of time investment anyway.

If gcrypt doesn't want to make this its global default, then
I would suggest that gcrypt should make its default be
configurable. I see from its docs:

https://gnupg.org/documentation/manuals/gcrypt/Configuration.html#Configuration

that it already supports a /etc/gcrypt/random.conf file.
Perhaps they would extend that to allow selection of the
default RNG backend, system-wide.

> 
> Signed-off-by: Cristian Rodríguez <cristian@rodriguez.im>
> ---
>  crypto/init.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/crypto/init.c b/crypto/init.c
> index fb7f1bff10..0c3fe6a841 100644
> --- a/crypto/init.c
> +++ b/crypto/init.c
> @@ -60,6 +60,7 @@ int qcrypto_init(Error **errp)
>  #endif
>  
>  #ifdef CONFIG_GCRYPT
> +    gcry_control(GCRYCTL_SET_PREFERRED_RNG_TYPE, GCRY_RNG_TYPE_SYSTEM);
>      if (!gcry_check_version(NULL)) {
>          error_setg(errp, "Unable to initialize gcrypt");
>          return -1;
> -- 
> 2.43.0
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] crypto/gcrypt: prefer kernel as direct source of entropy
  2024-01-22 14:48 ` Daniel P. Berrangé
@ 2024-01-22 20:08   ` Cristian Rodríguez
  2024-01-22 20:19     ` Daniel P. Berrangé
  0 siblings, 1 reply; 5+ messages in thread
From: Cristian Rodríguez @ 2024-01-22 20:08 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: open list:All patches CC here, Jason A. Donenfeld

[-- Attachment #1: Type: text/plain, Size: 2180 bytes --]

On Mon, Jan 22, 2024 at 11:48 AM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Fri, Jan 19, 2024 at 05:39:40PM -0300, Cristian Rodríguez wrote:
> > gcrypt by default uses an userspace RNG, which cannot know
> > when it is time to discard/invalidate its buffer
> > (suspend, resume, vm forks, other corner cases)
> > as a "when to discard" event is unavailable to userspace.
>
> So in this scenario QEMU is impacted when QEMU is running inside
> another VM. ie the L0 QEMU "forks" the guest, and the L1 QEMU
> needs to re-init its RNG.
>
> > Set GCRYCTL_SET_PREFERRED_RNG_TYPE to GCRY_RNG_TYPE_SYSTEM
> > which must be done before the first call to gcry_check_version()
>
> QEMU is just one out of many applications that use libgcrypt and
> I see no reason why QEMU should be special cased in this respect.
>
> Updating each application to hardcode use of GCRY_RNG_TYPE_SYSTEM
> does not feel like a good solution. If this was a good default
> to use everywhere, then gcrypt should have set this default
> already, rather than requiring every app to solve the forking
> problem itself.
>

this default is because either other OSs had problems or in the past the
linux rng was not as performant as it is right now,
 now it outputs 100's of MB per second on a potato.

Updating every app that uses gcrypt is not really practical
> in terms of time investment anyway.
>

Yeah, it will be pretty time consuming so I have so far only sent a few
patches for things I consider important.

>
> If gcrypt doesn't want to make this its global default, then
> I would suggest that gcrypt should make its default be
> configurable. I see from its docs:
>
>
> https://gnupg.org/documentation/manuals/gcrypt/Configuration.html#Configuration
>
> that it already supports a /etc/gcrypt/random.conf file.
> Perhaps they would extend that to allow selection of the
> default RNG backend, system-wide.


And things will remain problematic by default , because of all the
obscurity and that FIPS mode overrides
all defaults you choose anyways, including if I hardcode the preference in
the source code like I did here.
.


>
>
>

[-- Attachment #2: Type: text/html, Size: 3228 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] crypto/gcrypt: prefer kernel as direct source of entropy
  2024-01-22 20:08   ` Cristian Rodríguez
@ 2024-01-22 20:19     ` Daniel P. Berrangé
  2024-01-22 20:21       ` Cristian Rodríguez
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel P. Berrangé @ 2024-01-22 20:19 UTC (permalink / raw)
  To: Cristian Rodríguez; +Cc: open list:All patches CC here, Jason A. Donenfeld

On Mon, Jan 22, 2024 at 05:08:16PM -0300, Cristian Rodríguez wrote:
> On Mon, Jan 22, 2024 at 11:48 AM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> 
> > On Fri, Jan 19, 2024 at 05:39:40PM -0300, Cristian Rodríguez wrote:
> > > gcrypt by default uses an userspace RNG, which cannot know
> > > when it is time to discard/invalidate its buffer
> > > (suspend, resume, vm forks, other corner cases)
> > > as a "when to discard" event is unavailable to userspace.
> >
> > So in this scenario QEMU is impacted when QEMU is running inside
> > another VM. ie the L0 QEMU "forks" the guest, and the L1 QEMU
> > needs to re-init its RNG.
> >
> > > Set GCRYCTL_SET_PREFERRED_RNG_TYPE to GCRY_RNG_TYPE_SYSTEM
> > > which must be done before the first call to gcry_check_version()
> >
> > QEMU is just one out of many applications that use libgcrypt and
> > I see no reason why QEMU should be special cased in this respect.
> >
> > Updating each application to hardcode use of GCRY_RNG_TYPE_SYSTEM
> > does not feel like a good solution. If this was a good default
> > to use everywhere, then gcrypt should have set this default
> > already, rather than requiring every app to solve the forking
> > problem itself.
> >
> 
> this default is because either other OSs had problems or in the past the
> linux rng was not as performant as it is right now,
>  now it outputs 100's of MB per second on a potato.
> 
> Updating every app that uses gcrypt is not really practical
> > in terms of time investment anyway.
> >
> 
> Yeah, it will be pretty time consuming so I have so far only sent a few
> patches for things I consider important.
> 
> >
> > If gcrypt doesn't want to make this its global default, then
> > I would suggest that gcrypt should make its default be
> > configurable. I see from its docs:
> >
> >
> > https://gnupg.org/documentation/manuals/gcrypt/Configuration.html#Configuration
> >
> > that it already supports a /etc/gcrypt/random.conf file.
> > Perhaps they would extend that to allow selection of the
> > default RNG backend, system-wide.
> 
> 
> And things will remain problematic by default , because of all the
> obscurity and that FIPS mode overrides
> all defaults you choose anyways, including if I hardcode the preference in
> the source code like I did here.

If the DRBG is required for FIPS compliance, and QEMU hardcoded
the system RNG, then QEMU can't be used in a FIPS environment.
So I still think this question of defaults is something to be
fixed in the gcrypt code centrally, not in individual apps.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] crypto/gcrypt: prefer kernel as direct source of entropy
  2024-01-22 20:19     ` Daniel P. Berrangé
@ 2024-01-22 20:21       ` Cristian Rodríguez
  0 siblings, 0 replies; 5+ messages in thread
From: Cristian Rodríguez @ 2024-01-22 20:21 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: open list:All patches CC here, Jason A. Donenfeld

[-- Attachment #1: Type: text/plain, Size: 299 bytes --]

On Mon, Jan 22, 2024 at 5:19 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

>
> If the DRBG is required for FIPS compliance, and QEMU hardcoded
> the system RNG, then QEMU can't be used in a FIPS environment.
>

No, the library overrides this choice.. the DRBG has higher priority.

[-- Attachment #2: Type: text/html, Size: 644 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-01-22 20:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-19 20:39 [PATCH] crypto/gcrypt: prefer kernel as direct source of entropy Cristian Rodríguez
2024-01-22 14:48 ` Daniel P. Berrangé
2024-01-22 20:08   ` Cristian Rodríguez
2024-01-22 20:19     ` Daniel P. Berrangé
2024-01-22 20:21       ` Cristian Rodríguez

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).