See also https://cwiki.apache.org/confluence/display/KAFKA/KIP-297%3A+Externalizing+Secrets+for+Connect+Configurations, which just passed.
On Mon, Mar 19, 2018 at 11:16 PM, Ewen Cheslack-Postava <e...@confluent.io> wrote: > SSL authentication was added in KIP-208, which will be included in Kafka > 1.1.0: > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > 208%3A+Add+SSL+support+to+Kafka+Connect+REST+interface > > Connect isn't much different from the core Kafka/client configs currently > where in some security setups you need to pass in passwords directly, and > since there's various dynamic broker config improvements in the works, the > fact that Connect exposes these in a REST API doesn't make it any > different. I think the real long term solution to this is to add pluggable > password support where you could, e.g., get these values out of a separate > secrets management system instead of specifying them directly. Masking > passwords as described in this solution feels like it's more of a temporary > workaround and in order to be able to edit and update these connector > configs by working with the REST API, we'd have to address these issues > anyway. > > -Ewen > > On Mon, Mar 19, 2018 at 2:33 PM, Matt Farmer <m...@frmr.me> wrote: > > > 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 > > >>>>>> > > >>>>> > > >>>> > > >>> > > >> > > > > >
