On Mon, 06 Jun 2016, Daniel P. Berrange wrote:
On Mon, Jun 06, 2016 at 04:34:09PM +0300, Alexander Bokovoy wrote:
On Mon, 06 Jun 2016, Daniel P. Berrange wrote:
> On Mon, Jun 06, 2016 at 09:01:10AM -0400, Marc-André Lureau wrote:
> > Hi
> >
> > ----- Original Message -----
> > > I'm sending Alexander Bokovoy's patch as it is, also here is some notes from
> > > him:
> > >
> > > "I'd really like to find a way to do it with pure SASL properties so that the
> > > code would work for both SPNEGO and Kerberos. SPNEGO NTLMSSP would make it
> > > working for environments where you don't have Kerberos but what we have
> > > right now should be fine for pure Kerberos environments like FreeIPA or
> > > Active Directory."
> > >
> > > And also his blog post:
> > > https://vda.li/en/posts/2016/05/30/Single-sign-on-to-virtual-machines/
> > >
> > > On one hand I think would be good to have this issue partially fixed (as per
> > > Alexander's comment) for 0.32, on the other hand I don't like calling these
> > > kerberos functions directly. Also, we probably would have to add a kerberos
> > > check/option on configure, right? I can do that without any problems, but I
> > > firstly would like to hear the opinions from other people in the project.
> >
> > Yes, it will have to be optional (especially because compiling krb5 on mingw is *hard* - last time I checked)
>
> Even compiling cryus-sasl is hard - indeed last I looked fedora didn't
> have any mingw packages for it.
>
> >
> > > I'm willing to re-work this patch after the release and try to find an ideal
> > > solution (if possible) and also spend some more time digging into the
> > > differences on handling this between gtk-vnc and spice-gtk.
> >
> > From his blog, I gathered that it worked with gtk-vnc but not with
> > spice-gtk. Why do we need krb specific code when gtk-vnc doesn't need it?
>
> It looks like the code is trying to set a default username based on the
> current kerberos credential the user has. gtk-vnc doesn't bother trying
> todo this - the user just always has to supply the username explicitly
> IMHO it would be fine for spice-gtk todo the same and avoid the krb dep/
I tried that. Let me get a bit deeper into details, though.
Cyrus SASL GSSAPI would work if you provide NULL username but the code
in spice-gtk rejects such usernames:
https://cgit.freedesktop.org/spice/spice-gtk/tree/src/spice-channel.c#n1390
Hmm, that code looks really rather wrong - it is clearly making a bogus
assumption that a NULL username will result in auth failure - it should
definitely be left upto the SASL library to decide that on the server
side.
On the client side, you mean.
I tried to allow NULL username here but the problem is that we need
eventually to set actual username so that SPICE communication can
continue. And if SASL GSSAPI module did find default credentials, we
need to pick up the username from them. This is possible theoretically
but all my attempts to do so caused SPICE server side to drop actual
SPICE connection.
I'm not sure what failure you just remove that check, but I think we
need to investigate that further, as I don't think that check for
NULL is right.
It is wrong, for sure.
Hm.. I retried again with a simple patch (attached) and it worked this
time.
--
/ Alexander Bokovoy
From 59c0c86c64144ccd43dadf5906df3f2e829a779e Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@xxxxxxxxxx>
Date: Mon, 6 Jun 2016 17:04:52 +0300
Subject: [PATCH] sasl: fix SASL GSSAPI by allowing NULL username
SASL GSSAPI module will try to negotiate authentication based on the
credentials in the default credentials cache. It does not matter if
SPICE knows username or not as SASL negotiation will pass through the
discovered name from the GSSAPI module.
Signed-off-by: Alexander Bokovoy <abokovoy@xxxxxxxxxx>
---
src/spice-channel.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/spice-channel.c b/src/spice-channel.c
index 19237b3..48a7570 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1385,11 +1385,10 @@ spice_channel_gather_sasl_credentials(SpiceChannel *channel,
switch (interact[ninteract].id) {
case SASL_CB_AUTHNAME:
case SASL_CB_USER:
- if (spice_session_get_username(c->session) == NULL)
- return FALSE;
-
- interact[ninteract].result = spice_session_get_username(c->session);
- interact[ninteract].len = strlen(interact[ninteract].result);
+ if (spice_session_get_username(c->session) != NULL) {
+ interact[ninteract].result = spice_session_get_username(c->session);
+ interact[ninteract].len = strlen(interact[ninteract].result);
+ }
break;
case SASL_CB_PASS:
--
2.7.4
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel