[
https://issues.apache.org/jira/browse/HADOOP-14104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15924596#comment-15924596
]
Daryn Sharp commented on HADOOP-14104:
--------------------------------------
Still scrutinizing, initial comments:
Regarding need for constants, in general, would be preferable for “null” to
mean no provider instead of both empty string and null meaning no provider – in
which case a constant is not necessary.
Regarding concerns over getServerDefaults being called via webhdfs: if you are
using EZ with webhdfs, one little getServerDefaults call is the least of your
worries... You should terrified that apparently one compromised DN renders
encryption moot. At any rate, we'll see what we can do to reduce the number of
calls from webhdfs.
+DFSClient+
# {{get/setKeyProviderUri}} methods should use URI instances, not strings, as
name implies. The caller should have already done the string to uri
conversions.
# {{getServerDefaults}} never returns null, so don’t null check. Just assign
keyProviderUri and check for null.
+KMSUtil+
# If {{createKeyProviderFromUri(Configuration, String)}} fails to create a
client, it throws with a hardcoded
{{CommonConfigurationKeysPublic.HADOOP_SECURITY_KEY_PROVIDER_PATH}} which may
not be the key that was used. Although see comments in _KeyProviderCache_ that
may negate need for these changes at all.
+KeyProviderCache+
# The new {{get(Configuration,String)}} should take a URI, not String, and just
look it up. No need to perform all the String -> URI logic for every lookup
since DFSClient should call with a computed URI.
# Consider just calling {{KeyProviderFactory.get(uri, conf)}} directly instead
of roundabout trip through DFSUtilClient/KMSUtil. Then you probably don’t need
to change those classes since their methods are all about getting the key
provider from the config. We know what the URI is.
# Minor, in {{get}}, leave the unwrapping of the ExecutionException in place.
+DFSClient+
# In {{isHDFSEncryptionEnabled}}, it’s completely broken to swallow exceptions
and return false… It’s not just a StandbyException, per the comment, that may
bubble out. It could be the retry proxy giving up, connect refused, etc.
Change this method to allow IOE because DFSClient is a private interface. Then
the filesystem’s getTrashRoot can do the wrong thing by swallowing the
exception.
+DistributedFileSystem+
# I’d rather see {{addDelegationTokens}} not call the problematic
{{isHDFSEncryptionEnabled}} method, but instead call {{dfs.getKeyProvider()}}.
If not null, do the block of code to get a kms token.
I'll double check that token selection works correctly.
> Client should always ask namenode for kms provider path.
> --------------------------------------------------------
>
> Key: HADOOP-14104
> URL: https://issues.apache.org/jira/browse/HADOOP-14104
> Project: Hadoop Common
> Issue Type: Improvement
> Components: kms
> Reporter: Rushabh S Shah
> Assignee: Rushabh S Shah
> Attachments: HADOOP-14104-trunk.patch, HADOOP-14104-trunk-v1.patch,
> HADOOP-14104-trunk-v2.patch
>
>
> According to current implementation of kms provider in client conf, there can
> only be one kms.
> In multi-cluster environment, if a client is reading encrypted data from
> multiple clusters it will only get kms token for local cluster.
> Not sure whether the target version is correct or not.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
