[PATCH] winbind: Return error for failed PAC signature verification

[Andrew Bartlett]

On Wed, 2014-05-07 at 15:11 -0700, Christof Schmitt wrote:
> On Tue, May 06, 2014 at 11:03:08AM +1200, Andrew Bartlett wrote:
> > On Mon, 2014-05-05 at 14:48 -0700, Christof Schmitt wrote:
> > > On Sat, May 03, 2014 at 05:23:05PM +1200, Andrew Bartlett wrote:
> > > > On Fri, 2014-05-02 at 14:54 -0700, Christof Schmitt wrote:
> > > > > This is a follow-up to the discussion from July last year
> > > > > (https://lists.samba.org/archive/samba-technical/2012-July/thread.html#85283).
> > > > > 
> > > > > While looking at the winbind interface to decode the PAC again, i
> > > > > started thinking if we need to return the failed PAC signature
> > > > > verification back to the caller. A client with a valid kerberos ticket
> > > > > could generate its own PAC and authenticate to an application using the
> > > > > winbind PAC interface. If winbind does not return the failed signature
> > > > > verification, then the application could rely on false data. Based on
> > > > > this, it seems that it is better to return an error instead of untrusted
> > > > > data.
> > > > 
> > > > Shouldn't this have been done by the library that extracted the PAC from
> > > > the ticket?  (In Samba, we rely on exactly that in the two gensec gssapi
> > > > modules).
> > > 
> > > That would be useful, i have to check how that could be done.
> > 
> > The gssapi_obtain_pac_blob code() in auth/kerberos/gssapi_pac.c shows
> > you how to do it.  
> 
> I took a look at that code, but i don't have much insight into GSS.
> Retrieving the PAC from gss_get_name_attribute() and checking the
> 'authenticated' parameter should be enough?

Yes.

> > > > We can't check the PAC signature unless it used the same key we have in
> > > > winbindd.
> > > > 
> > > > Selecting the right key appears trivial at first, but is actually more
> > > > complex than it looks, and only the libkrb5 that did the
> > > > gss_accept_security_context() really knows, everyone else has to just
> > > > try all plausible keys. 
> > > 
> > > Yes, understood. The related issue i see is that there is no indication
> > > to the caller if the PAC information has been stored or not (according
> > > to the result of the signature verification). It would be useful to
> > > provide some status back to the caller. Failing the request with an
> > > error code is just a big hammer to do this.
> > 
> > Sounds reasonable, but I don't think it should fail.  I realise we need
> > the side-effect of loading the cache for the desired result (getting the
> > groups) to work.
> 
> I don't see a good way to return an extra flag or error code for this
> case in struct winbindd_response.

You may need to extend the union for the winbindd_response.

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba