What’s the status of this? This is a pretty hard blocker for us to meet requirements internally to deploy connect in a distributed fashion.
@Ewen - Regarding the concern of accessing information securely - has there been any consideration of adding authentication to the connect api? > On Jan 17, 2018, at 3:55 PM, Randall Hauch <rha...@gmail.com> wrote: > > Vincent, > > Can the KIP more explicitly say that this is opt-in, and that by default > nothing will change? > > Randall > > On Tue, Jan 16, 2018 at 11:18 PM, Ewen Cheslack-Postava <e...@confluent.io> > wrote: > >> Vincent, >> >> I think with the addition of a configuration to control this for >> compatibility, people would generally be ok with it. If you want to start a >> VOTE thread, the KIP deadline is coming up and the PR looks pretty small. I >> will take a pass at reviewing the PR so we'll be ready to merge if we can >> get the KIP voted through. >> >> Thanks, >> Ewen >> >> On Fri, Jan 12, 2018 at 10:18 AM, Vincent Meng <vm...@zefr.com> wrote: >> >>> @Ted: The issue is kinda hard to reproduce. It's just something we >> observe >>> over time. >>> >>> @Ewen: I agree. Opt-in seems to be a good solution to me. To your >> question, >>> if there is no ConfDef that defines which fields are Passwords we can >> just >>> return the config as is. >>> >>> There is a PR for this KIP already. Comments/Discussions are welcome. >>> https://github.com/apache/kafka/pull/4269 >>> >>> On Tue, Jan 2, 2018 at 8:52 PM, Ewen Cheslack-Postava <e...@confluent.io >>> >>> wrote: >>> >>>> Vincent, >>>> >>>> Thanks for the KIP. This is definitely an issue we know is a problem >> for >>>> some users. >>>> >>>> I think the major problem with the KIP as-is is that it makes it >>> impossible >>>> to get the original value back out of the API. This KIP probably ties >> in >>>> significantly with ideas for securing the REST API (SSL) and adding >> ACLs >>> to >>>> it. Both are things we know people want, but haven't happened yet. >>> However, >>>> it also interacts with other approaches to adding those features, e.g. >>>> layering proxies on top of the existing API (e.g. nginx, apache, etc). >>> Just >>>> doing a blanket replacement of password values with a constant would >>> likely >>>> break things for people who secure things via a proxy (and may just not >>>> allow reads of configs unless the user is authorized for the particular >>>> connector). These are the types of concerns we like to think through in >>> the >>>> compatibility section. One option to get the masking functionality in >>>> without depending on a bunch of other security improvements might be to >>>> make this configurable so users that need this (and can forgo seeing a >>>> valid config via the API) can opt-in. >>>> >>>> Regarding your individual points: >>>> >>>> * I don't think the particular value for the masked content matters >> much. >>>> Any constant indicating a password field is good. Your value seems fine >>> to >>>> me. >>>> * I don't think ConnectorInfo has enough info on its own to do proper >>>> masking. In fact, I think you need to parse the config enough to get >> the >>>> Connector-specific ConfigDef out in order to determine which fields are >>>> Password fields. I would probably try to push this to be as central as >>>> possible, maybe adding a method to AbstractHerder that can get configs >>> with >>>> a boolean indicating whether they need to have sensitive fields >> removed. >>>> That method could deal with parsing the config to get the right >>> connector, >>>> getting the connector config, and then sanitizing any configs that are >>>> sensitive. We could have this in one location, then have the relevant >>> REST >>>> APIs just use the right flag to determine if they get sanitized or >>>> unsanitized data. >>>> >>>> That second point raises another interesting point -- what happens if >> the >>>> connector configuration references a connector which the worker serving >>> the >>>> REST request *does not know about*? In that case, there will be no >>>> corresponding ConfigDef that defines which fields are Passwords and >> need >>> to >>>> be sensitized. Does it return an error? Or just return the config as >> is? >>>> >>>> -Ewen >>>> >>>> On Thu, Dec 28, 2017 at 3:34 AM, Ted Yu <yuzhih...@gmail.com> wrote: >>>> >>>>> For the last point you raised, can you come up with a unit test that >>>> shows >>>>> what you observed ? >>>>> >>>>> Cheers >>>>> >>>>> On Mon, Dec 18, 2017 at 11:14 AM, Vincent Meng <vm...@zefr.com> >> wrote: >>>>> >>>>>> Hi all, >>>>>> >>>>>> I've created KIP-242, a proposal to secure credentials in kafka >>> connect >>>>>> rest endpoint. >>>>>> >>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>>>> 242%3A+Mask+password+in+Kafka+Connect+Rest+API+response >>>>>> >>>>>> Here are something I'd like to discuss: >>>>>> >>>>>> - The "masked" value is set to "*********" (9 stars) currently. >>> It's >>>>> an >>>>>> arbitrary value I picked. Are there any better options? >>>>>> - The proposal change is in the >>>>>> *org.apache.kafka.connect.runtime.rest.resources. >>>> ConnectorsResource* >>>>>> class, where before the response is returned we go through >> config >>>> and >>>>>> mask >>>>>> the password. This has been proven to work. However I think it's >>>>>> cleaner if >>>>>> we do the masking in >>>>>> *org.apache.kafka.connect.runtime.rest.entities.ConnectorInfo* >>>> where >>>>>> config() method can return the masked config, so that we don't >>> have >>>> to >>>>>> mask >>>>>> the value in each endpoint (and new endpoints if added in the >>>>> future). I >>>>>> ran into some issue with this. So after a while, I start seeing >>>>>> incorrect >>>>>> password being used for the connector. My conjecture is that the >>>> value >>>>>> stored in kafka has been changed to the mask value. Can someone >>>>> confirm >>>>>> this might happen with kafka connect? Feel like >>>>> *ConnectorInfo.Config()* >>>>>> is used somewhere to update connect config storage topic. >>>>>> >>>>>> If there's any comments on the KIP let me know. Thank you very >> much. >>>>>> >>>>>> -Vincent >>>>>> >>>>> >>>> >>> >>
