HIVE-27322: Iceberg: metadata location overrides can cause data breach. #4673
Conversation
| // this property is set during the create operation before the hive table was created | ||
| // we are returning a dummy iceberg metadata file | ||
| authURI.append(encodeString(URI.create(locationProperty.get()).getPath())) | ||
| .append(encodeString("/metadata/dummy.metadata.json")); |
There was a problem hiding this comment.
old code, shows up in refactor, when location is provided in create table, the metadata.json file will be created in that location /metadata/some.json, we don't know the exact path, so we pass this dummy.json to check if we would be able to create a file inside /metadata directory, the name of the file is irrelevant here, as what I could decode from history
| @Override | ||
| public void addResourcesForCreateTable(Map<String, String> tblProps, HiveConf hiveConf) { | ||
| String metadataLocation = tblProps.get(BaseMetastoreTableOperations.METADATA_LOCATION_PROP); | ||
| if (StringUtils.isNotEmpty(metadataLocation)) { |
There was a problem hiding this comment.
Would it not be simpler if the metadataLocation is empty than throw exception?
There was a problem hiding this comment.
it would be empty in general unless someone provides the the metadata location as part of the create command. like in normal create command it would be empty
create table ice (schema) stored by iceberg;
it would be empty here.
create table ice TBLPROPERTIES(metadata-location='') stored by iceberg;
we handle this case, & if it is provided we do the logic
There was a problem hiding this comment.
may be I can change it to just null check, if it is empty iceberg should, if it doesn't resolve to anything
|
Kudos, SonarCloud Quality Gate passed!
|
…h. (apache#4673). (Ayush Saxena, reviewed by Denys Kuzmenko, Attila Turoczy)
What changes were proposed in this pull request?
Pass metadata location to authorizer if that is provided as part of create table
Why are the changes needed?
Better Authorizaton checks
Does this PR introduce any user-facing change?
Yes, metadata location will go through auth checks while creating table if they are specified.
Is the change a dependency upgrade?
No
How was this patch tested?
UT