hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yi Liu (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-10693) Implementation of AES-CTR CryptoCodec using JNI to OpenSSL
Date Fri, 27 Jun 2014 00:38:24 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-10693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14045387#comment-14045387

Yi Liu commented on HADOOP-10693:

Thanks [~cmccabe] for review :-).  I will update the patch later.
There are a bunch of warnings that this code generates that we should fix:
Right, I also noticed them, actually it was caused by the fix of one previous comment:
+  if (!dlsym_EVP_CipherUpdate(context, (unsigned char *)output_bytes,  \
+      &output_len, (unsigned char *)input_bytes, input_len)) {
These casts could be avoided if you just made the type of {{output_bytes}} and {{input_bytes}}
OK, I will change it back.

I ran {{TestCryptoCodec}} and it succeeded whether or not {{openssl.so}} was installed. Can
you make the test fail when {{-Drequire.openssl}} is specified and {{OpenSSLAESCTRCryptoCodec}}
is not available? You should be able to check {{System#getProperty}} or something like that.
That way we could be sure our test was covering this on upstream Jenkins.
Right, {{TestCryptoCodec}} checks {{OpenSSLCipher.isNativeCodeLoaded()}} to decide whether
to test  OpenSSLAESCTRCryptoCodec, this is the same as in {{TestCodec}}
Your suggestion is good, let’s assert the exception.

A comment about naming. I find names like {{testJCEAESCTRCryptoCodec}} awfully hard to read.
Once you get 10 capital letters in a row, it just kind of mashes together. I would prefer
something like {{testJceAesCtrCryptoCodec}}, where we capitalize only the first letter of
each acronym. I think it would make sense to rename some of this stuff in that way... what
do you think?
Agree, let’s change the name.

Think about what happens if we manage to get {{jKey}}, but getting {{jlv}} fails. Then we'll
throw, but never call {{ReleaseByteArrayElements}} on {{jKey}}.
Yes, there is potential issue, let’s check them separately. 

> Implementation of AES-CTR CryptoCodec using JNI to OpenSSL
> ----------------------------------------------------------
>                 Key: HADOOP-10693
>                 URL: https://issues.apache.org/jira/browse/HADOOP-10693
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: security
>    Affects Versions: fs-encryption (HADOOP-10150 and HDFS-6134)
>            Reporter: Yi Liu
>            Assignee: Yi Liu
>             Fix For: fs-encryption (HADOOP-10150 and HDFS-6134)
>         Attachments: HADOOP-10693.1.patch, HADOOP-10693.2.patch, HADOOP-10693.3.patch,
HADOOP-10693.4.patch, HADOOP-10693.patch
> In HADOOP-10603, we have an implementation of AES-CTR CryptoCodec using Java JCE provider.

> To get high performance, the configured JCE provider should utilize native code and AES-NI,
but in JDK6,7 the Java embedded provider doesn't support it.
> Considering not all hadoop user will use the provider like Diceros or able to get signed
certificate from oracle to develop a custom provider, so this JIRA will have an implementation
of AES-CTR CryptoCodec using JNI to OpenSSL directly.

This message was sent by Atlassian JIRA

View raw message