Re: [PR] DRILL-8465: Check Input Data for Iceberg Plugin (drill)
jnturton commented on PR #2853: URL: https://github.com/apache/drill/pull/2853#issuecomment-1878847019 Got it @pjfanning. Let's discuss further in the right forum. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@drill.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] DRILL-8465: Check Input Data for Iceberg Plugin (drill)
pjfanning commented on PR #2853: URL: https://github.com/apache/drill/pull/2853#issuecomment-1878512092 The short background to this in this link - https://lists.apache.org/thread/vpjz467rg8449m63v1n9nl3o56twwyzt (a private thread requiring ASF login). I'm no expert on Iceberg or the Drill Iceberg Plugin but I was hoping to maybe engage with someone who knows more about how they work and to get an understanding of if we need some constraints. Due to the security aspect of this, I'm not too comfortable going into more detail here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@drill.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] DRILL-8465: Check Input Data for Iceberg Plugin (drill)
jnturton commented on PR #2853: URL: https://github.com/apache/drill/pull/2853#issuecomment-1878400693 I've started looking at this. First question: if we're adding dynamically loaded class checks to protect against untrusted code then is checking the package name worth much? Or do we need to do something like verify signatures against a list of trusted keys? Second question: if this is about security then is the code we're loading actually untrusted or is it only ever loaded from serialisations that we produced ourselves (e.g. in IcebergWorkSerializer)? P.S. Please include this "Why we're doing this" background that I'm lacking in the Jira issue when it's nontrivial. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@drill.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] DRILL-8465: Check Input Data for Iceberg Plugin (drill)
cgivre commented on PR #2853: URL: https://github.com/apache/drill/pull/2853#issuecomment-1875413801 @jnturton Do you have any thoughts here? This seems like this would be a good PR to get into the bug fix release. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@drill.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] DRILL-8465: Check Input Data for Iceberg Plugin (drill)
pjfanning commented on PR #2853: URL: https://github.com/apache/drill/pull/2853#issuecomment-1849019362 @cgivre unfortunately, IcebergWork is also created by IcebergWorkDeserializer but this class is constructed using reflection based on `@JsonDeserialize(using = IcebergWork.IcebergWorkDeserializer.class)`. There is no point in changing IcebergWork unless we can find a way to inject the config value into IcebergWorkDeserializer too. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@drill.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] DRILL-8465: Check Input Data for Iceberg Plugin (drill)
cgivre commented on PR #2853: URL: https://github.com/apache/drill/pull/2853#issuecomment-1849004555 > @cgivre I'm not that familiar with how Drill plugin configurations work. If I was to extend IcebergFormatPluginConfig to add a configurable allowPackageList (`String[]` or `List` -- whichever works best) -- how would I go about accessing this from the IcebergWork class. It is IcebergWork where I need to apply the allow list? It think the thing to do would be to add an argument to the `IcebergWork` constructor. Then once that's done, it looks like the `IcebergWork` is instantiated in the `IcebergGroupScan`. https://github.com/apache/drill/blob/fe57fd11bf61a0835e4295f6822c4e1b0a045bfa/contrib/format-iceberg/src/main/java/org/apache/drill/exec/store/iceberg/IcebergGroupScan.java#L236-Ll44 The GroupScan has access to the `IcebergPlugin` and from there you can access the IcebergConfig. Does that make sense? > > @vvysotskyi you appear to have written most of the Iceberg code. Would you have any idea if this issue is one that we need to worry about? If it is, it looks like it will be hard to get the config values injected into IcebergWork because the class seems to only be instantiated by a custom Jackson Deserializer that itself only created only Java reflection. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@drill.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] DRILL-8465: Check Input Data for Iceberg Plugin (drill)
pjfanning commented on PR #2853: URL: https://github.com/apache/drill/pull/2853#issuecomment-1848951847 @cgivre I'm not that familiar with how Drill plugin configurations work. If I was to extend IcebergFormatPluginConfig to add a configurable allowPackageList (`String[]` or `List` -- whichever works best) -- how would I go about accessing this from the IcebergWork class. It is IcebergWork where I need to apply the allow list. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@drill.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
