Re: [libvirt] Pass additional environmental variables

[Michal Privoznik]

On 12.08.2014 21:09, Sean Noonan wrote:
 We're using sasl+gssapi+kerberos to do authentication for libvirt, 
 including from hypervisor to hypervisor.  However, the environmental 
 variable filtering implemented in libvirt prevents this from working, so 
 we're forced to run a locally patched version.
 
 Thoughts on the following patch to pass the location of the local 
 credential cache as well?
 
 --- a/src/util/vircommand.c2014-01-07 14:14:11.388934108 +
 +++ b/src/util/vircommand.c2014-01-07 14:18:14.725082505 +
 @@ -1314,6 +1314,7 @@
 
   virCommandAddEnvPair(cmd, LC_ALL, C);
 
 +virCommandAddEnvPassBlockSUID(cmd, KRB5CCNAME, NULL);
   virCommandAddEnvPassBlockSUID(cmd, LD_PRELOAD, NULL);
   virCommandAddEnvPassBlockSUID(cmd, LD_LIBRARY_PATH, NULL);
   virCommandAddEnvPassBlockSUID(cmd, PATH, /bin:/usr/bin);

I've got some doubts whether this is the correct approach. This will pass the 
environment variable to every command spawned. Do we really want every command 
have access to kerberos tickets? On the other hand, we've done this for a 
limited use case:

commit afc984af2e055b33f7c7b06ccacaf00103d4ce5e
Author: Michal Privoznik mpriv...@redhat.com
AuthorDate: Fri Sep 9 15:59:26 2011 +0200
Commit: Michal Privoznik mpriv...@redhat.com
CommitDate: Fri Sep 9 15:59:26 2011 +0200

virnetsocket: Pass KRB5CCNAME env variable

So we can allow GSSAPI authentication for ssh.

Signed-off-by: Matthias Witte wi...@netzquadrat.de

patch-snip/

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 275b70d..88dc2a4 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -615,6 +615,7 @@ int virNetSocketNewConnectSSH(const char *nodename,
 
 cmd = virCommandNew(binary ? binary : ssh);
 virCommandAddEnvPassCommon(cmd);
+virCommandAddEnvPass(cmd, KRB5CCNAME);
 virCommandAddEnvPass(cmd, SSH_AUTH_SOCK);
 virCommandAddEnvPass(cmd, SSH_ASKPASS);
 virCommandAddEnvPass(cmd, DISPLAY);


Moreover, is KRB5CCNAME enough? What about KRB5_KTNAME?

And regarding the patch itself, I think you want to increase the preallocation 
constant too:

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index e775ba6..aa493cd 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -1312,7 +1312,7 @@ virCommandAddEnvPassCommon(virCommandPtr cmd)
 if (!cmd || cmd-has_error)
 return;
 
-if (VIR_RESIZE_N(cmd-env, cmd-maxenv, cmd-nenv, 9)  0) {
+if (VIR_RESIZE_N(cmd-env, cmd-maxenv, cmd-nenv, 10)  0) {
 cmd-has_error = ENOMEM;
 return;
 }
@@ -1326,6 +1326,7 @@ virCommandAddEnvPassCommon(virCommandPtr cmd)
 virCommandAddEnvPassAllowSUID(cmd, USER);
 virCommandAddEnvPassAllowSUID(cmd, LOGNAME);
 virCommandAddEnvPassBlockSUID(cmd, TMPDIR, NULL);
+virCommandAddEnvPassBlockSUID(cmd, KRB5CCNAME, NULL);
 }
 
 /**


Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Pass additional environmental variables

[Daniel P. Berrange]

On Wed, Aug 13, 2014 at 09:57:28AM +0200, Michal Privoznik wrote:
 On 12.08.2014 21:09, Sean Noonan wrote:
  We're using sasl+gssapi+kerberos to do authentication for libvirt, 
  including from hypervisor to hypervisor.  However, the environmental 
  variable filtering implemented in libvirt prevents this from working, so 
  we're forced to run a locally patched version.
  
  Thoughts on the following patch to pass the location of the local 
  credential cache as well?
  
  --- a/src/util/vircommand.c2014-01-07 14:14:11.388934108 +
  +++ b/src/util/vircommand.c2014-01-07 14:18:14.725082505 +
  @@ -1314,6 +1314,7 @@
  
virCommandAddEnvPair(cmd, LC_ALL, C);
  
  +virCommandAddEnvPassBlockSUID(cmd, KRB5CCNAME, NULL);
virCommandAddEnvPassBlockSUID(cmd, LD_PRELOAD, NULL);
virCommandAddEnvPassBlockSUID(cmd, LD_LIBRARY_PATH, NULL);
virCommandAddEnvPassBlockSUID(cmd, PATH, /bin:/usr/bin);
 
 I've got some doubts whether this is the correct approach. This will
 pass the environment variable to every command spawned. Do we really
 want every command have access to kerberos tickets? On the other hand,
 we've done this for a limited use case:

Yeah, the reason we block nearly all env from commands by default
is to ensure we have a known safe environment. So I'd prefer that
we only set KRB5CCNAME in the specific places that need it. Beyond
the ssh client we're spawning, which places need it ?  I could
see that we probably want it for auto-spawning of libvirtd itself

Regards,
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list