Skip to content

Commit

Permalink
Send KeyUpdates after the first post-handshake write.
Browse files Browse the repository at this point in the history
This is intended to ensure that KeyUpdate remains viable on the web as
TLS 1.3 implementations deploy. Without exercise, we risk broken
implementations becoming established.

In time we probably don't need to have this at 100%, but 100% leads to
clear behaviour in case there are already broken implementations.

Finch control included in case we need to disable it.

Change-Id: Icb7699ee3f90d65195cd52c813efedd55e67c707
Reviewed-on: https://chromium-review.googlesource.com/c/1423423
Reviewed-by: David Benjamin <davidben@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624380}
  • Loading branch information
Adam Langley authored and Commit Bot committed Jan 19, 2019
1 parent fb4408b commit 68df3af
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 0 deletions.
3 changes: 3 additions & 0 deletions net/base/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,8 @@ const base::Feature kEnforceTLS13Downgrade{"EnforceTLS13Downgrade",
const base::Feature kSplitCacheByTopFrameOrigin{
"SplitCacheByTopFrameOrigin", base::FEATURE_DISABLED_BY_DEFAULT};

const base::Feature kTLS13KeyUpdate{"TLS13KeyUpdate",
base::FEATURE_ENABLED_BY_DEFAULT};

} // namespace features
} // namespace net
6 changes: 6 additions & 0 deletions net/base/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ NET_EXPORT extern const base::Feature kEnforceTLS13Downgrade;
// Splits cache entries by the request's top frame's origin if one is available.
NET_EXPORT extern const base::Feature kSplitCacheByTopFrameOrigin;

// Enables sending TLS 1.3 Key Update messages on TLS 1.3 connections in order
// to ensure that this corner of the spec is exercised. This is enabled by
// default and exists as a feature to allow it to be remotely disabled if
// needed.
NET_EXPORT extern const base::Feature kTLS13KeyUpdate;

} // namespace features
} // namespace net

Expand Down
8 changes: 8 additions & 0 deletions net/socket/ssl_client_socket_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1404,6 +1404,14 @@ int SSLClientSocketImpl::DoPayloadWrite() {
if (rv >= 0) {
net_log_.AddByteTransferEvent(NetLogEventType::SSL_SOCKET_BYTES_SENT, rv,
user_write_buf_->data());
if (first_post_handshake_write_ && SSL_is_init_finished(ssl_.get())) {
if (base::FeatureList::IsEnabled(features::kTLS13KeyUpdate) &&
SSL_version(ssl_.get()) == TLS1_3_VERSION) {
const int ok = SSL_key_update(ssl_.get(), SSL_KEY_UPDATE_REQUESTED);
DCHECK(ok);
}
first_post_handshake_write_ = false;
}
return rv;
}

Expand Down
1 change: 1 addition & 0 deletions net/socket/ssl_client_socket_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ class SSLClientSocketImpl : public SSLClientSocket,
// Used by Write function.
scoped_refptr<IOBuffer> user_write_buf_;
int user_write_buf_len_;
bool first_post_handshake_write_ = true;

// Used by DoPayloadRead() when attempting to fill the caller's buffer with
// as much data as possible without blocking.
Expand Down

0 comments on commit 68df3af

Please sign in to comment.