feat: support channel binding configuration in PgConnectOptions#1671
feat: support channel binding configuration in PgConnectOptions#1671jorsol wants to merge 1 commit into
Conversation
|
Can you please sign the Eclipse ECA? Would you mind adding the option field to connect options while you're at it? |
|
Ideally we'd also have a test that verifies it is not possible to connect if the user sets the |
|
@tsegismont Could you please review again? I'm not fully familiar with the code base, so maybe you can point me to improvements. |
312e6a4 to
86caa70
Compare
| // ctx.assertEquals("Channel bindins is required", err.getMessage()); | ||
| ctx.assertTrue(err instanceof ClosedConnectionException); // TODO: handle ChannelBindingException |
There was a problem hiding this comment.
As in #1672 (this could be due to changes in the way we handle exceptions since 5.1)
There was a problem hiding this comment.
I'll take a look after merging
There was a problem hiding this comment.
Thank you, I'm really lost on how to handle the exceptions here.
tsegismont
left a comment
There was a problem hiding this comment.
Thank you @jorsol ! Please find a few comments
| // ctx.assertEquals("Channel bindins is required", err.getMessage()); | ||
| ctx.assertTrue(err instanceof ClosedConnectionException); // TODO: handle ChannelBindingException |
There was a problem hiding this comment.
I'll take a look after merging
| socket.channelHandlerContext().channel() | ||
| .attr(AttributeKey.valueOf("channel_binding")) | ||
| .set(connectOptions.getChannelBinding()); |
There was a problem hiding this comment.
We can avoid indirect communication through a channel attribute.
You could add a method in this class like:
public ChannelBinding channelBinding() {
return connectOptions.getChannelBinding();
}In io.vertx.pgclient.impl.codec.InitPgCommandMessage#handleAuthenticationSasl
PgSocketConnection pgSocketConn = (PgSocketConnection) cmd.connection().unwrap();
ScramClientInitialMessage initialSaslMessage = scramSession.createInitialSaslMessage(in, encoder.channelHandlerContext(), pgSocketConn.channelBinding());
encoder.writeScramClientInitialMessage(initialSaslMessage);There was a problem hiding this comment.
Hi @tsegismont, thanks for pointing to this, those changes are done now.
Introduce support for configuring channel binding mechanisms during the SCRAM-SHA-256-PLUS authentication handshake. This allows users to enforce, prefer, or disable channel binding securely. - Add channelBinding property and getters/setters to PgConnectOptions - Update PgConnectionUriParser to parse 'channel_binding' from connection URIs - Store channel binding preference in Netty Channel attributes during initialization - Propagate the configuration to ScramAuthentication to negotiate -PLUS variants - Fail the connection handshake if a mismatch or unsupported negotiation occurs Signed-off-by: Jorge Solorzano <jorsol@gmail.com>
Motivation:
Introduce support for configuring channel binding mechanisms during the
SCRAM-SHA-256-PLUS authentication handshake. This allows users to require,
prefer, or disable channel binding securely.
Conformance:
You should have signed the Eclipse Contributor Agreement as explained in https://github.com/eclipse/vert.x/blob/master/CONTRIBUTING.md
Please also make sure you adhere to the code style guidelines: https://github.com/vert-x3/wiki/wiki/Vert.x-code-style-guidelines