Re: [Spice-devel] [spice-gtk] Support SASL GSSAPI

2016-06-08 Thread Alexander Bokovoy 

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 
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 
---
 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk] Support SASL GSSAPI

2016-06-06 Thread Fabiano Fidêncio 
On Mon, Jun 6, 2016 at 4:07 PM, Alexander Bokovoy  wrote:
> 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.

Nice, I really like the patch.
You have an ACK from me and if we don't have any objections in the
next days I'll push your patch _before_ the 0.32 release.

>
> --
> / Alexander Bokovoy

Thanks for patch and best regards,
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk] Support SASL GSSAPI

2016-06-06 Thread Daniel P. Berrange 
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.

> 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.


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 :|
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk] Support SASL GSSAPI

2016-06-06 Thread Daniel P. Berrange 
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/

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 :|
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk] Support SASL GSSAPI

2016-06-06 Thread Fabiano Fidêncio 
On Mon, Jun 6, 2016 at 3:01 PM, 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)

Currently we build mingw-spice-gtk with --without-sasl (both fedora
and downstream). So, it won't be that problematic.

>
>> 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?

Yeah, that's part of my plan to setup the environment and dig into it
as soon as I have time for it.

>
>>
>> Please, as I'm not whether Alexander is subscribed to our mailing list or
>> not,
>> let's keep him CC'ed for any further interaction.
>>
>
> Thanks again Alexander

Best Regards,
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk] Support SASL GSSAPI

2016-06-06 Thread Marc-André Lureau 
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)

> 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?

> 
> Please, as I'm not whether Alexander is subscribed to our mailing list or
> not,
> let's keep him CC'ed for any further interaction.
> 

Thanks again Alexander
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk] Support SASL GSSAPI

2016-06-06 Thread Fabiano Fidêncio 
On Mon, Jun 6, 2016 at 2:51 PM, Fabiano Fidêncio  wrote:
> 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.

Alexander just pointed out (on #freeIPA channel) that we don't need the kerberos
checks as these come to us via Cyrus-SASL already.

>
> 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.
>
> Please, as I'm not whether Alexander is subscribed to our mailing list or not,
> let's keep him CC'ed for any further interaction.
>
> Alexander Bokovoy (1):
>   spice-channel: support SASL GSSAPI
>
>  src/spice-channel.c | 61 
> +
>  1 file changed, 57 insertions(+), 4 deletions(-)
>
> --
> 2.7.4
>

Best Regards,
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk] Support SASL GSSAPI

2016-06-06 Thread Fabiano Fidêncio 
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.

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.

Please, as I'm not whether Alexander is subscribed to our mailing list or not,
let's keep him CC'ed for any further interaction.

Alexander Bokovoy (1):
  spice-channel: support SASL GSSAPI

 src/spice-channel.c | 61 +
 1 file changed, 57 insertions(+), 4 deletions(-)

-- 
2.7.4

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel