Skip to content

Commit

Permalink
Add cert_is_issued_by_known_root field in net::TransportInfo
Browse files Browse the repository at this point in the history
We are planning to use this field to decide whether to use stored
compression dictionaries.

So this CL adds cert_is_issued_by_known_root field in TransportInfo. So
that the ConnectedCallback can check it.

This CL also changes HttpBasicStream::GetSSLInfo() and
WebSocketBasicHandshakeStream::GetSSLInfo() not to use
HttpStreamParser::GetSSLInfo() because we want call these methods before
initializing the HttpStreamParser.

HttpNetworkTransaction::DoConnectedCallback() is called before
HttpNetworkTransaction::DoInitStream() initializes the HttpStreamParser.

  - HttpNetworkTransaction::DoConnectedCallback()
  - HttpNetworkTransaction::DoInitStream()
    -> HttpBasicStream::InitializeStream()
      -> HttpBasicState::Initialize()
        -> std::make_unique<HttpStreamParser>()

Bug: 1413922
Change-Id: I2b2064304b9192eea61a854044e5d1c526af1f51
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5035761
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1226633}
  • Loading branch information
horo-t authored and Chromium LUCI CQ committed Nov 19, 2023
1 parent e3d513f commit 0c9d008
Show file tree
Hide file tree
Showing 11 changed files with 48 additions and 36 deletions.
17 changes: 7 additions & 10 deletions net/base/transport_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,12 @@ TransportInfo::TransportInfo() = default;

TransportInfo::TransportInfo(TransportType type_arg,
IPEndPoint endpoint_arg,
std::string accept_ch_frame_arg)
std::string accept_ch_frame_arg,
bool cert_is_issued_by_known_root)
: type(type_arg),
endpoint(std::move(endpoint_arg)),
accept_ch_frame(std::move(accept_ch_frame_arg)) {
accept_ch_frame(std::move(accept_ch_frame_arg)),
cert_is_issued_by_known_root(cert_is_issued_by_known_root) {
switch (type) {
case TransportType::kCached:
case TransportType::kCachedFromProxy:
Expand All @@ -58,14 +60,7 @@ TransportInfo::TransportInfo(const TransportInfo&) = default;

TransportInfo::~TransportInfo() = default;

bool TransportInfo::operator==(const TransportInfo& other) const {
return type == other.type && endpoint == other.endpoint &&
accept_ch_frame == other.accept_ch_frame;
}

bool TransportInfo::operator!=(const TransportInfo& other) const {
return !(*this == other);
}
bool TransportInfo::operator==(const TransportInfo& other) const = default;

std::string TransportInfo::ToString() const {
return base::StrCat({
Expand All @@ -75,6 +70,8 @@ std::string TransportInfo::ToString() const {
endpoint.ToString(),
", accept_ch_frame = ",
accept_ch_frame,
", cert_is_issued_by_known_root = ",
cert_is_issued_by_known_root ? "true" : "false",
" }",
});
}
Expand Down
11 changes: 9 additions & 2 deletions net/base/transport_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ struct NET_EXPORT TransportInfo {
TransportInfo();
TransportInfo(TransportType type_arg,
IPEndPoint endpoint_arg,
std::string accept_ch_frame_arg);
std::string accept_ch_frame_arg,
bool cert_is_issued_by_known_root);
TransportInfo(const TransportInfo&);
~TransportInfo();

// Instances of this type are comparable for equality.
bool operator==(const TransportInfo& other) const;
bool operator!=(const TransportInfo& other) const;

// Returns a string representation of this struct, suitable for debugging.
std::string ToString() const;
Expand All @@ -62,6 +62,13 @@ struct NET_EXPORT TransportInfo {
// Invariant: if `type` is `kCached` or `kCachedFromProxy`, then this is
// empty.
std::string accept_ch_frame;

// True if the transport layer was secure and the certificate was rooted at a
// standard CA root. (As opposed to a user-installed root.)
//
// Invariant: if `type` is `kCached` or `kCachedFromProxy`, then this is
// always false.
bool cert_is_issued_by_known_root = false;
};

// Instances of these types are streamable for easier debugging.
Expand Down
5 changes: 2 additions & 3 deletions net/http/http_basic_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,10 @@ bool HttpBasicStream::GetAlternativeService(
}

void HttpBasicStream::GetSSLInfo(SSLInfo* ssl_info) {
if (!state_.connection()->socket()) {
if (!state_.connection()->socket() ||
!state_.connection()->socket()->GetSSLInfo(ssl_info)) {
ssl_info->Reset();
return;
}
parser()->GetSSLInfo(ssl_info);
}

void HttpBasicStream::GetSSLCertRequestInfo(
Expand Down
4 changes: 3 additions & 1 deletion net/http/http_cache_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3218,7 +3218,9 @@ int HttpCache::Transaction::DoConnectedCallback() {
auto type = response_.was_fetched_via_proxy ? TransportType::kCachedFromProxy
: TransportType::kCached;
return connected_callback_.Run(
TransportInfo(type, response_.remote_endpoint, ""), io_callback_);
TransportInfo(type, response_.remote_endpoint, /*accept_ch_frame_arg=*/"",
/*cert_is_issued_by_known_root=*/false),
io_callback_);
}

int HttpCache::Transaction::DoConnectedCallbackComplete(int result) {
Expand Down
12 changes: 11 additions & 1 deletion net/http/http_network_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -981,9 +981,19 @@ int HttpNetworkTransaction::DoConnectedCallback() {
if (!proxy_info_.is_direct()) {
type = TransportType::kProxied;
}

bool is_issued_by_known_root = false;
if (IsSecureRequest()) {
SSLInfo ssl_info;
CHECK(stream_);
stream_->GetSSLInfo(&ssl_info);
is_issued_by_known_root = ssl_info.is_issued_by_known_root;
}

return connected_callback_.Run(
TransportInfo(type, remote_endpoint_,
std::string{stream_->GetAcceptChViaAlps()}),
std::string{stream_->GetAcceptChViaAlps()},
is_issued_by_known_root),
base::BindOnce(&HttpNetworkTransaction::ResumeAfterConnected,
base::Unretained(this)));
}
Expand Down
7 changes: 0 additions & 7 deletions net/http/http_stream_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1137,13 +1137,6 @@ void HttpStreamParser::OnConnectionClose() {
stream_socket_ = nullptr;
}

void HttpStreamParser::GetSSLInfo(SSLInfo* ssl_info) {
if (!request_->url.SchemeIsCryptographic() ||
!stream_socket_->GetSSLInfo(ssl_info)) {
ssl_info->Reset();
}
}

void HttpStreamParser::GetSSLCertRequestInfo(
SSLCertRequestInfo* cert_request_info) {
cert_request_info->Reset();
Expand Down
3 changes: 0 additions & 3 deletions net/http/http_stream_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ class HttpRequestHeaders;
class HttpResponseInfo;
class IOBuffer;
class SSLCertRequestInfo;
class SSLInfo;
class StreamSocket;
class UploadDataStream;

Expand Down Expand Up @@ -110,8 +109,6 @@ class NET_EXPORT_PRIVATE HttpStreamParser {
}
base::TimeTicks first_early_hints_time() { return first_early_hints_time_; }

void GetSSLInfo(SSLInfo* ssl_info);

void GetSSLCertRequestInfo(SSLCertRequestInfo* cert_request_info);

// Encodes the given |payload| in the chunked format to |output|.
Expand Down
4 changes: 3 additions & 1 deletion net/http/http_transaction_test_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ static MockTransactionMap mock_transactions;

TransportInfo DefaultTransportInfo() {
return TransportInfo(TransportType::kDirect,
IPEndPoint(IPAddress::IPv4Localhost(), 80), "");
IPEndPoint(IPAddress::IPv4Localhost(), 80),
/*accept_ch_frame_arg=*/"",
/*cert_is_issued_by_known_root=*/false);
}

//-----------------------------------------------------------------------------
Expand Down
5 changes: 2 additions & 3 deletions net/websockets/websocket_basic_handshake_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -352,11 +352,10 @@ bool WebSocketBasicHandshakeStream::GetLoadTimingInfo(
}

void WebSocketBasicHandshakeStream::GetSSLInfo(SSLInfo* ssl_info) {
if (!state_.connection()->socket()) {
if (!state_.connection()->socket() ||
!state_.connection()->socket()->GetSSLInfo(ssl_info)) {
ssl_info->Reset();
return;
}
parser()->GetSSLInfo(ssl_info);
}

void WebSocketBasicHandshakeStream::GetSSLCertRequestInfo(
Expand Down
12 changes: 8 additions & 4 deletions services/network/private_network_access_checker_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,22 +55,26 @@ net::IPEndPoint PublicEndpoint() {

net::TransportInfo DirectTransport(const net::IPEndPoint& endpoint) {
return net::TransportInfo(net::TransportType::kDirect, endpoint,
kNoAcceptChFrame);
kNoAcceptChFrame,
/*cert_is_issued_by_known_root=*/false);
}

net::TransportInfo ProxiedTransport(const net::IPEndPoint& endpoint) {
return net::TransportInfo(net::TransportType::kProxied, endpoint,
kNoAcceptChFrame);
kNoAcceptChFrame,
/*cert_is_issued_by_known_root=*/false);
}

net::TransportInfo CachedTransport(const net::IPEndPoint& endpoint) {
return net::TransportInfo(net::TransportType::kCached, endpoint,
kNoAcceptChFrame);
kNoAcceptChFrame,
/*cert_is_issued_by_known_root=*/false);
}

net::TransportInfo MakeTransport(net::TransportType type,
const net::IPEndPoint& endpoint) {
return net::TransportInfo(type, endpoint, kNoAcceptChFrame);
return net::TransportInfo(type, endpoint, kNoAcceptChFrame,
/*cert_is_issued_by_known_root=*/false);
}

TEST(PrivateNetworkAccessCheckerTest, ClientSecurityStateNull) {
Expand Down
4 changes: 3 additions & 1 deletion services/network/url_loader_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2182,7 +2182,9 @@ class URLLoaderFakeTransportInfoTest
static net::TransportInfo FakeTransportInfo(
const URLLoaderFakeTransportInfoTestParams& params) {
return net::TransportInfo(params.transport_type,
FakeEndpoint(params.endpoint_address_space), "");
FakeEndpoint(params.endpoint_address_space),
/*accept_ch_frame_arg=*/"",
/*cert_is_issued_by_known_root=*/false);
}
};

Expand Down

0 comments on commit 0c9d008

Please sign in to comment.