Support Kerberos on Android

net/http/http_auth_handler_negotiate_parse.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/http/http_auth_handler_negotiate_parse.h"
+
+#include "base/base64.h"
+#include "base/strings/string_util.h"
+#include "net/http/http_auth_challenge_tokenizer.h"
+
+namespace net {
+
+namespace {
+
+bool schemeValid(const std::string& scheme,

[Ryan Sleevi, 2015/06/29 13:56:45]

STYLE: This is not a valid function name (drop-caps first aren't legit)

Perhaps SchemeIsValid() ?

[Ryan Sleevi, 2015/06/29 13:56:45]

document

[aberent, 2015/07/02 21:13:36]

Done.

[aberent, 2015/07/02 21:13:36]

Done.

+                 HttpAuthChallengeTokenizer* challenge) {
+  // There is no guarantee that challenge->scheme() is valid ASCII, but
+  // LowerCaseEqualsASCII will do the right thing even if it isn't.
+  return base::LowerCaseEqualsASCII(challenge->scheme(),

[Ryan Sleevi, 2015/06/29 13:56:45]

DESIGN: Should you be using base::strcasecmp / strncasecmp to compare these?
What are the implications of the embedded NULs?

Specifically: What can you do to avoid unnecessary copying of strings here?

[aberent, 2015/07/02 21:13:36]

embedded NULs are actually a good reason not to use str(n)casecmp. str(n)casecmp
would be terminated by an embedded null in the challenge's scheme, potentially
giving a false match, whereas LowerCaseEqualsASCII won't be.

There shouldn't be embedded nulls in |scheme| since it always comes from
constants in our code.
Not much, without changing the implementation of, or writing our own version of,
LowerCaseEqualsASCII. The copy is, however, unlikely to be a significant
performance problem since (at present) scheme is at most 9 characters (it is
either "Negotiate" or "NTLM"), and is called at most once per HTML transaction.

I have changed the arguments to the public members to take the scheme as const
std::string& to avoid a truly unnecessary copy.

+                                    base::StringToLowerASCII(scheme).c_str());

[Ryan Sleevi, 2015/06/29 13:56:45]

DESIGN: Should you be passing scheme as std::string, or is this better passed as
"const char*"

[aberent, 2015/07/02 21:13:36]

Might be better if it were changed everywhere, but the existing calling code for
Posix and Windows passes it around as a std::string through a number of layers,
so there is no advantage in just changing it here. Changing it everywhere is
beyond the scope of this CL.

Also, as far as I can see, we don't currently have a version of
StringToLowerASCII that takes a char*.

+}
+
+}  // namespace
+
+HttpAuth::AuthorizationResult ParseFirstNegotiateChallenge(
+    std::string scheme,
+    HttpAuthChallengeTokenizer* challenge) {
+  // Verify the challenge's auth-scheme.
+  if (!schemeValid(scheme, challenge))
+    return HttpAuth::AUTHORIZATION_RESULT_INVALID;
+
+  std::string encoded_auth_token = challenge->base64_param();
+  if (encoded_auth_token.empty()) {
+    return HttpAuth::AUTHORIZATION_RESULT_ACCEPT;
+  } else {

[Ryan Sleevi, 2015/06/29 13:56:45]

no else after return

[aberent, 2015/07/02 21:13:36]

Done.

+    return HttpAuth::AUTHORIZATION_RESULT_INVALID;
+  }
+}
+
+HttpAuth::AuthorizationResult ParseAnotherNegotiateChallenge(
+    std::string scheme,
+    HttpAuthChallengeTokenizer* challenge,
+    std::string* encoded_token,
+    std::string* decoded_token) {
+  // Verify the challenge's auth-scheme.
+  if (!schemeValid(scheme, challenge))
+    return HttpAuth::AUTHORIZATION_RESULT_INVALID;
+
+  *encoded_token = challenge->base64_param();
+  if (encoded_token->empty())
+    return HttpAuth::AUTHORIZATION_RESULT_REJECT;
+
+  // Make sure the additional token is base64 encoded.
+  if (!base::Base64Decode(*encoded_token, decoded_token))
+    return HttpAuth::AUTHORIZATION_RESULT_INVALID;
+  return HttpAuth::AUTHORIZATION_RESULT_ACCEPT;
+}
+
+}  // namespace net