Re: [Qemu-devel] [PATCH v7 01/24] build: Link user-only with crypto-rng-obj-y

2019-05-16 Thread Richard Henderson
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

  LINKarm-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~



Re: [Qemu-devel] [PATCH v7 01/24] build: Link user-only with crypto-rng-obj-y

2019-05-16 Thread Daniel P . Berrangé
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
> 
>   LINKarm-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 :|



Re: [Qemu-devel] [PATCH v7 01/24] build: Link user-only with crypto-rng-obj-y

2019-05-15 Thread Daniel P . Berrangé
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 :|



Re: [Qemu-devel] [PATCH v7 01/24] build: Link user-only with crypto-rng-obj-y

2019-05-15 Thread Laurent Vivier
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



Re: [Qemu-devel] [PATCH v7 01/24] build: Link user-only with crypto-rng-obj-y

2019-05-15 Thread Daniel P . Berrangé
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 :|



Re: [Qemu-devel] [PATCH v7 01/24] build: Link user-only with crypto-rng-obj-y

2019-05-15 Thread Richard Henderson
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~



Re: [Qemu-devel] [PATCH v7 01/24] build: Link user-only with crypto-rng-obj-y

2019-05-15 Thread Daniel P . Berrangé
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 
> ---
>  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 :|



Re: [Qemu-devel] [PATCH v7 01/24] build: Link user-only with crypto-rng-obj-y

2019-05-15 Thread Richard Henderson
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 
> 
>> Signed-off-by: Richard Henderson 
>> ---
>>  Makefile | 6 --
>>  Makefile.objs| 1 +
>>  Makefile.target  | 3 ++-
>>  crypto/Makefile.objs | 1 +
>>  4 files changed, 8 insertions(+), 3 deletions(-)




Re: [Qemu-devel] [PATCH v7 01/24] build: Link user-only with crypto-rng-obj-y

2019-05-15 Thread Laurent Vivier
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 

> Signed-off-by: Richard Henderson 
> ---
>  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
>