Hmm... Maybe we need to add some way to serialize and deserialize
KafkaPrincipal subclasses to/from string. We could add a method to
KafkaPrincipalBuilder#deserialize and a method KafkaPrincipal#serialize, I
suppose.
best,
Colin
On Thu, Apr 23, 2020, at 02:14, Tom Bentley wrote:
> Hi folks,
>
> Colin wrote:
>
> > The final broker knows it can trust the principal name in the envelope
> > (since EnvelopeRequest requires CLUSTERACTION on CLUSTER). So it can use
> > that principal name for authorization (given who you are, what can you
> > do?) The forwarded principal name will also be used for logging.
> >
>
> My understanding (and I'm happy to be corrected) is that a custom
> authoriser might rely on the KafkaPrincipal instance being a subclass of
> KafkaPrincipal (e.g. the subclass has extra fields with the principal's
> "roles"). So you can't construct a KafkaPrinicpal on the controller which
> would be guaranteed to work for arbitrary authorizers. You have to perform
> authorization on the first broker (rejecting some of the batched requests),
> forward the authorized ones to the controller, which then has to trust that
> the authorization has been done and make the ZK writes without
> authorization. Because the controller cannot invoke the authorizer that
> means that the authorizer audit logging (on the controller) would not
> include these operations. But they would be in the audit logging from the
> original broker, so the information would not be lost.
>
> There's also a problem with using the constructed principal for other
> logging (i.e. non authorizer logging) on the controller: There's nothing
> stopping a custom KafkaPrincipal subclass from overriding toString() to
> return something different from "${type}:${name}". If you're building a
> "fake" KafkaPrincipal on the controller from the type and name then its
> toString() would be "wrong". A solution to this would be to also serialize
> the toString() into the envelope and have some ProxiedKafkaPrincipal class
> which you instantiate on the controller which has overridden toString to
> return the right value. Obviously you could optimize this using an optional
> field so you only serialize the toString() if it differed from the value
> you'd get from KafkaPrincipal.toString(). Maybe non-audit logging using the
> wrong string value of a principal is sufficiently minor that this isn't
> even worth trying to solve.
>
> Kind regards,
>
> Tom
>
>
> On Wed, Apr 22, 2020 at 10:59 PM Sönke Liebau
> <soenke.lie...@opencore.com.invalid> wrote:
>
> > Hi Colin,
> >
> > thanks for your summary! Just one question - and I may be missing an
> > obvious point here..
> > You write:
> >
> > "The initial broker should do authentication (who are you?) and come up
> > with a principal name. Then it creates an envelope request, which will
> > contain that principal name, and sends it along with the unmodified
> > original request to the final broker. [... ] The final broker knows it
> > can trust the principal name in the envelope (since EnvelopeRequest
> > requires CLUSTERACTION on CLUSTER). So it can use that principal name for
> > authorization (given who you are, what can you do?) "
> >
> > My understanding is, that you don't want to serialize the Principal (due to
> > the discussed issues with custom principals) but reduce the principal down
> > to a string representation that would be used for logging and
> > authorization?
> > If that understanding is correct then I don't think we could use the
> > regular Authorizer on the target broker, because that would need the actual
> > principal object to work on.
> >
> > Also, a thought that just occurred to me, we might actually need to log
> > different principal strings for the case of queries like AlterConfigs
> > (mentioned by Rajini) which may contain multiple resources. Take an LDAP
> > authorizer that grants access based on group membership - the same
> > alterconfig request may contain resources that are authorized based on
> > group1 as well as resources authorized based on membership in group 2 ..
> > And in all cases we'd need to log the specific reason I think..
> >
> > Basically I think that we might have a hard time properly authorizing and
> > logging without being able to forward the entire principal.. but again, I
> > might be heading down an entirely wrong path here :)
> >
> > Best regards,
> > Sönke
> >
> >
> >
> >
> >
> >
> >
> >
> > On Wed, 22 Apr 2020 at 23:13, Guozhang Wang <wangg...@gmail.com> wrote:
> >
> > > Colin, Boyang: thanks for the updates, I agree that an EnvelopeRequest
> > > would be a less vulnerable approach than optional fields, and I'm just
> > > wondering if we would keep the EnvelopeRequest for a long time. I was
> > > thinking that, potentially if we require clients to be on newer version
> > > when talking to a 3.0+ (assuming 3.0 is the bridge release) brokers, then
> > > we do not need to keep this code for too long, but I think that would be
> > a
> > > very hasty compatibility breaking so maybe we indeed need to keep this
> > > forwarding mechanism many years.
> > >
> > > Regarding future use cases, I think the example that Boyang mentioned may
> > > not be very practical honestly, because when there's a connectivity
> > issue,
> > > it is either a network partition between "controller, A | B", or
> > > "controller | A, B". In other words, if the controller can talk to A,
> > then
> > > very likely A would not be able to talk to B either... anyways, since the
> > > forwarding would be there for a sufficiently long time, I think keeping
> > the
> > > additional envelope makes sense.
> > >
> > >
> > > Guozhang
> > >
> > > On Wed, Apr 22, 2020 at 1:47 PM Boyang Chen <reluctanthero...@gmail.com>
> > > wrote:
> > >
> > > > Thanks Colin for the summary! And Guozhang, regarding the future use
> > > cases,
> > > > consider a scenario where there are temporary connectivity issue
> > between
> > > > controller to a fellow broker A, the controller could then leverage
> > > another
> > > > healthy broker B to do a forwarding request to A in order to maintain a
> > > > communication.
> > > >
> > > > Even for KIP-590 scope, the forwarding mechanism shall not be transit
> > as
> > > we
> > > > do need to support older version of admin clients for a couple of years
> > > > IIUC.
> > > >
> > > > Boyang
> > > >
> > > > On Wed, Apr 22, 2020 at 1:29 PM Colin McCabe <cmcc...@apache.org>
> > wrote:
> > > >
> > > > > Hi all,
> > > > >
> > > > > I guess the way I see this working is that the request gets sent from
> > > the
> > > > > client, to the initial broker, and then forwarded to the final
> > broker.
> > > > >
> > > > > The initial broker should do authentication (who are you?) and come
> > up
> > > > > with a principal name. Then it creates an envelope request, which
> > will
> > > > > contain that principal name, and sends it along with the unmodified
> > > > > original request to the final broker. (I agree with Tom and Rajini
> > > that
> > > > we
> > > > > can't forward the information needed to do authentication on the
> > final
> > > > > broker, but I also think we don't need to, since we can do it on the
> > > > > initial broker.)
> > > > >
> > > > > The final broker knows it can trust the principal name in the
> > envelope
> > > > > (since EnvelopeRequest requires CLUSTERACTION on CLUSTER). So it can
> > > use
> > > > > that principal name for authorization (given who you are, what can
> > you
> > > > > do?) The forwarded principal name will also be used for logging.
> > > > >
> > > > > One question is why we need an EnvelopeRequest. Well, if we don't
> > have
> > > > an
> > > > > EnvelopeRequest, we need somewhere else to put the forwarded
> > principal
> > > > > name. I don't think overriding an existing field (like clientId) is
> > a
> > > > good
> > > > > option for this. It's messy, and loses information. It also raises
> > > the
> > > > > question of how the final broker knows that the clientId in the
> > > received
> > > > > message is not "really" a clientId, but is a principal name. Without
> > > an
> > > > > envelope, there's no way to clearly mark a request as forwarded, so
> > > > there's
> > > > > no reason for the final broker to treat this differently than a
> > regular
> > > > > clientId (or whatever).
> > > > >
> > > > > We talked about using optional fields to contain the forwarded
> > > principal
> > > > > name, but this is also messy and potentially dangerous. Older
> > brokers
> > > > will
> > > > > simply ignore the optional fields, which could result in them
> > executing
> > > > > operations as the wrong principal. Of course, this would require a
> > > > > misconfiguration in order to happen, but it still seems better to set
> > > up
> > > > > the system so that this misconfiguration is detected, rather than
> > > > silently
> > > > > ignored.
> > > > >
> > > > > It's true that the need for forwarding is "temporary" in some sense,
> > > > since
> > > > > we only need it for older clients. However, we will want to support
> > > > these
> > > > > older clients for many years to come.
> > > > >
> > > > > I agree that the usefulness of EnvelopeRequest is limited by it
> > being a
> > > > > superuser-only request at the moment. Perhaps there are some changes
> > > to
> > > > > how custom principals work that would allow us to get around this in
> > > the
> > > > > future. We should think about that so that we have this
> > functionality
> > > in
> > > > > the future if it's needed.
> > > > >
> > > > > best,
> > > > > Colin
> > > > >
> > > > >
> > > > > On Wed, Apr 22, 2020, at 11:21, Guozhang Wang wrote:
> > > > > > Hello Gwen,
> > > > > >
> > > > > > The purpose here is for maintaining compatibility old clients, who
> > do
> > > > not
> > > > > > have functionality to do re-routing admin requests themselves. New
> > > > > clients
> > > > > > can of course do this themselves by detecting who's the controller.
> > > > > >
> > > > > >
> > > > > > Hello Colin / Boyang,
> > > > > >
> > > > > > Regarding the usage of the envelope, I'm curious what are the
> > > potential
> > > > > > future use cases that would require request forwarding and hence
> > > > envelope
> > > > > > would be useful? Originally I thought that the forwarding mechanism
> > > is
> > > > > only
> > > > > > temporary as we need it for the bridge release, and moving forward
> > we
> > > > > will
> > > > > > get rid of this to simplify the code base. If we do have valid use
> > > > cases
> > > > > in
> > > > > > the future which makes us believe that request forwarding would
> > > > actually
> > > > > be
> > > > > > a permanent feature retained on the broker side, I'm on board with
> > > > adding
> > > > > > the envelope request protocol.
> > > > > >
> > > > > >
> > > > > >
> > > > > > Guozhang
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Wed, Apr 22, 2020 at 8:22 AM Gwen Shapira <g...@confluent.io>
> > > > wrote:
> > > > > >
> > > > > > > Hey Boyang,
> > > > > > >
> > > > > > > Sorry if this was already discussed, but I didn't see this as
> > > > rejected
> > > > > > > alternative:
> > > > > > >
> > > > > > > Until now, we always did client side routing - the client itself
> > > > found
> > > > > the
> > > > > > > controller via metadata and directed requests accordingly.
> > Brokers
> > > > that
> > > > > > > were not the controller, rejected those requests.
> > > > > > >
> > > > > > > Why did we decide to move to broker side routing? Was the
> > > client-side
> > > > > > > option discussed and rejected somewhere and I missed it?
> > > > > > >
> > > > > > > Gwen
> > > > > > >
> > > > > > > On Fri, Apr 3, 2020, 4:45 PM Boyang Chen <
> > > reluctanthero...@gmail.com
> > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hey all,
> > > > > > > >
> > > > > > > > I would like to start off the discussion for KIP-590, a
> > follow-up
> > > > > > > > initiative after KIP-500:
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-590%3A+Redirect+Zookeeper+Mutation+Protocols+to+The+Controller
> > > > > > > >
> > > > > > > > This KIP proposes to migrate existing Zookeeper mutation paths,
> > > > > including
> > > > > > > > configuration, security and quota changes, to controller-only
> > by
> > > > > always
> > > > > > > > routing these alterations to the controller.
> > > > > > > >
> > > > > > > > Let me know your thoughts!
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > Boyang
> > > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > -- Guozhang
> > > > > >
> > > > >
> > > >
> > >
> > >
> > > --
> > > -- Guozhang
> > >
> >
> >
> > --
> > Sönke Liebau
> > Partner
> > Tel. +49 179 7940878
> > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany
> >
>
