[Spice-devel] [spice-gtk v2] sasl: fix SASL GSSAPI by allowing NULL username

[Pavel Grunt]

On Tue, 2016-06-07 at 11:25 +0300, Alexander Bokovoy wrote:
> On Tue, 07 Jun 2016, Pavel Grunt wrote:
> > Hi,
> > 
> > On Mon, 2016-06-06 at 18:04 +0200, Fabiano Fidêncio wrote:
> > > From: Alexander Bokovoy <abokovoy at redhat.com>
> > > 
> > > 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.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1343361
> > > 
> > > Signed-off-by: Alexander Bokovoy <abokovoy at redhat.com>
> > > Acked-by: Fabiano Fidêncio <fidencio at redhat.com>
> > > ---
> > > Sending the patch to the ML for the record.
> > > I already ACKed the patch and anyone objects I'll push it Tomorrow.
> > > ---
> > >  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 c6e548d..0eb0e61 100644
> > > --- a/src/spice-channel.c
> > > +++ b/src/spice-channel.c
> > > @@ -1387,11 +1387,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;
> > so few lines above 'c->auth_needs_username = TRUE' is set, but it is ok to
> > ignore the missing username ? It is really confusing for me.
> > 
> > How does it affect info about auth failure provided by
> > spice_channel_failed_authentication() ?
> If c->auth_needs_username is set, spice_channel_failed_authentication()
> will tell that a username is required. This is certainly true -- if SASL
> GSSAPI failed, username/password are indeed required. So it wouldn't be
> a problem, at least from my reading of the code and tests with spicy
> tool.
> 
> However, there is a problem with cases like virt-manager which assumes
> there is only a password per channel required and never shows you a
> request to enter username in case of SASL GSSAPI failure. When you enter
> a password, the underlying code would complain that username is missing
> but no way to enter username would be provided. This is virt-manager's
> issue
Is there a bug ?
> , not spice-gtk.
> 

Thank you for the explanation, I think the patch is ok to go in.

Thanks,
Pavel

(not related to the patch - it would be nice to rewrite/clean up the "SASL" part
of spice-channel.c)