[MERGE] GSSAPI Authentication support for HTTP

Tue Feb 17 10:29:12 GMT 2009

>>>>> "Jelmer" == Jelmer Vernooij <jelmer at samba.org> writes:

    Jelmer> The attached patch adds support for WWW-Authenticate:
    Jelmer> Negotiate using Kerberos (GSSAPI).

Great.

    Jelmer> I've tested it here locally against a couple of
    Jelmer> servers, but it's hard to do automated tests, as
    Jelmer> setting up a KDC + mod_auth_kerberos is nontrivial.

You should try it via the lp:~vila/bzr/local_test_server plugin
anyway. That will document the steps you went through to get a
working Kerberos installation (and what means KDC ?).

    Jelmer> Since this code is only executed for
    Jelmer> "WWW-Authenticate: Negotiate", it can potentially
    Jelmer> only break in situations that didn't work previously
    Jelmer> anyway.

Which is a bit short of a reason to not test it :-)

Look at bzrlib.test.test_http.TestActivity for a way to test an
unmodified client against a server for which all responses are
pre-canned and bzrlib.test.test_http.TestAuth for tests currently
done for the supported authentication schemes.

Recording the session against a working http server with Kerberos
(with some random user/pass combo, but note that the tests use
joe/foo) should get most of the needed responses (AFAICT from the
client side you propose below).

If you encounter problems due to limitations in the canned
responses approach you may redefine some tests as being not
applicable until we get a proper test server (a local test server
counts as one :).

    Jelmer> At the moment, it's necessary to specify a username
    Jelmer> in the URL as bzr requires it for authentication,
    Jelmer> even though this is not required by
    Jelmer> Kerberos/GSSAPI. The specified username is ignored.

Patches welcome even if it implements only the part necessary to
make Negotiate remove that constraint while leaving it for the
other schemes.

    Jelmer> # Bazaar merge directive format 2 (Bazaar 0.90)
    Jelmer> # revision_id: jelmer at samba.org-20090217015430-62v560na6f1ngybp
    Jelmer> # target_branch: http://bazaar-vcs.org/bzr/bzr.dev
    Jelmer> # testament_sha1: c22b4672fb24f1beef348033eff85d841b22cb97
    Jelmer> # timestamp: 2009-02-17 02:55:39 +0100
    Jelmer> # base_revision_id: pqm at pqm.ubuntu.com-20090216172448-vj35mjoe463c3bk2
    Jelmer> # 
    Jelmer> # Begin patch
    Jelmer> === modified file 'NEWS'
    Jelmer> --- NEWS	2009-02-15 22:56:22 +0000
    Jelmer> +++ NEWS	2009-02-17 01:54:30 +0000
    Jelmer> @@ -27,6 +27,8 @@
    Jelmer>        generation of a working tree in the new branch.
    Jelmer>        (Daniel Watkins, John Klinger, #273993)

    Jelmer> +    * Support for GSSAPI authentication when using HTTP. (Jelmer Vernooij)
    Jelmer> +

Does it work for https too ? If so, it's worth mentioning (and
testing :-).

<snip/>

    Jelmer> === modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
    Jelmer> --- bzrlib/transport/http/_urllib2_wrappers.py	2009-02-09 18:25:43 +0000
    Jelmer> +++ bzrlib/transport/http/_urllib2_wrappers.py	2009-02-17 01:54:30 +0000
    Jelmer> @@ -943,7 +943,7 @@
    Jelmer>      preventively set authentication headers after the first
    Jelmer>      successful authentication.

    Jelmer> -    This can be used for http and proxy, as well as for basic and
    Jelmer> +    This can be used for http and proxy, as well as for basic, negotiate and
    Jelmer>      digest authentications.

    Jelmer>      This provides an unified interface for all authentication handlers
    Jelmer> @@ -1143,6 +1143,63 @@
    Jelmer>      https_request = http_request # FIXME: Need test

    Jelmer> +class NegotiateAuthHandler(AbstractAuthHandler):
    Jelmer> +    """A authentication handler that handles WWW-Authenticate: Negotiate.
    Jelmer> +
    Jelmer> +    At the moment this handler supports just Kerberos. In the future, 
    Jelmer> +    NTLM support may also be added.
    Jelmer> +    """
    Jelmer> +
    Jelmer> +    handler_order = 480
    Jelmer> +
    Jelmer> +    auth_regexp = re.compile('realm="([^"]*)"', re.I)
    Jelmer> +
    Jelmer> +    def auth_match(self, header, auth):
    Jelmer> +        try:
    Jelmer> +            scheme, raw_auth = header.split(None, 1)
    Jelmer> +        except ValueError:
    Jelmer> +            scheme = header
    Jelmer> +            raw_auth = ""
    Jelmer> +        scheme = scheme.lower()
    Jelmer> +        if scheme != 'negotiate':
    Jelmer> +            return False
    Jelmer> +        match = self.auth_regexp.search(raw_auth)
    Jelmer> +        if match:
    Jelmer> +            self.update_auth(auth, 'realm', match.groups()[0])
    Jelmer> +        self.update_auth(auth, 'scheme', scheme)
    Jelmer> +        resp = self._auth_match_kerberos(auth)
    Jelmer> +        if resp is None:
    Jelmer> +            return False
    Jelmer> +        # Optionally should try to authenticate using NTLM here
    Jelmer> +        self.update_auth(auth, 'negotiate_response', resp)
    Jelmer> +        return True
    Jelmer> +
    Jelmer> +    def _auth_match_kerberos(self, auth):
    Jelmer> +        """Try to create a GSSAPI response for authenticating against a host."""
    Jelmer> +        try:
    Jelmer> +            import kerberos
    Jelmer> +        except ImportError:
    Jelmer> +            return None

That would be better done at the module level (setting some flag
on import errors) so that the import is attempted only once
instead of once by *connection*.

    Jelmer> +        ret, vc = kerberos.authGSSClientInit("HTTP@%(host)s" % auth)

Is it valid for https too ?

    Jelmer> +        if ret < 1:
    Jelmer> +            trace.warning('Unable to create GSSAPI context for %(host)s', auth)
    Jelmer> +            return None

What kind of error is that ? Shouldn't we give more feedback about
some bad credentials is it just some internal check that should
cover possible bugs in the implementation or server setup ?

Or is it just the way you detect that the server implements
Kerberos scheme ?

    Jelmer> +        ret = kerberos.authGSSClientStep(vc, "")
    Jelmer> +        if ret < 0:
    Jelmer> +            trace.mutter('authGSSClientStep failed: %d', ret)
    Jelmer> +            return None
    Jelmer> +        return kerberos.authGSSClientResponse(vc)
    Jelmer> +
    Jelmer> +    def build_auth_header(self, auth, request):
    Jelmer> +        return "Negotiate %s" % auth['negotiate_response']
    Jelmer> +
    Jelmer> +    def auth_params_reusable(self, auth):
    Jelmer> +        # If the auth scheme is known, it means a previous
    Jelmer> +        # authentication was successful, all information is
    Jelmer> +        # available, no further checks are needed.
    Jelmer> +        return auth.get('scheme', None) == 'negotiate'

Shouldn't you test the 'negotiate_response' here to ensure you
really authenticate before ?

In summary: the patch is warmly welcome, but since you have *now*
the knowledge and a working setup, it will be a shame to not
capture that knowledge in the tests, so:

BB:resubmit

       Vincent