> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java,
> > lines 368-381
> > <https://reviews.apache.org/r/33620/diff/13/?file=1021979#file1021979line368>
> >
> > If handshake status is BUFFER_OVERFLOW, we will return to the caller
> > and then to the selector. However, we may have read all incoming bytes into
> > netReadBuffer. So, the key may never be selected again to complete the
> > handshake. It seems that this case can never happen during handshake since
> > we don't expect to use the appReadBuffer. Perhaps we can just assert that
> > state is illegal when handling NEED_UNWRAP in handshake().
>
> Sriharsha Chintalapani wrote:
> Sorry where you want to add assert? and which state you want to handle?
Basically, I am saying that in handshake(), we should never hit BUFFER_OVERFLOW
since we never read anything to appReadBuffer. Had we hit BUFFER_OVERFLOW, the
issue is that you may have all socket bytes read into netReadBuffer, but not
all bytes in netReadBuffer have been read by SslEngine. In the next
Selector.poll() call, those bytes will be handled. However, this key may never
be selected since there are no more bytes from the socket. If this is the case,
I'd suggest that we either assert that BUFFER_OVERFLOW should never happen when
handling unwrap or at least add a comment that this should never happen.
case NEED_UNWRAP:
log.trace("SSLHandshake NEED_UNWRAP channelId {},
appReadBuffer pos {}, netReadBuffer pos {}, netWriteBuffer pos {}",
channelId, appReadBuffer.position(),
netReadBuffer.position(), netWriteBuffer.position());
handshakeResult = handshakeUnwrap(read);
if (handshakeResult.getStatus() == Status.BUFFER_UNDERFLOW)
{
int currentPacketBufferSize = packetBufferSize();
netReadBuffer = Utils.ensureCapacity(netReadBuffer,
currentPacketBufferSize);
if (netReadBuffer.position() >=
currentPacketBufferSize) {
throw new IllegalStateException("Buffer underflow
when there is available data");
}
} else if (handshakeResult.getStatus() ==
Status.BUFFER_OVERFLOW) {
int currentAppBufferSize = applicationBufferSize();
appReadBuffer = Utils.ensureCapacity(appReadBuffer,
currentAppBufferSize);
if (appReadBuffer.position() > currentAppBufferSize) {
throw new IllegalStateException("Buffer underflow
when available data size (" + appReadBuffer.position() +
") > packet buffer
size (" + currentAppBufferSize + ")");
}
} else if (handshakeResult.getStatus() == Status.CLOSED) {
throw new EOFException("SSL handshake status CLOSED
during handshake UNWRAP");
}
- Jun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33620/#review93862
-----------------------------------------------------------
On Aug. 17, 2015, 7:21 p.m., Sriharsha Chintalapani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> -----------------------------------------------------------
>
> (Updated Aug. 17, 2015, 7:21 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1690
> https://issues.apache.org/jira/browse/KAFKA-1690
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1690. new java producer needs ssl support as a client.
>
>
> KAFKA-1690. new java producer needs ssl support as a client.
>
>
> KAFKA-1690. new java producer needs ssl support as a client.
>
>
> KAFKA-1690. new java producer needs ssl support as a client. SSLFactory tests.
>
>
> KAFKA-1690. new java producer needs ssl support as a client. Added
> PrincipalBuilder.
>
>
> KAFKA-1690. new java producer needs ssl support as a client. Addressing
> reviews.
>
>
> KAFKA-1690. new java producer needs ssl support as a client. Addressing
> reviews.
>
>
> KAFKA-1690. new java producer needs ssl support as a client. Addressing
> reviews.
>
>
> KAFKA-1690. new java producer needs ssl support as a client. Fixed minor
> issues with the patch.
>
>
> KAFKA-1690. new java producer needs ssl support as a client. Fixed minor
> issues with the patch.
>
>
> KAFKA-1690. new java producer needs ssl support as a client.
>
>
> KAFKA-1690. new java producer needs ssl support as a client.
>
>
> Merge remote-tracking branch 'refs/remotes/origin/trunk' into KAFKA-1690-V1
>
>
> KAFKA-1690. Broker side ssl changes.
>
>
> KAFKA-1684. SSL for socketServer.
>
>
> KAFKA-1690. Added SSLProducerSendTest and fixes to get right port for SSL.
>
>
> Merge branch 'trunk' into KAFKA-1690-V1
>
>
> KAFKA-1690. Post merge fixes.
>
>
> KAFKA-1690. Added SSLProducerSendTest.
>
>
> KAFKA-1690. Minor fixes based on patch review comments.
>
>
> Merge commit
>
>
> KAFKA-1690. Added SSL Consumer Test.
>
>
> KAFKA-1690. SSL Support.
>
>
> KAFKA-1690. Addressing reviews.
>
>
> Merge branch 'trunk' into KAFKA-1690-V1
>
>
> Merge branch 'trunk' into KAFKA-1690-V1
>
>
> KAFKA-1690. Addressing reviews. Removed interestOps from SSLTransportLayer.
>
>
> KAFKA-1690. Addressing reviews.
>
>
> KAFKA-1690. added staged receives to selector.
>
>
> KAFKA-1690. Addressing reviews.
>
>
> Merge branch 'trunk' into KAFKA-1690-V1
>
>
> KAFKA-1690. Addressing reviews.
>
>
> KAFKA-1690. Add SSL support to broker, producer and consumer.
>
>
> Merge remote-tracking branch 'refs/remotes/origin/trunk' into KAFKA-1690-V1
>
>
> KAFKA-1690. Add SSL support to Kafka Broker, Producer & Client.
>
>
> KAFKA-1690. Add SSL support for Kafka Brokers, Producers and Consumers.
>
>
> Merge remote-tracking branch 'refs/remotes/origin/trunk' into KAFKA-1690-V1
>
>
> KAFKA-1690. Add SSL support for Kafka Brokers, Producers and Consumers.
>
>
> KAFKA-1690. Add SSL Support Kafka Broker, Producer and Consumer.
>
>
> Diffs
> -----
>
> build.gradle 983587fd0b7604c3a26fcbb6a1d63e5e470d23fe
> checkstyle/import-control.xml e3f4f84c6becfd9087627f018690e1e2fc2b3bba
> clients/src/main/java/org/apache/kafka/clients/ClientUtils.java
> 0d68bf1e1e90fe9d5d4397ddf817b9a9af8d9f7a
> clients/src/main/java/org/apache/kafka/clients/CommonClientConfigs.java
> 2c421f42ed3fc5d61cf9c87a7eaa7bb23e26f63b
> clients/src/main/java/org/apache/kafka/clients/NetworkClient.java
> 0e51d7bd461d253f4396a5b6ca7cd391658807fa
> clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java
> d35b421a515074d964c7fccb73d260b847ea5f00
> clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java
> be46b6c213ad8c6c09ad22886a5f36175ab0e13a
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
> 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7
> clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java
> aa264202f2724907924985a5ecbe74afc4c6c04b
> clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java
> 6c317480a181678747bfb6b77e315b08668653c5
> clients/src/main/java/org/apache/kafka/common/config/SSLConfigs.java
> PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/network/Authenticator.java
> PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/network/ByteBufferSend.java
> df0e6d5105ca97b7e1cb4d334ffb7b443506bd0b
> clients/src/main/java/org/apache/kafka/common/network/ChannelBuilder.java
> PRE-CREATION
>
> clients/src/main/java/org/apache/kafka/common/network/DefaultAuthenticator.java
> PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/network/KafkaChannel.java
> PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/network/NetworkReceive.java
> 3ca0098b8ec8cfdf81158465b2d40afc47eb6f80
>
> clients/src/main/java/org/apache/kafka/common/network/PlaintextChannelBuilder.java
> PRE-CREATION
>
> clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
> PRE-CREATION
>
> clients/src/main/java/org/apache/kafka/common/network/SSLChannelBuilder.java
> PRE-CREATION
>
> clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java
> PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/network/Selectable.java
> 618a0fa53848ae6befea7eba39c2f3285b734494
> clients/src/main/java/org/apache/kafka/common/network/Selector.java
> ce20111ac434eb8c74585e9c63757bb9d60a832f
> clients/src/main/java/org/apache/kafka/common/network/Send.java
> 8f6daadf6b67c3414911cda77765512131e56fd3
> clients/src/main/java/org/apache/kafka/common/network/TransportLayer.java
> PRE-CREATION
>
> clients/src/main/java/org/apache/kafka/common/protocol/SecurityProtocol.java
> dab1a94dd29563688b6ecf4eeb0e180b06049d3f
>
> clients/src/main/java/org/apache/kafka/common/security/auth/DefaultPrincipalBuilder.java
> PRE-CREATION
>
> clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
> PRE-CREATION
>
> clients/src/main/java/org/apache/kafka/common/security/auth/PrincipalBuilder.java
> PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/security/ssl/SSLFactory.java
> PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/utils/Utils.java
> 80a914ee1d12a4b855fbdab24c7e870d33fb475e
> clients/src/test/java/org/apache/kafka/clients/ClientUtilsTest.java
> 13ce519f03d13db041e1f2dbcd6b59414d2775b8
>
> clients/src/test/java/org/apache/kafka/clients/producer/KafkaProducerTest.java
> f3f8334f848be4cc043d5a573975609a3681fe7e
> clients/src/test/java/org/apache/kafka/common/network/EchoServer.java
> PRE-CREATION
> clients/src/test/java/org/apache/kafka/common/network/SSLSelectorTest.java
> PRE-CREATION
> clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java
> 158f9829ff64a969008f699e40c51e918287859e
>
> clients/src/test/java/org/apache/kafka/common/security/ssl/SSLFactoryTest.java
> PRE-CREATION
> clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java
> e7951d835472e5defe49be435f2c93685ba544d5
> clients/src/test/java/org/apache/kafka/test/MockSelector.java
> 51eb9d142f566c94a87add68b8c4f78b56d6ec3e
> clients/src/test/java/org/apache/kafka/test/TestSSLUtils.java PRE-CREATION
> core/src/main/scala/kafka/api/FetchResponse.scala
> 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215
> core/src/main/scala/kafka/network/SocketServer.scala
> dbe784b63817fd94e1593136926db17fac6fa3d7
> core/src/main/scala/kafka/server/KafkaConfig.scala
> 394f21b5d70bd527e77b611b2d1f9b36f0d0be81
> core/src/main/scala/kafka/server/KafkaServer.scala
> 6d6550732f393037ed1750cc8a7010a29d2c574a
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
> 4b6358c2329448a3b3fbdce455c7f38b3978a274
> core/src/test/scala/integration/kafka/api/SSLConsumerTest.scala
> PRE-CREATION
> core/src/test/scala/integration/kafka/api/SSLProducerSendTest.scala
> PRE-CREATION
> core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala
> 05b9a87d6918ba93cb2ab364ae7bd52f8d9b4b78
> core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala
> 4dba7dce57e674104f34556018fa8f1543c6c247
> core/src/test/scala/unit/kafka/network/SocketServerTest.scala
> d08b8b833110771a2a33ce73f09b62e445f96a0c
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala
> 9688b8cf64d3d74021d21eb516ebd6ba424e013c
> core/src/test/scala/unit/kafka/utils/TestUtils.scala
> f00f00a260af4b0bc503e1f9096749d7d4ad1f24
>
> Diff: https://reviews.apache.org/r/33620/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sriharsha Chintalapani
>
>
