Skip to content

Consider enforcing ALPN negotiation #1042

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

Closed
Lukasa opened this issue Nov 16, 2020 · 1 comment
Closed

Consider enforcing ALPN negotiation #1042

Lukasa opened this issue Nov 16, 2020 · 1 comment
Labels
kind/enhancement Improvements to existing feature.

Comments

@Lukasa
Copy link
Collaborator

Lukasa commented Nov 16, 2020

Right now both the server and the client ignore the result of the ALPN negotiation. The client does log the outcome, but otherwise does nothing: the server largely ignores it and sniffs the initial content. This is likely suboptimal: the ALPN is normative and we should probably pay attention to it. In some cases we likely want to skip that, but we can make that configuration.

@Lukasa Lukasa added the kind/enhancement Improvements to existing feature. label Nov 16, 2020
glbrntt added a commit to glbrntt/grpc-swift that referenced this issue Nov 24, 2020
Motivation:

To determine whether we're dealing with gRPC or gRPC Web (i.e. HTTP/2 or
HTTP/1) the first bytes received on a new connection are parsed. When
TLS is enabled and ALPN is supported there's no need to do this: we
will be informed about the protocol we negotiated with our peer and
therefore how to configure the channel. We also never enforced that the
protocol negotiated via ALPN was used!

Modifications:

- Add a 'GRPCServerPipelineConfigurator' handler to supersede the
  'HTTPProtocolSwitcher'. The new handler will use either ALPN and fall
  back to parsing the incoming bytes.
- The parsing behavior is slightly different to that of
  'HTTPProtocolSwitcher' in that we only read off enough bytes for the
  HTTP/2 connection preface, or enough bytes to parse an HTTP/1 request
  line (rather than reading the whole buffer as a string and processing
  that).
- Added a server configuration option to disable ALPN being required.
- Added tests.

Result:

- Server pipeline configuration is driven by ALPN, falling back to
  parsing.
- ALPN is enforced on the server (partially resolving grpc#1042)
glbrntt added a commit to glbrntt/grpc-swift that referenced this issue Nov 25, 2020
Motivation:

To determine whether we're dealing with gRPC or gRPC Web (i.e. HTTP/2 or
HTTP/1) the first bytes received on a new connection are parsed. When
TLS is enabled and ALPN is supported there's no need to do this: we
will be informed about the protocol we negotiated with our peer and
therefore how to configure the channel. We also never enforced that the
protocol negotiated via ALPN was used!

Modifications:

- Add a 'GRPCServerPipelineConfigurator' handler to supersede the
  'HTTPProtocolSwitcher'. The new handler will use either ALPN and fall
  back to parsing the incoming bytes.
- The parsing behavior is slightly different to that of
  'HTTPProtocolSwitcher' in that we only read off enough bytes for the
  HTTP/2 connection preface, or enough bytes to parse an HTTP/1 request
  line (rather than reading the whole buffer as a string and processing
  that).
- Added a server configuration option to disable ALPN being required.
- Added tests.

Result:

- Server pipeline configuration is driven by ALPN, falling back to
  parsing.
- ALPN is enforced on the server (partially resolving grpc#1042)
glbrntt added a commit that referenced this issue Nov 26, 2020
Motivation:

To determine whether we're dealing with gRPC or gRPC Web (i.e. HTTP/2 or
HTTP/1) the first bytes received on a new connection are parsed. When
TLS is enabled and ALPN is supported there's no need to do this: we
will be informed about the protocol we negotiated with our peer and
therefore how to configure the channel. We also never enforced that the
protocol negotiated via ALPN was used!

Modifications:

- Add a 'GRPCServerPipelineConfigurator' handler to supersede the
  'HTTPProtocolSwitcher'. The new handler will use either ALPN and fall
  back to parsing the incoming bytes.
- The parsing behavior is slightly different to that of
  'HTTPProtocolSwitcher' in that we only read off enough bytes for the
  HTTP/2 connection preface, or enough bytes to parse an HTTP/1 request
  line (rather than reading the whole buffer as a string and processing
  that).
- Added a server configuration option to disable ALPN being required.
- Added tests.

Result:

- Server pipeline configuration is driven by ALPN, falling back to
  parsing.
- ALPN is enforced on the server (partially resolving #1042)
@glbrntt
Copy link
Collaborator

glbrntt commented Apr 30, 2025

Resolved by #1055

@glbrntt glbrntt closed this as completed Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements to existing feature.
Projects
None yet
Development

No branches or pull requests

2 participants