> On May 14, 2015, 10:21 a.m., Rajini Sivaram wrote: > > clients/src/main/java/org/apache/kafka/common/network/Authenticator.java, > > line 47 > > <https://reviews.apache.org/r/33620/diff/5/?file=957060#file957060line47> > > > > Don't think authenticator interface should handle selection > > interestOps. Needs better encapsulation.
SaslAuth works by reading socketChannel and doing accept token on incoming data on server side and similarly saslclient sends a new token . Why do you think authenticate shouldn't handle interestOps since its reading and writing to socketChannel. Whats your proposal > On May 14, 2015, 10:21 a.m., Rajini Sivaram wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 321 > > <https://reviews.apache.org/r/33620/diff/5/?file=957065#file957065line321> > > > > The man-in-the-middle attack in CVE-2009-3555 has been fixed in Java > > for quite some time. But since there have been reported DOS vulnerabilities > > etc., it may be ok to disable re-negotiation for now. > > > > There are other scenarios (apart from access to protected resources) > > where re-negotiation may be useful, for instance to refresh encryption > > keys. While this may not be strictly necessary for the first version of SSL > > implementation, it will be good to ensure that the transport layer > > interfaces are sufficient to incorporate re-negotiation in future. Not sure > > if the current interface can support re-negotiation and it will be good to > > get the interfaces sorted now in any case. Adding rehandshake to the transportLayer is not an issue. As I said in my previous comments its not a necessity. > On May 14, 2015, 10:21 a.m., Rajini Sivaram wrote: > > clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java, > > line 45 > > <https://reviews.apache.org/r/33620/diff/5/?file=957077#file957077line45> > > > > SelectorTest should be run with SSL as well as PlainText since all > > selector functions should be tested with SSL as the transport layer. > > Simplest way is to parameterize the test. > > https://reviews.apache.org/r/33133/diff/ has the code for parameterizing > > the test, which you may be able to reuse with some tweaks. > > > > > > Tried this and testMute() fails with SSL There is SSLSelectorTest which configures the Selector with SSL and runs tests with EchoServer with sslserversocket. Not sure why Selector which is PlainText should run SSL? - Sriharsha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review83741 ----------------------------------------------------------- On May 12, 2015, 11:20 p.m., Sriharsha Chintalapani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33620/ > ----------------------------------------------------------- > > (Updated May 12, 2015, 11:20 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. > > > Diffs > ----- > > build.gradle fef515b3b2276b1f861e7cc2e33e74c3ce5e405b > checkstyle/checkstyle.xml a215ff36e9252879f1e0be5a86fef9a875bb8f38 > checkstyle/import-control.xml f2e6cec267e67ce8e261341e373718e14a8e8e03 > clients/src/main/java/org/apache/kafka/clients/ClientUtils.java > 0d68bf1e1e90fe9d5d4397ddf817b9a9af8d9f7a > clients/src/main/java/org/apache/kafka/clients/CommonClientConfigs.java > cf32e4e7c40738fe6d8adc36ae0cfad459ac5b0b > clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java > bdff518b732105823058e6182f445248b45dc388 > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java > d301be4709f7b112e1f3a39f3c04cfa65f00fa60 > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > 8e336a3aa96c73f52beaeb56b931baf4b026cf21 > clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java > 187d0004c8c46b6664ddaffecc6166d4b47351e5 > clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java > c4fa058692f50abb4f47bd344119d805c60123f5 > clients/src/main/java/org/apache/kafka/common/network/Authenticator.java > PRE-CREATION > clients/src/main/java/org/apache/kafka/common/network/Channel.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/PlainTextTransportLayer.java > PRE-CREATION > clients/src/main/java/org/apache/kafka/common/network/SSLFactory.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 > b5f8d83e89f9026dc0853e5f92c00b2d7f043e22 > clients/src/main/java/org/apache/kafka/common/network/Selector.java > 57de0585e5e9a53eb9dcd99cac1ab3eb2086a302 > 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/utils/Utils.java > f73eedb030987f018d8446bb1dcd98d19fa97331 > clients/src/test/java/org/apache/kafka/common/network/EchoServer.java > PRE-CREATION > clients/src/test/java/org/apache/kafka/common/network/SSLFactoryTest.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 > d5b306b026e788b4e5479f3419805aa49ae889f3 > clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java > 2ebe3c21f611dc133a2dbb8c7dfb0845f8c21498 > clients/src/test/java/org/apache/kafka/test/TestSSLUtils.java PRE-CREATION > > Diff: https://reviews.apache.org/r/33620/diff/ > > > Testing > ------- > > > Thanks, > > Sriharsha Chintalapani > >
