The broker's configuration options are "listeners" (plural) and
"listeners.security.protocol.map". I agree that following the pattern set
by the broker is better, so these are really good ideas. However, at this
point I don't see a need for the "listeners.security.procotol.map", which
for the broker must be set if the listener name is not a security protocol.
Can we not simply just allow "HTTP" and "HTTPS" as the names of the
listeners (rather than the broker's "PLAINTEXT", "SSL", etc.)? If so, then
for example "listeners" might be set to "http://myhost:8081,
https://myhost:80";, which seems to work out nicely without needing listener
names other than security protocols.

I also like using the worker's SSL and SASL security configs by default if
"https" is included in the listener, but allowing the overriding of this
via other additional properties. Here, I'm not a big fan of
"listeners.name.https.*" prefix, which I think is pretty verbose, but I
could see "listener.https.*" as a prefix. This allows us to add other
security protocols at some point, if that ever becomes necessary.

+1 for continuing down this road. Nice work.

On Mon, Oct 16, 2017 at 9:51 AM, Ted Yu <yuzhih...@gmail.com> wrote:

> +1 to this proposal.
>
> On Mon, Oct 16, 2017 at 7:49 AM, Jakub Scholz <ja...@scholz.cz> wrote:
>
> > I was having some more thoughts about it. We can simply take over what
> > Kafka broker implements for the listeners:
> > - We can take over the "listener" and "listener.security.protocol.map"
> > options to define multiple REST listeners and the security protocol they
> > should use
> > - The HTTPS interface will by default use the default configuration
> options
> > ("ssl.keystore.localtion" etc.). But if desired, the values can be
> > overridden for given listener (again, as in Kafka broker "listener.name
> > .<LISTENER_NAME>.ssl.keystore.location")
> >
> > This should address both issues raised. But before I incorporate it into
> > the KIP, I would love to get some feedback if this sounds OK. Please let
> me
> > know what do you think ...
> >
> > Jakub
> >
> > On Sun, Oct 15, 2017 at 12:23 AM, Jakub Scholz <ja...@scholz.cz> wrote:
> >
> > > I agree, adding both HTTP and HTTPS is not complicated. I just didn't
> saw
> > > the use case for it. But I can add it. Would you add just support for a
> > > single HTTP and single HTTPS interface? Or do you see some value even
> in
> > > allowing more than 2 interfaces (for example one HTTP and two HTTPS
> with
> > > different configuration)? It could be done similarly to how the Kafka
> > > broker does it through the "listener" configuration parameter with
> comma
> > > separated list. What do you think?
> > >
> > > As for the "rest" prefix - if we remove it, some of the same
> > configuration
> > > options are already used today as the option for connecting from Kafka
> > > Connect to Kafka broker. So I'm not sure we should mix them. I can
> > > definitely imagine some cases where the client SSL configuration will
> not
> > > be the same as the REST HTTPS configuration. That is why I added the
> > > prefix. If we remove the prefix, how would you deal with this?
> > >
> > > On Fri, Oct 13, 2017 at 6:25 PM, Randall Hauch <rha...@gmail.com>
> wrote:
> > >
> > >> Also, do we need these properties to be preceded with `rest`? I'd
> argue
> > >> that we're just configuring the worker's SSL information, and that the
> > >> REST
> > >> API would just use that. If we added another non-REST API, we'd want
> to
> > >> use
> > >> the same security configuration.
> > >>
> > >> It's not that complicated in Jetty to support both "http" and "https"
> > >> simultaneously, so IMO we should add that from the beginning.
> > >>
> > >> On Fri, Oct 13, 2017 at 9:34 AM, Randall Hauch <rha...@gmail.com>
> > wrote:
> > >>
> > >> > It'd be useful to specify the default values for the configuration
> > >> > properties.
> > >> >
> > >> > On Tue, Oct 10, 2017 at 2:53 AM, Jakub Scholz <ja...@scholz.cz>
> > wrote:
> > >> >
> > >> >> FYI: Based on Ewen's suggestion from the related JIRA, I added a
> > >> >> clarification to the KIP that it doesn't do anything around
> > >> authorization
> > >> >> /
> > >> >> ACLs. While authorization / ACLs would be for sure valuable
> feature I
> > >> >> would
> > >> >> prefer to leave it for different KIP.
> > >> >>
> > >> >> Jakub
> > >> >>
> > >> >> On Mon, Oct 9, 2017 at 5:25 PM, Jakub Scholz <ja...@scholz.cz>
> > wrote:
> > >> >>
> > >> >> > Hi,
> > >> >> >
> > >> >> > I would like to start a discussion about KIP-208: Add SSL support
> > to
> > >> >> Kafka
> > >> >> > Connect REST interface (https://cwiki.apache.org/
> > >> >> > confluence/display/KAFKA/KIP-208%3A+Add+SSL+support+to+
> > >> >> > Kafka+Connect+REST+interface).
> > >> >> >
> > >> >> > I think this would be useful feature to improve the security of
> > Kafka
> > >> >> > Connect.
> > >> >> >
> > >> >> > Thanks & Regards
> > >> >> > Jakub
> > >> >> >
> > >> >>
> > >> >
> > >> >
> > >>
> > >
> > >
> >
>

Reply via email to