Hi everyone,
Apologies for the very delayed response. Thank you both for the feedback. > For clarity it might make sense to mention this feature will be useful when using a ConfigProvider with Kafka Connect as providers are set in the runtime and can then be used by connectors. This feature has no use when using a ConfigProvider in server.properties or in clients. I have updated the KIP to address this suggestion. Please let me know if it's not clear enough. > When trying to use a path not allowed, you propose returning an error. With Connect does that mean the connector will be failed? The EnvVarConfigProvider returns empty string in case a user tries to access an environment variable not allowed. I wonder if we should follow the same pattern so the behavior is "consistent" across all built-in providers. I agree with this, it makes sense to have consistent behaviour across all the providers. I made this update. > 1. In the past Connect removed the FileStream connectors in order to prevent a REST API attacker from accessing the filesystem. Is this the only remaining attack vector for reading the file system? Meaning, if this feature is configured and all custom plugins are audited for filesystem accesses, would someone with access to the REST API be unable to access arbitrary files on disk? Once this feature is configured, it will stop someone from accessing the file system via config providers. However, I’m not sure whether there are other ways users can access file systems via REST API. Mickael, perhaps you have some thoughts on this? > 2. Could you explain how this feature would prevent a path traversal attack, and how we will verify that such attacks are not feasible? The intention is to generate File objects based on the String value provided for allowed.paths and the String path passed to the get() function. This would allow validation of path inclusion within the specified allowed paths using their corresponding Path objects, rather than doing String comparisons. This hopefully will mitigate the risk of path traversal. The implementation should include unit tests to verify this. > 3. This applies a single "allowed paths" to a whole worker, but I've seen situations where preventing one connector from accessing another's secrets may also be desirable. Is there any way to extend this feature now or in the future to make that possible? One approach could be creating multiple providers, each assigned a unique name and specific allowed.paths configuration. Users would then be assigned a provider name, granting them appropriate access on the file system to load variables for their connectors. However, during provider configuration, administrators would have to anticipate and specify the files and directories users may require access to. Regards, Tina On Wed, Nov 8, 2023 at 7:49 PM Greg Harris <greg.har...@aiven.io.invalid> wrote: > Hey Tina, > > Thanks for the KIP! Unrestricted file system access over a REST API is > an unfortunate anti-pattern, so I'm glad that you're trying to change > it. I had a few questions, mostly from the Connect perspective. > > 1. In the past Connect removed the FileStream connectors in order to > prevent a REST API attacker from accessing the filesystem. Is this the > only remaining attack vector for reading the file system? Meaning, if > this feature is configured and all custom plugins are audited for > filesystem accesses, would someone with access to the REST API be > unable to access arbitrary files on disk? > 2. Could you explain how this feature would prevent a path traversal > attack, and how we will verify that such attacks are not feasible? > 3. This applies a single "allowed paths" to a whole worker, but I've > seen situations where preventing one connector from accessing > another's secrets may also be desirable. Is there any way to extend > this feature now or in the future to make that possible? > > Thanks! > Greg > > On Tue, Nov 7, 2023 at 7:06 AM Mickael Maison <mickael.mai...@gmail.com> > wrote: > > > > Hi Tina, > > > > Thanks for the KIP. > > For clarity it might make sense to mention this feature will be useful > > when using a ConfigProvider with Kafka Connect as providers are set in > > the runtime and can then be used by connectors. This feature has no > > use when using a ConfigProvider in server.properties or in clients. > > > > When trying to use a path not allowed, you propose returning an error. > > With Connect does that mean the connector will be failed? The > > EnvVarConfigProvider returns empty string in case a user tries to > > access an environment variable not allowed. I wonder if we should > > follow the same pattern so the behavior is "consistent" across all > > built-in providers. > > > > Thanks, > > Mickael > > > > On Tue, Nov 7, 2023 at 1:52 PM Gantigmaa Selenge <gsele...@redhat.com> > wrote: > > > > > > Hi everyone, > > > > > > Please let me know if you have any comments on the KIP. > > > > > > I will leave it for a few more days. If there are still no comments, I > will > > > start the vote on it. > > > > > > Regards, > > > Tina > > > > > > On Wed, Oct 25, 2023 at 8:31 AM Gantigmaa Selenge <gsele...@redhat.com > > > > > wrote: > > > > > > > Hi everyone, > > > > > > > > I would like to start a discussion on KIP-933 that proposes > restricting > > > > files accessed by File and Directory ConfigProviders. > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-993%3A+Allow+restricting+files+accessed+by+File+and+Directory+ConfigProviders > > > > > > > > Regards, > > > > Tina > > > > > >
