On Tue, May 30, 2017, at 15:55, Dong Lin wrote:
> Hey Colin,
>
> I think one big advantage of the broker side config is that it can not be
> ignored by the malicious client, right?
Hi Dong,
The scenario I was thinking of is where a malicious client communicates
directly with ZooKeeper, bypassing the broker. As far as I can see,
nothing that we do on the broker can prevent this from happening. It
has to be blocked by ZooKeeper itself.
>
> Thanks,
> Dong
>
> On Tue, May 30, 2017 at 3:53 PM, Dong Lin <lindon...@gmail.com> wrote:
>
> > Do we have an old version of bin/kafka-topics.sh which creates topic via
> > ZK and still allows user to access ZK with ACL? Another concern is that
> > some user may not have ACL service deployed in their cluster. If neither of
> > these is issue, then I would prefer the zookeeper approach instead of
> > adding a new broker config if the zookeeper approach is doable.
Unfortunately, the latest version of kafka-topics.sh still creates
topics by talking directly to ZK. It has not been converted to use the
new AdminClient, although that is planned.
best,
Colin
> >
> > However, regardless of whether we secure the zookeeper from unauthorized
> > user, I think KIP-108 should provide a solution to guarantee that all topic
> > creation logic goes through the topic creation policy.
> >
> > Thanks,
> > Dong
> >
> > On Tue, May 30, 2017 at 3:39 PM, Colin McCabe <cmcc...@apache.org> wrote:
> >
> >> It seems like, to make it really secure, we need the enforcement to be
> >> done at the ZooKepeer level. Any broker or client-side configuration
> >> can just be ignored by a malicious client. Do we have documentation or
> >> code that configures ZK to prevent unprivileged users from modifying the
> >> topic configurations?
> >>
> >> best,
> >> Colin
> >>
> >>
> >> On Tue, May 30, 2017, at 15:02, Dong Lin wrote:
> >> > Hey Ismael,
> >> >
> >> > I agree that it makes sense not to cover ZK-based topic creation with
> >> the
> >> > topic creation policy and limit ZK access to brokers only going forward.
> >> > My
> >> > point is that we need a way to disable ZK-based topic creation so that
> >> > all
> >> > topic creation goes through the topic creation policy as specified in
> >> > KIP-108. Does this make sense?
> >> >
> >> > One example solution is to add a broker-side config
> >> > "enable.zookeeper.topic.creation"
> >> > which defaults to "true". If user has overridden this config to be
> >> > "false",
> >> > then controller will delete the znode /brokers/topics/{topic} that is
> >> not
> >> > created by the controller. We probably need some trick to differentiate
> >> > between znode created by controller and znode created by outdated tools.
> >> > For example, the new controller code can add a new field "isController"
> >> > in
> >> > the znode /brokers/topics/{topic} when it creates this new znode. Then
> >> if
> >> > the znode doesn't have this field AND there is no child under this
> >> znode,
> >> > controller can be sure it is created by outdated tools and remove this
> >> > znode from zookeeper. Users who are using outdated tools to create topic
> >> > will find that the topic is not created.
> >> >
> >> > Dong
> >> >
> >> > On Tue, May 30, 2017 at 2:24 PM, Ismael Juma <ism...@juma.me.uk> wrote:
> >> >
> >> > > Hi Dong,
> >> > >
> >> > > No, ZK-based topic creation doesn't go through the policy since it
> >> doesn't
> >> > > go through the broker. Given that, I am not sure how the broker config
> >> > > would work. Can you please elaborate? It seems like the way forward
> >> is to
> >> > > limit ZK access to brokers only.
> >> > >
> >> > > Ismael
> >> > >
> >> > > On Tue, May 30, 2017 at 10:19 PM, Dong Lin <lindon...@gmail.com>
> >> wrote:
> >> > >
> >> > > > Hey Ismael,
> >> > > >
> >> > > > Thanks for the KIP. This is definitely useful.
> >> > > >
> >> > > > Does the KIP apply the topic creation policy to ZK-based topic
> >> creation?
> >> > > If
> >> > > > not, which seems to be the case from my understanding, should we
> >> have a
> >> > > new
> >> > > > broker config to disable ZK-based topic creation? This seems
> >> necessary to
> >> > > > prevent user from using stray builds to evade the topic creation
> >> policy.
> >> > > >
> >> > > > Thanks,
> >> > > > Dong
> >> > > >
> >> > > >
> >> > > >
> >> > > >
> >> > > >
> >> > > >
> >> > > > On Mon, Jan 9, 2017 at 1:42 PM, Roger Hoover <
> >> roger.hoo...@gmail.com>
> >> > > > wrote:
> >> > > >
> >> > > > > Got it. Thanks, Ismael.
> >> > > > >
> >> > > > > On Mon, Jan 9, 2017 at 10:42 AM, Ismael Juma <ism...@juma.me.uk>
> >> > > wrote:
> >> > > > >
> >> > > > > > Hi Roger,
> >> > > > > >
> >> > > > > > That's a good question. The server defaults are passed via the
> >> > > > > `configure`
> >> > > > > > method of the `Configurable` interface that is implemented by
> >> > > > > > `CreateTopicPolicy`. I'll mention this explicitly in the KIP.
> >> > > > > >
> >> > > > > > Ismael
> >> > > > > >
> >> > > > > > On Mon, Jan 9, 2017 at 6:04 PM, Roger Hoover <
> >> roger.hoo...@gmail.com
> >> > > >
> >> > > > > > wrote:
> >> > > > > >
> >> > > > > > > This is great. Thanks, Ismael.
> >> > > > > > >
> >> > > > > > > One question. When TopicDetails are passed to the policy
> >> > > > > implementation,
> >> > > > > > > would the server defaults already have been merged? If not, I
> >> > > think
> >> > > > > the
> >> > > > > > > policy also needs access to the server defaults.
> >> > > > > > >
> >> > > > > > > Cheers,
> >> > > > > > >
> >> > > > > > > Roger
> >> > > > > > >
> >> > > > > > > On Fri, Jan 6, 2017 at 9:26 AM, Ismael Juma <
> >> ism...@juma.me.uk>
> >> > > > wrote:
> >> > > > > > >
> >> > > > > > > > Thanks for the review Jun. Yes, that's a good point, I have
> >> > > updated
> >> > > > > the
> >> > > > > > > > KIP.
> >> > > > > > > >
> >> > > > > > > > Ismael
> >> > > > > > > >
> >> > > > > > > > On Fri, Jan 6, 2017 at 5:15 PM, Jun Rao <j...@confluent.io>
> >> > > wrote:
> >> > > > > > > >
> >> > > > > > > > > Hi, Ismael,
> >> > > > > > > > >
> >> > > > > > > > > Thanks for the KIP. Looks reasonable to me. To be
> >> consistent
> >> > > with
> >> > > > > the
> >> > > > > > > > > pattern used in other pluggable interfaces, we probably
> >> should
> >> > > > make
> >> > > > > > the
> >> > > > > > > > new
> >> > > > > > > > > interface configurable and closable?
> >> > > > > > > > >
> >> > > > > > > > > Jun
> >> > > > > > > > >
> >> > > > > > > > > On Fri, Jan 6, 2017 at 4:16 AM, Ismael Juma <
> >> ism...@juma.me.uk
> >> > > >
> >> > > > > > wrote:
> >> > > > > > > > >
> >> > > > > > > > > > Thanks Dan and Colin for the feedback. I updated the
> >> KIP to
> >> > > > > include
> >> > > > > > > the
> >> > > > > > > > > > addition of a validation mode. Since we need to bump the
> >> > > > protocol
> >> > > > > > > > version
> >> > > > > > > > > > for that, I also added an error message per topic to the
> >> > > > > response.
> >> > > > > > I
> >> > > > > > > > had
> >> > > > > > > > > > the latter as "Future Work", but I actually felt that it
> >> > > should
> >> > > > > be
> >> > > > > > in
> >> > > > > > > > the
> >> > > > > > > > > > first version (good to have feedback confirming that).
> >> > > > > > > > > >
> >> > > > > > > > > > Let me know if the changes look good to you.
> >> > > > > > > > > >
> >> > > > > > > > > > Ismael
> >> > > > > > > > > >
> >> > > > > > > > > > On Thu, Jan 5, 2017 at 9:54 PM, Colin McCabe <
> >> > > > cmcc...@apache.org
> >> > > > > >
> >> > > > > > > > wrote:
> >> > > > > > > > > >
> >> > > > > > > > > > > Yeah, I agree... having a validation mode would be
> >> nice.
> >> > > We
> >> > > > > > should
> >> > > > > > > > be
> >> > > > > > > > > > > explicit that passing validation doesn't 100%
> >> guarantee
> >> > > that
> >> > > > a
> >> > > > > > > > > > > subsequent call to create the topic will succeed,
> >> though.
> >> > > > > There
> >> > > > > > is
> >> > > > > > > > an
> >> > > > > > > > > > > obvious race condition there-- for example, with a
> >> plugin
> >> > > > which
> >> > > > > > > > > consults
> >> > > > > > > > > > > some external authentication system, there could be a
> >> > > change
> >> > > > to
> >> > > > > > the
> >> > > > > > > > > > > privileges in between validation and attempted
> >> creation.
> >> > > > > > > > > > >
> >> > > > > > > > > > > It also seems like we should try to provide a helpful
> >> > > > exception
> >> > > > > > > > message
> >> > > > > > > > > > > for the cases where topic creation fails. This might
> >> > > involve
> >> > > > > > > adding
> >> > > > > > > > > > > more detail about error conditions to
> >> > > CreateTopicsRequest...
> >> > > > > > right
> >> > > > > > > > now
> >> > > > > > > > > > > it just returns an error code, but a text message
> >> would be
> >> > > a
> >> > > > > nice
> >> > > > > > > > > > > addition.
> >> > > > > > > > > > >
> >> > > > > > > > > > > cheers,
> >> > > > > > > > > > > Colin
> >> > > > > > > > > > >
> >> > > > > > > > > > >
> >> > > > > > > > > > > On Thu, Jan 5, 2017, at 13:41, dan wrote:
> >> > > > > > > > > > > > it would be nice to have a dry-run or validate
> >> ability
> >> > > > added
> >> > > > > to
> >> > > > > > > > this
> >> > > > > > > > > > kip.
> >> > > > > > > > > > > > since we are offloading validation to a 3rd party
> >> > > > > implementor a
> >> > > > > > > > > random
> >> > > > > > > > > > > > user
> >> > > > > > > > > > > > can't know a priori (based solely on kafka configs)
> >> > > > whether a
> >> > > > > > > call
> >> > > > > > > > > > should
> >> > > > > > > > > > > > succeed without actually creating the topic.
> >> > > > > > > > > > > >
> >> > > > > > > > > > > > a similar case is in connect where there is a
> >> separate
> >> > > > > endpoint
> >> > > > > > > > > > > > <https://github.com/apache/kaf
> >> ka/blob/trunk/connect/
> >> > > > > > > > > > > runtime/src/main/java/org/apac
> >> > > he/kafka/connect/runtime/rest/
> >> > > > > > > > resources/
> >> > > > > > > > > > > ConnectorPluginsResource.java#L49-L58>
> >> > > > > > > > > > > > to attempt to validate a connect configuration
> >> without
> >> > > > > actually
> >> > > > > > > > > > creating
> >> > > > > > > > > > > > the connector.
> >> > > > > > > > > > > >
> >> > > > > > > > > > > > thanks
> >> > > > > > > > > > > > dan
> >> > > > > > > > > > > >
> >> > > > > > > > > > > >
> >> > > > > > > > > > > > On Thu, Jan 5, 2017 at 7:34 AM, Ismael Juma <
> >> > > > > ism...@juma.me.uk
> >> > > > > > >
> >> > > > > > > > > wrote:
> >> > > > > > > > > > > >
> >> > > > > > > > > > > > > Hi all,
> >> > > > > > > > > > > > >
> >> > > > > > > > > > > > > We've posted "KIP-108: Create Topic Policy" for
> >> > > > discussion:
> >> > > > > > > > > > > > >
> >> > > > > > > > > > > > > https://cwiki.apache.org/confl
> >> uence/display/KAFKA/KIP-
> >> > > > > > > > > > > > > 108%3A+Create+Topic+Policy
> >> > > > > > > > > > > > >
> >> > > > > > > > > > > > > Please take a look. Your feedback is appreciated.
> >> > > > > > > > > > > > >
> >> > > > > > > > > > > > > Thanks,
> >> > > > > > > > > > > > > Ismael
> >> > > > > > > > > > > > >
> >> > > > > > > > > > >
> >> > > > > > > > > >
> >> > > > > > > > >
> >> > > > > > > >
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >>
> >
> >
