| Message ID | 20190514191653.31488-2-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add qemu_getrandom and ARMv8.5-RNG etc | expand |
On 14/05/2019 21:16, Richard Henderson wrote: > For user-only, we require only the random number bits of the > crypto subsystem. > > We need to preserve --static linking, which for many recent Linux > distributions precludes using GnuTLS or GCrypt. Instead, use our > random-platform module unconditionally. > Perhaps we can rename "crypto-aes-obj" to "crypto-user-obj" and put aes.o and random-platform.o into? The only aim of crypto-aes-obj was to link aes.o with linux-user binaries. Anyway, it's only cosmetic, so you can add: Reviewed-by: Laurent Vivier <lvivier@redhat.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > Makefile | 6 ++++-- > Makefile.objs | 1 + > Makefile.target | 3 ++- > crypto/Makefile.objs | 1 + > 4 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/Makefile b/Makefile > index 66d5c65156..524f2f8a57 100644 > --- a/Makefile > +++ b/Makefile > @@ -411,6 +411,7 @@ dummy := $(call unnest-vars,, \ > block-obj-m \ > crypto-obj-y \ > crypto-aes-obj-y \ > + crypto-rng-obj-y \ > qom-obj-y \ > io-obj-y \ > common-obj-y \ > @@ -482,8 +483,9 @@ subdir-capstone: .git-submodule-status > subdir-slirp: .git-submodule-status > $(call quiet-command,$(MAKE) -C $(SRC_PATH)/slirp BUILD_DIR="$(BUILD_DIR)/slirp" CC="$(CC)" AR="$(AR)" LD="$(LD)" RANLIB="$(RANLIB)" CFLAGS="$(QEMU_CFLAGS) $(CFLAGS)" LDFLAGS="$(LDFLAGS)") > > -$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) \ > - $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY)) > +$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) $(qom-obj-y) \ > + $(crypto-aes-obj-$(CONFIG_USER_ONLY)) \ > + $(crypto-rng-obj-$(CONFIG_USER_ONLY)) > > ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS)) > # Only keep -O and -g cflags > diff --git a/Makefile.objs b/Makefile.objs > index cf065de5ed..0c13ff47ea 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -26,6 +26,7 @@ block-obj-m = block/ > > crypto-obj-y = crypto/ > crypto-aes-obj-y = crypto/ > +crypto-rng-obj-y = crypto/ > > ####################################################################### > # qom-obj-y is code used by both qemu system emulation and qemu-img > diff --git a/Makefile.target b/Makefile.target > index ae02495951..4e579a0a84 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -181,6 +181,7 @@ dummy := $(call unnest-vars,.., \ > chardev-obj-y \ > crypto-obj-y \ > crypto-aes-obj-y \ > + crypto-rng-obj-y \ > qom-obj-y \ > io-obj-y \ > common-obj-y \ > @@ -189,7 +190,7 @@ all-obj-y += $(common-obj-y) > all-obj-y += $(qom-obj-y) > all-obj-$(CONFIG_SOFTMMU) += $(authz-obj-y) > all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y) $(chardev-obj-y) > -all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y) > +all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y) $(crypto-rng-obj-y) > all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y) > all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y) > > diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs > index 256c9aca1f..ee7e628ca6 100644 > --- a/crypto/Makefile.objs > +++ b/crypto/Makefile.objs > @@ -37,5 +37,6 @@ crypto-obj-y += block-luks.o > > # Let the userspace emulators avoid linking gnutls/etc > crypto-aes-obj-y = aes.o > +crypto-rng-obj-y = random-platform.o > > stub-obj-y += pbkdf-stub.o >
On 5/15/19 9:42 AM, Laurent Vivier wrote: > On 14/05/2019 21:16, Richard Henderson wrote: >> For user-only, we require only the random number bits of the >> crypto subsystem. >> >> We need to preserve --static linking, which for many recent Linux >> distributions precludes using GnuTLS or GCrypt. Instead, use our >> random-platform module unconditionally. >> > > Perhaps we can rename "crypto-aes-obj" to "crypto-user-obj" and put > aes.o and random-platform.o into? > > The only aim of crypto-aes-obj was to link aes.o with linux-user binaries. That does seem better. I'll make the change. r~ > > Anyway, it's only cosmetic, so you can add: > > Reviewed-by: Laurent Vivier <lvivier@redhat.com> > >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> Makefile | 6 ++++-- >> Makefile.objs | 1 + >> Makefile.target | 3 ++- >> crypto/Makefile.objs | 1 + >> 4 files changed, 8 insertions(+), 3 deletions(-)
On Tue, May 14, 2019 at 12:16:30PM -0700, Richard Henderson wrote: > For user-only, we require only the random number bits of the > crypto subsystem. > > We need to preserve --static linking, which for many recent Linux > distributions precludes using GnuTLS or GCrypt. Instead, use our > random-platform module unconditionally. I don't think we need to special case in this way. Today if you do a default build with all targets & tools and want to use --static, but don't have static libs available for some things you can achieve that ./configure --static --disable-gnutls --disable-gcrypt --disable-nettle Previously if you took care to disable system emulators & tools you could avoid the need to pass the --disable-* args, but I think that's fairly minor. So I think we should just use $(crypto-obj-y) unconditionally in the user emulators, and get rid of crypto-aes-obj-y too. This will give a consistent crypto story across all the things we build with no special cases. If people want a minimal static build they can stsill pass the above --disable-XXX args which will result in them only using the aes.o and rng-platform.o pieces. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > Makefile | 6 ++++-- > Makefile.objs | 1 + > Makefile.target | 3 ++- > crypto/Makefile.objs | 1 + > 4 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/Makefile b/Makefile > index 66d5c65156..524f2f8a57 100644 > --- a/Makefile > +++ b/Makefile > @@ -411,6 +411,7 @@ dummy := $(call unnest-vars,, \ > block-obj-m \ > crypto-obj-y \ > crypto-aes-obj-y \ > + crypto-rng-obj-y \ > qom-obj-y \ > io-obj-y \ > common-obj-y \ > @@ -482,8 +483,9 @@ subdir-capstone: .git-submodule-status > subdir-slirp: .git-submodule-status > $(call quiet-command,$(MAKE) -C $(SRC_PATH)/slirp BUILD_DIR="$(BUILD_DIR)/slirp" CC="$(CC)" AR="$(AR)" LD="$(LD)" RANLIB="$(RANLIB)" CFLAGS="$(QEMU_CFLAGS) $(CFLAGS)" LDFLAGS="$(LDFLAGS)") > > -$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) \ > - $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY)) > +$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) $(qom-obj-y) \ > + $(crypto-aes-obj-$(CONFIG_USER_ONLY)) \ > + $(crypto-rng-obj-$(CONFIG_USER_ONLY)) > > ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS)) > # Only keep -O and -g cflags > diff --git a/Makefile.objs b/Makefile.objs > index cf065de5ed..0c13ff47ea 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -26,6 +26,7 @@ block-obj-m = block/ > > crypto-obj-y = crypto/ > crypto-aes-obj-y = crypto/ > +crypto-rng-obj-y = crypto/ > > ####################################################################### > # qom-obj-y is code used by both qemu system emulation and qemu-img > diff --git a/Makefile.target b/Makefile.target > index ae02495951..4e579a0a84 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -181,6 +181,7 @@ dummy := $(call unnest-vars,.., \ > chardev-obj-y \ > crypto-obj-y \ > crypto-aes-obj-y \ > + crypto-rng-obj-y \ > qom-obj-y \ > io-obj-y \ > common-obj-y \ > @@ -189,7 +190,7 @@ all-obj-y += $(common-obj-y) > all-obj-y += $(qom-obj-y) > all-obj-$(CONFIG_SOFTMMU) += $(authz-obj-y) > all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y) $(chardev-obj-y) > -all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y) > +all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y) $(crypto-rng-obj-y) > all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y) > all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y) > > diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs > index 256c9aca1f..ee7e628ca6 100644 > --- a/crypto/Makefile.objs > +++ b/crypto/Makefile.objs > @@ -37,5 +37,6 @@ crypto-obj-y += block-luks.o > > # Let the userspace emulators avoid linking gnutls/etc > crypto-aes-obj-y = aes.o > +crypto-rng-obj-y = random-platform.o > > stub-obj-y += pbkdf-stub.o > -- > 2.17.1 > > 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 :|
On 5/15/19 9:53 AM, Daniel P. Berrangé wrote: > On Tue, May 14, 2019 at 12:16:30PM -0700, Richard Henderson wrote: >> For user-only, we require only the random number bits of the >> crypto subsystem. >> >> We need to preserve --static linking, which for many recent Linux >> distributions precludes using GnuTLS or GCrypt. Instead, use our >> random-platform module unconditionally. > > I don't think we need to special case in this way. > > Today if you do a default build with all targets & tools and want > to use --static, but don't have static libs available for some > things you can achieve that > > ./configure --static --disable-gnutls --disable-gcrypt --disable-nettle But we don't really want all of those --disable arguments by default. It would be one thing if one explicitly used --enable-gnutls and got link errors. We must preserve --static working all by itself. > Previously if you took care to disable system emulators & tools > you could avoid the need to pass the --disable-* args, but I > think that's fairly minor. Well, no, you get link errors. (As an aside, IMO pkg-config is stupid in being only able to ask "is version X installed" without also being about to ask "is a static version of X installed". pkg-config has a --static option, it just doesn't use it.) But suppose we add back the patch for --static sanity check from v6. What are we left with? No crypto libraries remain on Fedora 30. It appears that Ubuntu Bionic ships a static version of nettle, but nothing else. Is that useful on its own? > So I think we should just use $(crypto-obj-y) unconditionally in > the user emulators, and get rid of crypto-aes-obj-y too. > > This will give a consistent crypto story across all the things we > build with no special cases. Well, maybe. But what are we trying to accomplish? What use is crypto to the host side of linux-user? In general, all the crypto that the application will do is on the guest side, within guest versions of gnutls etc. All crypto that the guest expects of its kernel is done passing off the syscall to the host kernel. That's why, here in v7, I began to think that perhaps all the faffing about with pkg-config vs --static was just a waste of time. Have I missed something? r~
On Wed, May 15, 2019 at 10:22:08AM -0700, Richard Henderson wrote: > On 5/15/19 9:53 AM, Daniel P. Berrangé wrote: > > On Tue, May 14, 2019 at 12:16:30PM -0700, Richard Henderson wrote: > >> For user-only, we require only the random number bits of the > >> crypto subsystem. > >> > >> We need to preserve --static linking, which for many recent Linux > >> distributions precludes using GnuTLS or GCrypt. Instead, use our > >> random-platform module unconditionally. > > > > I don't think we need to special case in this way. > > > > Today if you do a default build with all targets & tools and want > > to use --static, but don't have static libs available for some > > things you can achieve that > > > > ./configure --static --disable-gnutls --disable-gcrypt --disable-nettle > > But we don't really want all of those --disable arguments by default. It would > be one thing if one explicitly used --enable-gnutls and got link errors. We > must preserve --static working all by itself. That's already not working today unless you add extra args to disable build of the system emulators and tools. > > Previously if you took care to disable system emulators & tools > > you could avoid the need to pass the --disable-* args, but I > > think that's fairly minor. > > Well, no, you get link errors. > > (As an aside, IMO pkg-config is stupid in being only able to ask "is version X > installed" without also being about to ask "is a static version of X > installed". pkg-config has a --static option, it just doesn't use it.) Yeah it is very frustrating that it isn't actually useful in the way you'd expect. > But suppose we add back the patch for --static sanity check from v6. What are > we left with? No crypto libraries remain on Fedora 30. It appears that Ubuntu > Bionic ships a static version of nettle, but nothing else. Is that useful on > its own? nettle isn't useful from POV of RNG. > > So I think we should just use $(crypto-obj-y) unconditionally in > > the user emulators, and get rid of crypto-aes-obj-y too. > > > > This will give a consistent crypto story across all the things we > > build with no special cases. > > Well, maybe. But what are we trying to accomplish? With this v7, if building dynamically we get some parts of QEMU using the full crypto/ impl and thus GNUTLS for RNG, and some parts of QEMU using just the rng-platform.o. I'd like it all to use the full crypto impl when building dynamically. Similarly if building statically, it should again result in the same fallback choices. If a static gnutls is available it should use that, but if none is present everything, it will get build with rng-platform.o IOW, either we just document need to pass --disable-gnutls --disable-gcrypt --disable-nettle if the distro lacks static versions of those libs - we already need thus documented as we already suffer from that problem when building statically. Or we need to make configure more clever and check if static linking actually works for these libs. > What use is crypto to the host side of linux-user? In general, all the crypto > that the application will do is on the guest side, within guest versions of > gnutls etc. All crypto that the guest expects of its kernel is done passing > off the syscall to the host kernel. > > That's why, here in v7, I began to think that perhaps all the faffing about > with pkg-config vs --static was just a waste of time. > > Have I missed something? 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 :|
On 15/05/2019 19:49, Daniel P. Berrangé wrote: > On Wed, May 15, 2019 at 10:22:08AM -0700, Richard Henderson wrote: >> On 5/15/19 9:53 AM, Daniel P. Berrangé wrote: >>> On Tue, May 14, 2019 at 12:16:30PM -0700, Richard Henderson wrote: >>>> For user-only, we require only the random number bits of the >>>> crypto subsystem. >>>> >>>> We need to preserve --static linking, which for many recent Linux >>>> distributions precludes using GnuTLS or GCrypt. Instead, use our >>>> random-platform module unconditionally. >>> >>> I don't think we need to special case in this way. >>> >>> Today if you do a default build with all targets & tools and want >>> to use --static, but don't have static libs available for some >>> things you can achieve that >>> >>> ./configure --static --disable-gnutls --disable-gcrypt --disable-nettle >> >> But we don't really want all of those --disable arguments by default. It would >> be one thing if one explicitly used --enable-gnutls and got link errors. We >> must preserve --static working all by itself. > > That's already not working today unless you add extra args to disable > build of the system emulators and tools. > Perhaps it can help, I have a series queued by Paolo to cleanup the build dependencies for --{disable,enable}-{system,user,tools}: [v3,0/5] build: cleanup in Makefile.objs https://patchwork.kernel.org/cover/10880135/ Thanks, Laurent
On Wed, May 15, 2019 at 09:38:00PM +0200, Laurent Vivier wrote: > On 15/05/2019 19:49, Daniel P. Berrangé wrote: > > On Wed, May 15, 2019 at 10:22:08AM -0700, Richard Henderson wrote: > >> On 5/15/19 9:53 AM, Daniel P. Berrangé wrote: > >>> On Tue, May 14, 2019 at 12:16:30PM -0700, Richard Henderson wrote: > >>>> For user-only, we require only the random number bits of the > >>>> crypto subsystem. > >>>> > >>>> We need to preserve --static linking, which for many recent Linux > >>>> distributions precludes using GnuTLS or GCrypt. Instead, use our > >>>> random-platform module unconditionally. > >>> > >>> I don't think we need to special case in this way. > >>> > >>> Today if you do a default build with all targets & tools and want > >>> to use --static, but don't have static libs available for some > >>> things you can achieve that > >>> > >>> ./configure --static --disable-gnutls --disable-gcrypt --disable-nettle > >> > >> But we don't really want all of those --disable arguments by default. It would > >> be one thing if one explicitly used --enable-gnutls and got link errors. We > >> must preserve --static working all by itself. > > > > That's already not working today unless you add extra args to disable > > build of the system emulators and tools. > > > > Perhaps it can help, I have a series queued by Paolo to cleanup the > build dependencies for --{disable,enable}-{system,user,tools}: > > [v3,0/5] build: cleanup in Makefile.objs > https://patchwork.kernel.org/cover/10880135/ I don't think it'll make a difference to use of --static when trying to build a default config (ie all targets + tools) 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 :|
On 5/15/19 9:53 AM, Daniel P. Berrangé wrote: > So I think we should just use $(crypto-obj-y) unconditionally in > the user emulators, and get rid of crypto-aes-obj-y too. That results in LINK arm-linux-user/qemu-arm ../crypto/tlssession.o: In function `qcrypto_tls_session_check_certificate': /home/rth/qemu/qemu/crypto/tlssession.c:356: undefined reference to `qauthz_is_allowed_by_id' collect2: error: ld returned 1 exit status which means all of the authz objects need to come in as well. I suppose they're not big, but still... I'm leaning toward reviving crypto-user-obj-y, with just the crypto random and aes objects in. Thoughts? r~
On Thu, May 16, 2019 at 07:48:30AM -0700, Richard Henderson wrote: > On 5/15/19 9:53 AM, Daniel P. Berrangé wrote: > > So I think we should just use $(crypto-obj-y) unconditionally in > > the user emulators, and get rid of crypto-aes-obj-y too. > > That results in > > LINK arm-linux-user/qemu-arm > ../crypto/tlssession.o: In function `qcrypto_tls_session_check_certificate': > /home/rth/qemu/qemu/crypto/tlssession.c:356: undefined reference to > `qauthz_is_allowed_by_id' > collect2: error: ld returned 1 exit status > > which means all of the authz objects need to come in as well. > I suppose they're not big, but still... > > I'm leaning toward reviving crypto-user-obj-y, with just the > crypto random and aes objects in. > > Thoughts? Coincidentally I think I need to split the $authz-obj-y variable into two parts authz-obj-y containing only base.o authz-obj-impl-y containing everything else Most things will thus be satisfied by just $authz-obj-y. Only the system emulators and qemu-nbd then need to have authz-obj-impl-y This will avoid qemu-img, qemu-io, and other tools and linux-user from having to link to the pam library. 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 :|
diff --git a/Makefile b/Makefile index 66d5c65156..524f2f8a57 100644 --- a/Makefile +++ b/Makefile @@ -411,6 +411,7 @@ dummy := $(call unnest-vars,, \ block-obj-m \ crypto-obj-y \ crypto-aes-obj-y \ + crypto-rng-obj-y \ qom-obj-y \ io-obj-y \ common-obj-y \ @@ -482,8 +483,9 @@ subdir-capstone: .git-submodule-status subdir-slirp: .git-submodule-status $(call quiet-command,$(MAKE) -C $(SRC_PATH)/slirp BUILD_DIR="$(BUILD_DIR)/slirp" CC="$(CC)" AR="$(AR)" LD="$(LD)" RANLIB="$(RANLIB)" CFLAGS="$(QEMU_CFLAGS) $(CFLAGS)" LDFLAGS="$(LDFLAGS)") -$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) \ - $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY)) +$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) $(qom-obj-y) \ + $(crypto-aes-obj-$(CONFIG_USER_ONLY)) \ + $(crypto-rng-obj-$(CONFIG_USER_ONLY)) ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS)) # Only keep -O and -g cflags diff --git a/Makefile.objs b/Makefile.objs index cf065de5ed..0c13ff47ea 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -26,6 +26,7 @@ block-obj-m = block/ crypto-obj-y = crypto/ crypto-aes-obj-y = crypto/ +crypto-rng-obj-y = crypto/ ####################################################################### # qom-obj-y is code used by both qemu system emulation and qemu-img diff --git a/Makefile.target b/Makefile.target index ae02495951..4e579a0a84 100644 --- a/Makefile.target +++ b/Makefile.target @@ -181,6 +181,7 @@ dummy := $(call unnest-vars,.., \ chardev-obj-y \ crypto-obj-y \ crypto-aes-obj-y \ + crypto-rng-obj-y \ qom-obj-y \ io-obj-y \ common-obj-y \ @@ -189,7 +190,7 @@ all-obj-y += $(common-obj-y) all-obj-y += $(qom-obj-y) all-obj-$(CONFIG_SOFTMMU) += $(authz-obj-y) all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y) $(chardev-obj-y) -all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y) +all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y) $(crypto-rng-obj-y) all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y) all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y) diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs index 256c9aca1f..ee7e628ca6 100644 --- a/crypto/Makefile.objs +++ b/crypto/Makefile.objs @@ -37,5 +37,6 @@ crypto-obj-y += block-luks.o # Let the userspace emulators avoid linking gnutls/etc crypto-aes-obj-y = aes.o +crypto-rng-obj-y = random-platform.o stub-obj-y += pbkdf-stub.o
For user-only, we require only the random number bits of the crypto subsystem. We need to preserve --static linking, which for many recent Linux distributions precludes using GnuTLS or GCrypt. Instead, use our random-platform module unconditionally. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- Makefile | 6 ++++-- Makefile.objs | 1 + Makefile.target | 3 ++- crypto/Makefile.objs | 1 + 4 files changed, 8 insertions(+), 3 deletions(-) -- 2.17.1