[ 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. {quote} There are a bunch of warnings that this code generates that we should fix: {quote} Right, I also noticed them, actually it was caused by the fix of one previous comment: _{quote} {code} + if (!dlsym_EVP_CipherUpdate(context, (unsigned char *)output_bytes, \ + &output_len, (unsigned char *)input_bytes, input_len)) { {code} These casts could be avoided if you just made the type of {{output_bytes}} and {{input_bytes}} unsigned. {quote}_ OK, I will change it back. {quote} 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. {quote} 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. {quote} 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? {quote} Agree, let’s change the name. {quote} Think about what happens if we manage to get {{jKey}}, but getting {{jlv}} fails. Then we'll throw, but never call {{ReleaseByteArrayElements}} on {{jKey}}. {quote} 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 (v6.2#6252)