[libvirt] [PATCH] build: fix build of virt-login-shell on systems with older gnutls

On systems where gnutls uses libgcrypt, I'm seeing the following build failure libvirt.c:314: error: variable 'virTLSThreadImpl' has initializer but incomplete type libvirt.c:319: error: 'GCRY_THREAD_OPTION_PTHREAD' undeclared here (not in a function) ... Fix by undefining WITH_GNUTLS_GCRYPT in config-post.h Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- Although I'm more confident in this fix vs the previously reported virt-login-shell build failure, I'd still prefer an ACK before pushing. config-post.h | 1 + 1 file changed, 1 insertion(+) diff --git a/config-post.h b/config-post.h index d371e8c..8367200 100644 --- a/config-post.h +++ b/config-post.h @@ -34,6 +34,7 @@ # undef WITH_CURL # undef WITH_DTRACE_PROBES # undef WITH_GNUTLS +# undef WITH_GNUTLS_GCRYPT # undef WITH_MACVTAP # undef WITH_NUMACTL # undef WITH_SASL -- 1.8.1.4

On 10/22/2013 06:29 AM, Jim Fehlig wrote:
On systems where gnutls uses libgcrypt, I'm seeing the following build failure
libvirt.c:314: error: variable 'virTLSThreadImpl' has initializer but incomplete type libvirt.c:319: error: 'GCRY_THREAD_OPTION_PTHREAD' undeclared here (not in a function) ...
Fix by undefining WITH_GNUTLS_GCRYPT in config-post.h
Signed-off-by: Jim Fehlig <jfehlig@suse.com> ---
Although I'm more confident in this fix vs the previously reported virt-login-shell build failure, I'd still prefer an ACK before pushing.
I'm not sure on the other one yet, but can definitely ACK this one. (Oh, and it's a pity that gnutls is now effectively LGPLv3+ if it uses libnettle which in turn uses LGPLv3+ libgmp - because that makes libvirt unusable with GPLv2-only programs like Oz on platforms with newer gnutls. At least your situation with older gnutls is not impacted, because things are still LGPLv2+) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Eric Blake wrote:
On 10/22/2013 06:29 AM, Jim Fehlig wrote:
On systems where gnutls uses libgcrypt, I'm seeing the following build failure
libvirt.c:314: error: variable 'virTLSThreadImpl' has initializer but incomplete type libvirt.c:319: error: 'GCRY_THREAD_OPTION_PTHREAD' undeclared here (not in a function) ...
Fix by undefining WITH_GNUTLS_GCRYPT in config-post.h
Signed-off-by: Jim Fehlig <jfehlig@suse.com> ---
Although I'm more confident in this fix vs the previously reported virt-login-shell build failure, I'd still prefer an ACK before pushing.
I'm not sure on the other one yet, but can definitely ACK this one.
Thanks, I pushed this one. Michal pushed the other one that Daniel ACKed. One thing that concerned me, which I forgot to mention in the mail or commit message, was this comment above libvirt_setuid_rpc_client in src/Makefile.am # Since virt-login-shell will be setuid, we must do everything # we can to avoid linking to other libraries. Regards, Jim

On 10/22/2013 04:59 PM, Jim Fehlig wrote:
Michal pushed the other one that Daniel ACKed. One thing that concerned me, which I forgot to mention in the mail or commit message, was this comment above libvirt_setuid_rpc_client in src/Makefile.am
# Since virt-login-shell will be setuid, we must do everything # we can to avoid linking to other libraries.
We can't eliminate every library, but we can assume that some very basic libraries are capable of being safely used in setuid apps (selinux, apparmor, and libc among that list). Meanwhile, we do have proof that other libraries are not so friendly; among them, Daniel analyzed using LD_PRELOAD that at least libnspr.so (used by libcurl.so) has at least one unprotected getenv() within a constructor that could be used merely by loading the shared library into memory as a way to cause an overwrite of an unintended file in a setuid app. So maybe the comment could be tweaked to say "we avoid linking to all but very basic and well-audited libraries". -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Tue, Oct 22, 2013 at 09:42:44PM +0100, Eric Blake wrote:
On 10/22/2013 04:59 PM, Jim Fehlig wrote:
Michal pushed the other one that Daniel ACKed. One thing that concerned me, which I forgot to mention in the mail or commit message, was this comment above libvirt_setuid_rpc_client in src/Makefile.am
# Since virt-login-shell will be setuid, we must do everything # we can to avoid linking to other libraries.
We can't eliminate every library, but we can assume that some very basic libraries are capable of being safely used in setuid apps (selinux, apparmor, and libc among that list).
Meanwhile, we do have proof that other libraries are not so friendly; among them, Daniel analyzed using LD_PRELOAD that at least libnspr.so (used by libcurl.so) has at least one unprotected getenv() within a constructor that could be used merely by loading the shared library into memory as a way to cause an overwrite of an unintended file in a setuid app. So maybe the comment could be tweaked to say "we avoid linking to all but very basic and well-audited libraries".
The goal of virt-login-shell was to avoid linking to every library except those strictly required for its work. This boiled down to libxml and libselinux. It makes sense that libselinux would be replaced by libapparmour on relevant distros. If you want to be doubly paranoid though, look at the libapparmour sources and verify that there is no function that is marked with __attribute__((constructor)). If there is such a function, then it should be audited to make sure its code is safe, in particular wrt environment variables. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Mon, Oct 21, 2013 at 11:29:33PM -0600, Jim Fehlig wrote:
On systems where gnutls uses libgcrypt, I'm seeing the following build failure
libvirt.c:314: error: variable 'virTLSThreadImpl' has initializer but incomplete type libvirt.c:319: error: 'GCRY_THREAD_OPTION_PTHREAD' undeclared here (not in a function) ...
Fix by undefining WITH_GNUTLS_GCRYPT in config-post.h
Signed-off-by: Jim Fehlig <jfehlig@suse.com> ---
Although I'm more confident in this fix vs the previously reported virt-login-shell build failure, I'd still prefer an ACK before pushing.
config-post.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/config-post.h b/config-post.h index d371e8c..8367200 100644 --- a/config-post.h +++ b/config-post.h @@ -34,6 +34,7 @@ # undef WITH_CURL # undef WITH_DTRACE_PROBES # undef WITH_GNUTLS +# undef WITH_GNUTLS_GCRYPT # undef WITH_MACVTAP # undef WITH_NUMACTL # undef WITH_SASL
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Jim Fehlig