[GitHub] aremily commented on issue #92: OpenSSL 1.1.0 updates with backward compatibility for OpenSSL 1.0.2 and 1.0.1

[GitBox]

aremily commented on issue #92: OpenSSL 1.1.0 updates with backward 
compatibility for OpenSSL 1.0.2 and 1.0.1
URL: https://github.com/apache/commons-crypto/pull/92#issuecomment-467268331
 
 
   I'll have a look using OpenJDK.  To this point I've done everything on 
Oracle Java.  Will Travis support an OpenJDK build if I were to include it in 
the Travis.yml matrix?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] aremily commented on issue #92: OpenSSL 1.1.0 updates with backward compatibility for OpenSSL 1.0.2 and 1.0.1

[GitBox]

aremily commented on issue #92: OpenSSL 1.1.0 updates with backward 
compatibility for OpenSSL 1.0.2 and 1.0.1
URL: https://github.com/apache/commons-crypto/pull/92#issuecomment-465382885
 
 
   The project I'm using commons crypto for makes heavy use of a keyed 
pseudorandom function, so eventually I'd like to add CMAC and/or HMAC 
functionality to commons crypto.  Thoughts?  Good idea?  Bad idea?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] aremily commented on issue #92: OpenSSL 1.1.0 updates with backward compatibility for OpenSSL 1.0.2 and 1.0.1

[GitBox]

aremily commented on issue #92: OpenSSL 1.1.0 updates with backward 
compatibility for OpenSSL 1.0.2 and 1.0.1
URL: https://github.com/apache/commons-crypto/pull/92#issuecomment-465373572
 
 
   Great.  When will I be able to pull the version with these changes out of 
the Maven repo?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] aremily commented on issue #92: OpenSSL 1.1.0 updates with backward compatibility for OpenSSL 1.0.2 and 1.0.1

[GitBox]

aremily commented on issue #92: OpenSSL 1.1.0 updates with backward 
compatibility for OpenSSL 1.0.2 and 1.0.1
URL: https://github.com/apache/commons-crypto/pull/92#issuecomment-464273910
 
 
   I think I got all the changes.  I finally changed my c file formatter to 
stop defaulting with tabs.  Should have done that a month ago.  Please have a 
look.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] aremily commented on issue #92: OpenSSL 1.1.0 updates with backward compatibility for OpenSSL 1.0.2 and 1.0.1

[GitBox]

aremily commented on issue #92: OpenSSL 1.1.0 updates with backward 
compatibility for OpenSSL 1.0.2 and 1.0.1
URL: https://github.com/apache/commons-crypto/pull/92#issuecomment-463455606
 
 
   Please review this latest commit for incorporation into the master branch.  
Thanks.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] aremily commented on issue #92: OpenSSL 1.1.0 updates with backward compatibility for OpenSSL 1.0.2 and 1.0.1

[GitBox]

aremily commented on issue #92: OpenSSL 1.1.0 updates with backward 
compatibility for OpenSSL 1.0.2 and 1.0.1
URL: https://github.com/apache/commons-crypto/pull/92#issuecomment-462469860
 
 
   Completely spaced off that that was a compile time check.  I'll swap it out
   with a dynamic one.  What diff tool are you using?  I should clean up the
   formatting stuff before I check it in.
   
   On Mon, Feb 11, 2019 at 1:59 PM Marcelo Vanzin 
   wrote:
   
   > *@vanzin* commented on this pull request.
   >
   > Looks almost there. I still noticed a couple of places where it seems you
   > have a compile-time check disguised as a runtime check.
   > --
   >
   > In .gitignore
   > :
   >
   > > @@ -1,37 +0,0 @@
   > -*~
   >
   > Don't delete this file.
   > --
   >
   > In .travis.yml
   > :
   >
   > > @@ -24,7 +24,6 @@ matrix:
   >  - "curl -L --cookie 'oraclelicense=accept-securebackup-cookie;'  
http://download.oracle.com/otn-pub/java/jce/8/jce_policy-8.zip -o 
/tmp/policy.zip && sudo unzip -j -o /tmp/policy.zip *.jar -d `jdk_switcher home 
oraclejdk8`/jre/lib/security && rm /tmp/policy.zip"
   >after_success:
   >  - mvn clean test jacoco:report coveralls:report
   > -
   >
   > Undo.
   > --
   >
   > In Makefile
   > :
   >
   > > @@ -18,7 +18,7 @@
   >  include Makefile.common
   >
   >  COMMONS_CRYPTO_OUT:=$(TARGET)/$(commons-crypto)-$(os_arch)
   > -COMMONS_CRYPTO_OBJ:=$(addprefix 
$(COMMONS_CRYPTO_OUT)/,OpenSslCryptoRandomNative.o OpenSslNative.o 
OpenSslInfoNative.o)
   > +COMMONS_CRYPTO_OBJ:=$(addprefix 
$(COMMONS_CRYPTO_OUT)/,OpenSslInfoNative.o OpenSslCryptoRandomNative.o 
OpenSslNative.o)
   >
   > Nothing is changing here, right? If not, undo.
   > --
   >
   > In src/main/java/org/apache/commons/crypto/jna/OpenSslJnaCryptoRandom.java
   > :
   >
   > > @@ -103,10 +103,9 @@ public void nextBytes(byte[] bytes) {
   >  //to support multithreading 
https://wiki.openssl.org/index.php/Manual:Threads(3) needs to be done
   >
   >  if(rdrandEnabled && 
OpenSslNativeJna.RAND_get_rand_method().equals(OpenSslNativeJna.RAND_SSLeay())) 
{
   > -close();
   >
   > Why do you need to remove this?
   > --
   >
   > In src/main/native/org/apache/commons/crypto/OpenSslInfoNative.c
   > :
   >
   > > @@ -135,3 +96,35 @@ JNIEXPORT jstring JNICALL 
Java_org_apache_commons_crypto_OpenSslInfoNative_Nativ
   >  {
   >  return (*env)->NewStringUTF(env, PROJECT_NAME);
   >  }
   > +
   > +JNIEXPORT jlong JNICALL 
Java_org_apache_commons_crypto_OpenSslInfoNative_OpenSSL
   > +  (JNIEnv *env, jclass clazz)
   > +{
   > +if (!load_library(env)) {
   > +return 0;
   > +}
   > +if(OPENSSL_VERSION_NUMBER > VERSION_1_1_X){
   >
   > nit: space after if (also in some other places)
   >
   > Where does OPENSSL_VERSION_NUMBER come from? A quick google search seems
   > to indicate it's a macro in an OpenSSL header. Which makes this a compile
   > time check, basically.
   > --
   >
   > In src/main/native/org/apache/commons/crypto/cipher/OpenSslNative.c
   > :
   >
   > > @@ -716,3 +677,52 @@ JNIEXPORT void JNICALL 
Java_org_apache_commons_crypto_cipher_OpenSslNative_clean
   >EVP_CTX_Wrapper *wrapper = CTX_WRAPPER(ctx);
   >free_context_wrapper(wrapper);
   >  }
   > +
   > +static int check_update_max_output_len(JNIEnv *env, EVP_CIPHER_CTX 
*context, jlong ctx, int input_len, int max_output_len)
   >
   > All these arguments feel a little redundant. e.g. you don't need context
   > since you can just use wrapper->context.
   >
   > With some small changes you could also just pass the wrapper directly from
   > the callers, instead of passing the jni env + the pointer address.
   > --
   >
   > In src/main/native/org/apache/commons/crypto/cipher/OpenSslNative.c
   > :
   >
   > > +  }
   > +  }
   > +  return 0;
   > +  }
   > +}
   > +
   > +static int check_doFinal_max_output_len(JNIEnv *env, EVP_CIPHER_CTX 
*context, int max_output_len)
   > +{
   > +  if (dlsym_EVP_CIPHER_CTX_test_flags(context, EVP_CIPH_NO_PADDING) == 
EVP_CIPH_NO_PADDING) {
   > +return 1;
   > +  } else {
   > +int b = dlsym_EVP_CIPHER_CTX_block_size(context);
   > +if (max_output_len >= b) {
   > +  return 1;
   > +}
   > +  return 0;
   >
 

[GitHub] aremily commented on issue #92: OpenSSL 1.1.0 updates with backward compatibility for OpenSSL 1.0.2 and 1.0.1

[GitBox]

aremily commented on issue #92: OpenSSL 1.1.0 updates with backward 
compatibility for OpenSSL 1.0.2 and 1.0.1
URL: https://github.com/apache/commons-crypto/pull/92#issuecomment-462098898
 
 
   Please review this most recent commit for compliance with the requirements 
for merging into the master branch.  I've ran mvn clean site against OpenSSL 
1.0.1, 1.0.2 and 1.1.0.  All version handling is done at runtime.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] aremily commented on issue #92: OpenSSL 1.1.0 updates with backward compatibility for OpenSSL 1.0.2 and 1.0.1

[GitBox]

aremily commented on issue #92: OpenSSL 1.1.0 updates with backward 
compatibility for OpenSSL 1.0.2 and 1.0.1
URL: https://github.com/apache/commons-crypto/pull/92#issuecomment-462013332
 
 
   Travis is running OpenSSL 1.0.1, correct?  I've been developing to 1.0.2
   and 1.1.0.  I'm guessing that the build errors on Travis are version
   related, so I'll stand up a 1.0.1 environment and see if I can recreate the
   failure.
   
   On Thu, Feb 7, 2019 at 1:58 PM Marcelo Vanzin 
   wrote:
   
   > If I understand context->encrypt correctly, it just tracks whether the
   > context is being used for encryption or decryption, right?
   >
   > If that's the case, you could stash that information in the
   > EVP_CTX_Wrapper created by the commons-crypto library. Set it in
   > Java_org_apache_commons_crypto_cipher_OpenSslNative_init, and change some
   > internal functions to take the wrapper instead of the OpenSSL context
   > directly.
   >
   > Does that solve your problem?
   >
   > —
   > You are receiving this because you authored the thread.
   > Reply to this email directly, view it on GitHub
   > ,
   > or mute the thread
   > 

   > .
   >
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] aremily commented on issue #92: OpenSSL 1.1.0 updates with backward compatibility for OpenSSL 1.0.2 and 1.0.1

[GitBox]

aremily commented on issue #92: OpenSSL 1.1.0 updates with backward 
compatibility for OpenSSL 1.0.2 and 1.0.1
URL: https://github.com/apache/commons-crypto/pull/92#issuecomment-462010162
 
 
   It does.  Thank you.  I ran the build successfully with 1.1.0 on Mac OSX
   and 1.0.2 on Ubuntu 16.04, but when I pushed to GitHub Travis fails testing
   because of a system crash that I can't reproduce.  Not sure how to
   proceed.  Can you have a look?  I think it's what you're looking for in
   terms of support to 1.0.2 and 1.1.0.
   
   On Thu, Feb 7, 2019 at 1:58 PM Marcelo Vanzin 
   wrote:
   
   > If I understand context->encrypt correctly, it just tracks whether the
   > context is being used for encryption or decryption, right?
   >
   > If that's the case, you could stash that information in the
   > EVP_CTX_Wrapper created by the commons-crypto library. Set it in
   > Java_org_apache_commons_crypto_cipher_OpenSslNative_init, and change some
   > internal functions to take the wrapper instead of the OpenSSL context
   > directly.
   >
   > Does that solve your problem?
   >
   > —
   > You are receiving this because you authored the thread.
   > Reply to this email directly, view it on GitHub
   > ,
   > or mute the thread
   > 

   > .
   >
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] aremily commented on issue #92: OpenSSL 1.1.0 updates with backward compatibility for OpenSSL 1.0.2 and 1.0.1

[GitBox]

aremily commented on issue #92: OpenSSL 1.1.0 updates with backward 
compatibility for OpenSSL 1.0.2 and 1.0.1
URL: https://github.com/apache/commons-crypto/pull/92#issuecomment-460892037
 
 
   Time got away from me over the holidays, but I finally got back to this
   project.  Most of the conversion was indeed trivial, but between OpenSSL
   version 1.0.X and 1.1.X the EVP_CIPHER_CTX structure was made opaque, such
   that in 1.1.X the elements of the struct can only be accessed using the
   appropriate accessor function.  My specific challenge is that I can't
   compile context->encrypt in 1.1.X, and I can't find an
   EVP_CIPHER_CTX_encrypting function in 1.0.X.  I've tried mirroring the
   struct locally and casting it, but the structures didn't align properly and
   "encrypt" didn't have a value of 1 or 0.  If you know of a straightforward
   solution for sneaking a data type passed the compiler please share it.
   Otherwise, I'm pondering a few ways forward, in no particular order.
   
   1.  Pass the "encrypt" flag from the Java environment.  This would be very
   invasive to the existing codebase and may even change the API.  Probably a
   lot of refactoring.
   2.  Build a separate library that contains the required functions and dlsym
   them into the OpenSslNative.c object.  I think this would allow us to get
   the context->encrypt reference through the 1.1.0 compiler, but we would
   have to package another library with the distribution.  I'm also not sure
   how much 1.0.X code we'd have to pull over into the legacy support library
   in order to get all the required dependencies.
   3.  Limit the functionality of the Commons Crypto library to avoid
   context->encrypt.  The only place I can see that it's used is in the
   AEADBadTagException checking.  Two of 113 tests error out.  Everything else
   passes, but for that tiny snippet of code.  How important is the bad tag
   checking?
   
   Any advice?
   
   On Thu, Jan 3, 2019 at 2:40 PM Marcelo Vanzin 
   wrote:
   
   > If you use runtime checks, you don't need to touch the build at all. The
   > same JNI library should work everywhere. If you test on a 1.1 system, it
   > should cover everything (since Travis here will test your changes on a
   > 1.0.2 system).
   >
   > If you use compile-time checks ("ifdefs"), then you need to build
   > different libraries, which is why I prefer the approach above.
   >
   > —
   > You are receiving this because you authored the thread.
   > Reply to this email directly, view it on GitHub
   > ,
   > or mute the thread
   > 

   > .
   >
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] aremily commented on issue #92: OpenSSL 1.1.0 updates with backward compatibility for OpenSSL 1.0.2 and 1.0.1

[GitBox]

aremily commented on issue #92: OpenSSL 1.1.0 updates with backward 
compatibility for OpenSSL 1.0.2 and 1.0.1
URL: https://github.com/apache/commons-crypto/pull/92#issuecomment-451253240
 
 
   It's building the JNI library or libraries that I'll have to work out.  I
   don't routinely code in C, so I need to do some research about how to build
   multi-version support into one library, if that's even possible, or to
   compile a separate JNI library for each supported version.
   
   On Thu, Jan 3, 2019 at 2:24 PM Marcelo Vanzin 
   wrote:
   
   > It really should not be hard. The concept is the same you're using on the
   > JNA side: check things at runtime. That means instead of using #ifdefs
   > and that macro that returns if a symbol is not found, you write code that
   > tries different things depending on whether a symbol is found or not.
   >
   > —
   > You are receiving this because you authored the thread.
   > Reply to this email directly, view it on GitHub
   > ,
   > or mute the thread
   > 

   > .
   >
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] aremily commented on issue #92: OpenSSL 1.1.0 updates with backward compatibility for OpenSSL 1.0.2 and 1.0.1

[GitBox]

aremily commented on issue #92: OpenSSL 1.1.0 updates with backward 
compatibility for OpenSSL 1.0.2 and 1.0.1
URL: https://github.com/apache/commons-crypto/pull/92#issuecomment-451248384
 
 
   Thanks for the feedback.  Let me think on it a bit more and I'll take it up
   again once I've come up with an approach.  Perhaps ossdev07 would be
   interested in collaborating, as it seems we're both trying to achive the
   same objective.
   
   On Wed, Jan 2, 2019 at 2:00 PM Marcelo Vanzin 
   wrote:
   
   > *@vanzin* commented on this pull request.
   >
   > Sorry, but your code has exactly the same problem as other attempts at
   > doing this.
   >
   > Anytime you compile things conditionally, it means the JNI library only
   > works against a specific version of OpenSSL. We can't have that, unless you
   > also change the Java code to load different JNI libraries depending on the
   > OpenSSL version, and the libraries are created with a different name
   > depending on the version. And I don't see your code doing that.
   >
   > (I also have a preference for a single JNI library, but if you do the
   > alternative I won't complain too much.)
   >
   > I would also be good to add 1.1 to the test matrix; seems like the latest
   > Linux on Travis is 16.04, which still has 1.0.2; so maybe re-enabling
   > MacOS? See the .travis.yaml change in:
   > dc28a2b
   > 

   > --
   >
   > In .gitignore
   > :
   >
   > > @@ -1,37 +0,0 @@
   > -*~
   >
   > Why delete this file?
   > --
   >
   > In Makefile.common
   > :
   >
   > >  Mac-x86_64_STRIP := strip -x
   > -Mac-x86_64_CFLAGS:= -Ilib/inc_mac -I$(JAVA_HOME)/include -O2 -fPIC 
-mmacosx-version-min=10.5 -fvisibility=hidden -I/usr/local/include 
-I/usr/local/opt/openssl/include
   > -Mac-x86_64_CXXFLAGS  := -Ilib/inc_mac -I$(JAVA_HOME)/include -O2 -fPIC 
-mmacosx-version-min=10.5 -fvisibility=hidden -I/usr/local/include 
-I/usr/local/opt/openssl/include
   > -Mac-x86_64_LINKFLAGS := -dynamiclib -L/usr/local/lib
   > +Mac-x86_64_CFLAGS:= -Ilib/inc_mac -I$(JAVA_HOME)/include -O2 -fPIC 
-mmacosx-version-min=10.7 -fvisibility=hidden -I/usr/local/include 
-I/usr/local/opt/openssl/include
   > +Mac-x86_64_CXXFLAGS  := -Ilib/inc_mac -I$(JAVA_HOME)/include -O2 -fPIC 
-mmacosx-version-min=10.7 -fvisibility=hidden -I/usr/local/include 
-I/usr/local/opt/openssl/include
   > +Mac-x86_64_LINKFLAGS := -dynamiclib -L/usr/local/lib
   >
   > nit: trailing whitespace
   > --
   >
   > In pom.xml
   > :
   >
   > > -  4.0.0
   > -
   > -  
   > -org.apache.commons
   > -commons-parent
   > -43
   > -  
   > -
   > -  org.apache.commons
   > -  commons-crypto
   > -  1.1.0-SNAPSHOT
   > -  jar
   > -
   > -  Apache Commons Crypto
   > -  
   > +
   > In src/main/java/org/apache/commons/crypto/OpenSslInfoNative.java
   > :
   >
   > > @@ -18,13 +18,16 @@
   >  package org.apache.commons.crypto;
   >
   >  /**
   > - * JNI interface of {@see CryptoRandom} implementation for OpenSSL.
   > + * JNI interface of @see CryptoRandom implementation for OpenSSL.
   >
   > Since you're touching this, it should be {@link CryptoRandom}.
   > --
   >
   > In src/main/java/org/apache/commons/crypto/OpenSslInfoNative.java
   > :
   >
   > >   * The native method in this class is defined in
   >   * OpenSslCryptoRandomNative.h (generated at build time by javah)
   >   * and implemented in the file
   >   * 
src/main/native/org/apache/commons/crypto/random/OpenSslCryptoRandomNative.c
   >   */
   > -class OpenSslInfoNative {
   > +public class OpenSslInfoNative {
   >
   > Why public? Users of the library should not need to care about this.
   >
   > It seems you're just calling this from Crypto.java so package private
   > should be fine.
   > --
   >
   > In src/main/java/org/apache/commons/crypto/jna/OpenSsl102NativeJna.java
   > :
   >
   > > + * distributed under the License is distributed on an "AS IS" BASIS,
   > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
implied.
   > + * See the License for the specific language governing permissions and
   > + * limitations under the License.
   > + */
   > +
   > +package org.apache.commons.crypto.jna;
   > +
   > +import java.nio.ByteBuffer;
   > +
   > +import com.sun.jna.Native;
   > +import com.sun.jna.NativeLong;
   > +import