[
https://issues.apache.org/jira/browse/HBASE-15830?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15296824#comment-15296824
]
Gary Helmling commented on HBASE-15830:
---------------------------------------
Sorry for the delay in the reivew. A couple comments on the patch:
* please rename {{startHbaseConnectionWithEncryption(Channel ch)}} to just
{{startConnectionWithEncryption(Channel ch)}}. The extra "HBase" is
extraneous. I realize that the corresponding {{startHBaseConnection()}} method
is already named this way, but there is no need to continue it.
* in {{getChannelHeaderBytes(AuthMethod authMethod)}}, why not use
IPCUtil.getTotalSizeWhenWrittenDelimited() instead of hard-coding the extra 4
bytes?
* in {{SaslClientHandler}}, please avoid the whitespace-only / formatting
changes. These make it harder to trace actual code changes over time. Unless
you're making a substantive change to the line itself, these should not be
necessary.
* in {{SaslClientHandler.channelRead()}}:
{code}
if (!useWrap) {
ctx.pipeline().remove(this);
successfulConnectHandler.onSuccess(ctx.channel());
} else {
byte[] wrappedCH = saslClient.wrap(connectionHeader, 0,
connectionHeader.length);
// write connection header
writeSaslToken(ctx, wrappedCH);
successfulConnectHandler.onSaslProtectionSucess(ctx.channel());
}
{code}
It looks like we only write the connection header when qop != auth. Is this
right? Don't we need to write the connection header in both cases?
Have you tested this on a secure cluster with the different QoP configs (at
least auth vs conf)?
> Sasl encryption doesn't work with AsyncRpcChannelImpl
> -----------------------------------------------------
>
> Key: HBASE-15830
> URL: https://issues.apache.org/jira/browse/HBASE-15830
> Project: HBase
> Issue Type: Bug
> Reporter: Colin Ma
> Attachments: HBASE-15830.001.patch, HBASE-15830.002.patch
>
>
> Currently, sasl encryption doesn't work with AsyncRpcChannelImpl, there has 3
> problems:
> 1.
> [sourcecode|https://github.com/apache/hbase/blob/master/hbase-client/src/main/java/org/apache/hadoop/hbase/security/SaslClientHandler.java#L308]
> will throw the following exception:
> java.lang.UnsupportedOperationException: direct buffer
> at
> io.netty.buffer.UnpooledUnsafeDirectByteBuf.array(UnpooledUnsafeDirectByteBuf.java:199)
> at
> org.apache.hadoop.hbase.security.SaslClientHandler.write(SaslClientHandler.java:308)
> 2.
> [sourcecode|https://github.com/apache/hbase/blob/master/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/AsyncRpcChannelImpl.java#L212]
> has deadlocks problem.
> 3. TestAsyncSecureIPC doesn't cover the sasl encryption test case.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
