I agree that this would be useful to have and shouldn't create issues in 99% of all cases. But it would be a breaking change to a public API. I had a quick look at the two large projects that come to mind which might be affected: Ranger and Sentry - both seem to operate directly with KafkaPrincipal instead of subclassing it. But anybody who extended KafkaPrincipal would probably need to update their code..
Writing this sparked the thought that this issue should also concern delegation tokens, as Principals need to be stored/sent around for those too. Had a brief look at the code and for Delegation Tokens we seem to use SecurityUtils#parseKafkaPrincipal[1] which would ignore any additional information from custom Principals. We'll probably want to at least add a note on that to the docs - unless it is there already, I've only looked for about 30 seconds.. Best regards, Sönke [1] https://github.com/apache/kafka/blob/e9fcfe4fb7b9ae2f537ce355fe2ab51a58034c64/clients/src/main/java/org/apache/kafka/common/utils/SecurityUtils.java#L52 On Thu, 23 Apr 2020 at 14:35, Colin McCabe <cmcc...@apache.org> wrote: > 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 > > > > > > -- Sönke Liebau Partner Tel. +49 179 7940878 OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany
