chromium/src
0
Fork 0
Code Activity

Remove allow_unsafe_buffers from HTTP auth code.

Browse Source 
This does unfortunately add an UNSAFE_BUFFERS() call to
http_auth_sspi_win, as it calls a Windows API that expects an input
data buffer of variable size to immediately follow a
SEC_CHANNEL_BINDINGS object.

Bug: 40284755
Change-Id: I4e0ef78dfeaa9d3ac8c180bfbd1a20619b4aab9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6386746
Commit-Queue: mmenke <mmenke@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1438415}
This commit is contained in:
Matt Menke
2025-03-26 14:45:16 -07:00
committed by Chromium LUCI CQ
parent 8d5ab72bfe
commit ee8af1f4e4
3 changed files with 40 additions and 48 deletions

52
net/http/http_auth_gssapi_posix.cc

@ -2,11 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifdef UNSAFE_BUFFERS_BUILD
// TODO(crbug.com/40284755): Remove this and spanify to fix the errors.
#pragma allow_unsafe_buffers
#endif
#include "net/http/http_auth_gssapi_posix.h"
#include <limits>
@ -326,10 +321,10 @@ base::Value::Dict GetContextStateAsValue(GSSAPILibrary* gssapi_lib,
namespace {
// Return a NetLog value for the result of loading a library.
base::Value::Dict LibraryLoadResultParams(std::string_view library_name,
base::Value::Dict LibraryLoadResultParams(const base::FilePath& library_name,
std::string_view load_result) {
base::Value::Dict params;
params.Set("library_name", library_name);
params.Set("library_name", library_name.value());
if (!load_result.empty())
params.Set("load_result", load_result);
return params;
@ -364,49 +359,44 @@ bool GSSAPISharedLibrary::InitImpl(const NetLogWithSource& net_log) {
base::NativeLibrary GSSAPISharedLibrary::LoadSharedLibrary(
const NetLogWithSource& net_log) {
const char* const* library_names;
size_t num_lib_names;
const char* user_specified_library[1];
std::vector<base::FilePath> library_names;
if (!gssapi_library_name_.empty()) {
user_specified_library[0] = gssapi_library_name_.c_str();
library_names = user_specified_library;
num_lib_names = 1;
library_names.emplace_back(gssapi_library_name_);
} else {
static const char* const kDefaultLibraryNames[] = {
#if BUILDFLAG(IS_APPLE)
"/System/Library/Frameworks/GSS.framework/GSS"
library_names.emplace_back("/System/Library/Frameworks/GSS.framework/GSS");
#elif BUILDFLAG(IS_OPENBSD)
"libgssapi.so" // Heimdal - OpenBSD
// Heimdal - OpenBSD
library_names.emplace_back("libgssapi.so");
#else
"libgssapi_krb5.so.2", // MIT Kerberos - FC, Suse10, Debian
"libgssapi.so.4", // Heimdal - Suse10, MDK
"libgssapi.so.2", // Heimdal - Gentoo
"libgssapi.so.1" // Heimdal - Suse9, CITI - FC, MDK, Suse10
// MIT Kerberos - FC, Suse10, Debian
library_names.emplace_back("libgssapi_krb5.so.2");
// Heimdal - Suse10, MDK
library_names.emplace_back("libgssapi.so.4");
// Heimdal - Gentoo
library_names.emplace_back("libgssapi.so.2");
// Heimdal - Suse9, CITI - FC, MDK, Suse10
library_names.emplace_back("libgssapi.so.1");
#endif
};
library_names = kDefaultLibraryNames;
num_lib_names = std::size(kDefaultLibraryNames);
}
net_log.BeginEvent(NetLogEventType::AUTH_LIBRARY_LOAD);
// There has to be at least one candidate.
DCHECK_NE(0u, num_lib_names);
CHECK(!library_names.empty());
const char* library_name = nullptr;
base::NativeLibraryLoadError load_error;
for (size_t i = 0; i < num_lib_names; ++i) {
for (const auto& library_name : library_names) {
load_error = base::NativeLibraryLoadError();
library_name = library_names[i];
base::FilePath file_path(library_name);
// TODO(asanka): Move library loading to a separate thread.
// http://crbug.com/66702
base::ScopedAllowBlocking scoped_allow_blocking_temporarily;
base::NativeLibrary lib = base::LoadNativeLibrary(file_path, &load_error);
base::NativeLibrary lib =
base::LoadNativeLibrary(library_name, &load_error);
if (lib) {
if (BindMethods(lib, library_name, net_log)) {
if (BindMethods(lib, library_name.value(), net_log)) {
net_log.EndEvent(NetLogEventType::AUTH_LIBRARY_LOAD, [&] {
return LibraryLoadResultParams(library_name, "");
});
@ -421,7 +411,7 @@ base::NativeLibrary GSSAPISharedLibrary::LoadSharedLibrary(
// library. Doing so also always logs the failure when the GSSAPI library
// name is explicitly specified.
net_log.EndEvent(NetLogEventType::AUTH_LIBRARY_LOAD, [&] {
return LibraryLoadResultParams(library_name, load_error.ToString());
return LibraryLoadResultParams(library_names.back(), load_error.ToString());
});
return nullptr;
}

19
net/http/http_auth_handler_digest.cc

@ -2,13 +2,9 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifdef UNSAFE_BUFFERS_BUILD
// TODO(crbug.com/40284755): Remove this and spanify to fix the errors.
#pragma allow_unsafe_buffers
#endif
#include "net/http/http_auth_handler_digest.h"
#include <array>
#include <string>
#include <string_view>
@ -20,6 +16,7 @@
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "crypto/random.h"
#include "net/base/features.h"
#include "net/base/net_errors.h"
#include "net/base/net_string_util.h"
@ -69,12 +66,18 @@ HttpAuthHandlerDigest::DynamicNonceGenerator::DynamicNonceGenerator() = default;
std::string HttpAuthHandlerDigest::DynamicNonceGenerator::GenerateNonce()
const {
// This is how mozilla generates their cnonce -- a 16 digit hex string.
static const char domain[] = "0123456789abcdef";
std::array<uint8_t, 8> rand_bytes;
crypto::RandBytes(rand_bytes);
std::string cnonce;
cnonce.reserve(16);
for (int i = 0; i < 16; ++i) {
cnonce.push_back(domain[base::RandInt(0, 15)]);
for (const uint8_t byte : rand_bytes) {
// It shouldn't matter whether this is capitalized or not, but safest to
// preserve behavior of using lowercase hex strings.
base::AppendHexEncodedByte(byte, cnonce, /*uppercase=*/false);
}
DCHECK_EQ(cnonce.size(), 16u);
return cnonce;
}

17
net/http/http_auth_sspi_win.cc

@ -2,17 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifdef UNSAFE_BUFFERS_BUILD
// TODO(crbug.com/40284755): Remove this and spanify to fix the errors.
#pragma allow_unsafe_buffers
#endif
// See "SSPI Sample Application" at
// http://msdn.microsoft.com/en-us/library/aa918273.aspx
#include "net/http/http_auth_sspi_win.h"
#include "base/base64.h"
#include "base/compiler_specific.h"
#include "base/logging.h"
#include "base/notreached.h"
#include "base/strings/string_util.h"
@ -486,11 +482,12 @@ int HttpAuthSSPI::GetNextSecurityToken(const std::string& spn,
CtxtHandle* ctxt_ptr = nullptr;
SecBufferDesc in_buffer_desc, out_buffer_desc;
SecBufferDesc* in_buffer_desc_ptr = nullptr;
SecBuffer in_buffers[2], out_buffer;
std::array<SecBuffer, 2> in_buffers;
SecBuffer out_buffer;
in_buffer_desc.ulVersion = SECBUFFER_VERSION;
in_buffer_desc.cBuffers = 0;
in_buffer_desc.pBuffers = in_buffers;
in_buffer_desc.pBuffers = in_buffers.data();
if (in_token_len > 0) {
// Prepare input buffer.
SecBuffer& sec_buffer = in_buffers[in_buffer_desc.cBuffers++];
@ -512,9 +509,11 @@ int HttpAuthSSPI::GetNextSecurityToken(const std::string& spn,
sec_channel_bindings_buffer.reserve(sizeof(SEC_CHANNEL_BINDINGS) +
channel_bindings.size());
sec_channel_bindings_buffer.resize(sizeof(SEC_CHANNEL_BINDINGS));
// SAFETY: `sec_channel_bindings_buffer` was allocated to be long enough to
// hold a SEC_CHANNEL_BINDINGS object above.
SEC_CHANNEL_BINDINGS* bindings_desc =
reinterpret_cast<SEC_CHANNEL_BINDINGS*>(
sec_channel_bindings_buffer.data());
UNSAFE_BUFFERS(reinterpret_cast<SEC_CHANNEL_BINDINGS*>(
sec_channel_bindings_buffer.data()));
bindings_desc->cbApplicationDataLength = channel_bindings.size();
bindings_desc->dwApplicationDataOffset = sizeof(SEC_CHANNEL_BINDINGS);
sec_channel_bindings_buffer.insert(sec_channel_bindings_buffer.end(),