Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create SecureSocket directly instead of upgrading. #369

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 2.6.1

* Create `Securesocket` directly when `ChannelCredentials.authority` is not
present instead of always creating an insecure socket first and upgrading.
* Throw on insecure connections when disallowed by clients dealing with
mraleph marked this conversation as resolved.
Show resolved Hide resolved
socket code, if set via `isInsecureConnectionAllowed`.

## 2.6.0

* Create gRPC servers and clients with [Server|Client]TransportConnnection.
Expand Down
3 changes: 2 additions & 1 deletion lib/src/client/call.dart
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,8 @@ class ClientCall<Q, R> implements Response {
void _sendRequest(ClientConnection connection, Map<String, String> metadata) {
try {
_stream = connection.makeRequest(
_method.path, options.timeout, metadata, _onRequestError);
_method.path, options.timeout, metadata, _onRequestError,
callOptions: options);
} catch (e) {
_terminateWithError(GrpcError.unavailable('Error making call: $e'));
return;
Expand Down
33 changes: 24 additions & 9 deletions lib/src/client/http2_connection.dart
Original file line number Diff line number Diff line change
Expand Up @@ -308,21 +308,36 @@ class _SocketTransportConnector implements ClientTransportConnector {
@override
Future<ClientTransportConnection> connect() async {
final securityContext = _options.credentials.securityContext;
_socket = await Socket.connect(_host, _port);
// Don't wait for io buffers to fill up before sending requests.
_socket.setOption(SocketOption.tcpNoDelay, true);
if (securityContext != null) {
// Todo(sigurdm): We want to pass supportedProtocols: ['h2'].
// http://dartbug.com/37950
_socket = await SecureSocket.secure(_socket,
if (securityContext == null) {
if (!isInsecureConnectionAllowed(_host)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible please add at least a simple test.

throw StateError("Insecure gRPC is not allowed by platform: $_host");
}
_socket = await Socket.connect(_host, _port);
} else {
if (_options.credentials.authority == null) {
// Todo(sigurdm): We want to pass supportedProtocols: ['h2'].
// http://dartbug.com/37950
_socket = await SecureSocket.connect(
_host,
_port,
context: securityContext,
onBadCertificate: _validateBadCertificate,
);
} else {
_socket = await SecureSocket.secure(
await Socket.connect(_host, _port),
// This is not really the host, but the authority to verify the TLC
// connection against.
//
// We don't use `this.authority` here, as that includes the port.
host: _options.credentials.authority ?? _host,
host: _options.credentials.authority,
context: securityContext,
onBadCertificate: _validateBadCertificate);
onBadCertificate: _validateBadCertificate,
);
}
}
// Don't wait for io buffers to fill up before sending requests.
_socket.setOption(SocketOption.tcpNoDelay, true);

return ClientTransportConnection.viaSocket(_socket);
}
Expand Down
3 changes: 0 additions & 3 deletions lib/src/client/transport/xhr_transport.dart
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,6 @@ class XhrClientConnection extends ClientConnection {
for (final header in metadata.keys) {
request.setRequestHeader(header, metadata[header]);
}
request.setRequestHeader('Content-Type', 'application/grpc-web+proto');
request.setRequestHeader('X-User-Agent', 'grpc-web-dart/0.1');
request.setRequestHeader('X-Grpc-Web', '1');
// Overriding the mimetype allows us to stream and parse the data
request.overrideMimeType('text/plain; charset=x-user-defined');
request.responseType = 'text';
Expand Down
5 changes: 5 additions & 0 deletions lib/src/server/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@ class Server extends ConnectionServer {
socket.setOption(SocketOption.tcpNoDelay, true);
final connection = ServerTransportConnection.viaSocket(socket,
settings: http2ServerSettings);
if (security != null && !security.validateClient(socket)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should write a simple test for this as well given how easy we have accidentally lost it (I did not notice it in my review, sorry).

_printSocketError('unable to serve $address:$port - '
'unable to validate client socket');
return socket.close();
}
serveConnection(connection);
}, onError: _printSocketError);
}
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: grpc
description: Dart implementation of gRPC, a high performance, open-source universal RPC framework.

version: 2.6.0
version: 2.6.1

author: Dart Team <misc@dartlang.org>
homepage: https://github.com/dart-lang/grpc-dart
Expand Down