[
https://issues.apache.org/jira/browse/KAFKA-1684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14199501#comment-14199501
]
Jun Rao commented on KAFKA-1684:
--------------------------------
Ivan,
Thanks for the patch. Some high level comments.
1. SocketOption is only in java 7 and beyond. Do we want to stop supporting
java 6?
2. SSLConnectionConfig is hard-coded to load from a config file. Since the
Kafka broker may be started from a container which may have its own way of
dealing with configuration, we should pass in the SSL specific configs through
the constructor of KafkaServer. It seems that the easiest way is to add those
SSL configs to KafkaConfig directly.
3. You added a new ZK path to register the broker secure port. I am wondering
if it's simpler to just add that info to the value of the existing broker path
in ZK.
4. You added a new request type TopicMetadataForChannelRequest. Again, I am
wondering if it's simpler to just add the secure port in the existing
TopicMetadataResponse, instead of creating a new type of request.
5. You evolved the request format of UpdateMetadataRequest. For backward
compatibility, we need to bump up the version number and make sure that the
receiver can parse both versions of the request.
6. We need to think through how to do rolling upgrades with this change. This
is a bit tricky since the broker registry in ZK is read by the controller,
which can be of any broker. So, if a broker is registered in the new format in
ZK and then the controller tries to read it, but doesn't understand its format
(since the controller broker hasn't been upgraded), then things will break.
Similarly, UpdateMetadataRequest is used for communication btw brokers, if a
broker receives a request but doesn't understand its format, bad things will
happen. One way to do this kind of upgrade is to do this in two steps. In step
one, we upgrade the software for each broker, but disallow the broker to use
the new ZK or request format. In step two, we make a config change to every
broker to enable the brokers to start using the new format.
7. Do we plan to use the same secure port for either SSL or SASL? If so, this
port should probably be named in a more general way.
8. Which port should the follower use? If both the plaintext and the secure
port are configured, should we just pick one (e.g., secure) or make it
configurable?
9. What's the plan for unit testing this feature?
Some low level comments.
10. SSLSocketChannel: Some of the logic in this class is a bit hard to follow.
10.1 It's not clear to me what values initialized can have and what those
values mean.
10.2 It's not very clear to me why we need to write to a socket (through
writeIfReadyAndNeeded()) while reading from a socket (when initialized != -1).
10.3 Could you explain a bit what isExtraReadRequired() and
Processor.readExtraIfNeeded() do?
10.4 We will need a non-blocking version of connect(), for the new java clients.
10.5 It's a bit weird to simulate blocking read using a selector. Should we
just do the read directly since the socket has already been properly configured
with blocking or not?
It would probably be useful to first document the config/request/ZK changes in
a wiki and also outline the upgrade path.
> Implement TLS/SSL authentication
> --------------------------------
>
> Key: KAFKA-1684
> URL: https://issues.apache.org/jira/browse/KAFKA-1684
> Project: Kafka
> Issue Type: Sub-task
> Components: security
> Affects Versions: 0.9.0
> Reporter: Jay Kreps
> Assignee: Ivan Lyutov
> Attachments: KAFKA-1684.patch
>
>
> Add an SSL port to the configuration and advertise this as part of the
> metadata request.
> If the SSL port is configured the socket server will need to add a second
> Acceptor thread to listen on it. Connections accepted on this port will need
> to go through the SSL handshake prior to being registered with a Processor
> for request processing.
> SSL requests and responses may need to be wrapped or unwrapped using the
> SSLEngine that was initialized by the acceptor. This wrapping and unwrapping
> is very similar to what will need to be done for SASL-based authentication
> schemes. We should have a uniform interface that covers both of these and we
> will need to store the instance in the session with the request. The socket
> server will have to use this object when reading and writing requests. We
> will need to take care with the FetchRequests as the current
> FileChannel.transferTo mechanism will be incompatible with wrap/unwrap so we
> can only use this optimization for unencrypted sockets that don't require
> userspace translation (wrapping).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
