Skip to content

Try direct path for MessageDigest before invasive path #194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 1, 2020

Conversation

headius
Copy link
Member

@headius headius commented Feb 28, 2020

This logic is responsible for the remaining known "illegal access"
warning on Java 9+. I am not sure why we do not trust the default
provide to give us most algorithms, since it should be identical
to the one provied by Bouncy Castle. This change reverses the
logic and only uses the invasive provider-specific form when the
direct lookup fails.

Relates to jruby/jruby#6098.

This logic is responsible for the remaining known "illegal access"
warning on Java 9+. I am not sure why we do not trust the default
provide to give us most algorithms, since it should be identical
to the one provied by Bouncy Castle. This change reverses the
logic and only uses the invasive provider-specific form when the
direct lookup fails.

Relates to jruby/jruby#6098.
@kares kares merged commit b310858 into master Mar 1, 2020
@kares
Copy link
Member

kares commented Mar 1, 2020

Thanks, not sure these reports will stop coming unless smt is done about SecurityHelper as a whole.

@headius
Copy link
Member Author

headius commented Mar 2, 2020

@kares Ideally this is a step toward not using it at all. Both of the cases I've fixed have turned out to be unnecessary. I left the fallback logic in place for unusual hashes, but it seems like nearly all of the hashes supported by CRuby are supported by OpenJDK:

2.6.5 :002 > OpenSSL::Digest.constants
 => [:SHA384, :SHA512, :MD5, :DigestError, :Digest, :SHA1, :MD2, :MD4, :MDC2, :RIPEMD160, :SHA224, :SHA256] 

Of the digests listed here, only two appear to have issues: MD4 and MDC2:

irb(main):003:0> OpenSSL::Digest.constants.grep(/^[A-Z0-9]+$/).sort.each {|name| begin; (OpenSSL::Digest.const_get(name).new); rescue Exception; puts $!.message; else; puts "#{name} ok"; end}
DSS ok
DSS1 ok
MD2 ok
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.jruby.ext.openssl.SecurityHelper (file:/Users/headius/projects/jruby/lib/ruby/stdlib/jopenssl.jar) to field java.security.MessageDigest.provider
WARNING: Please consider reporting this to the maintainers of org.jruby.ext.openssl.SecurityHelper
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
MD4 ok
MD5 ok
Unsupported digest algorithm (MDC2)
RIPEMD160 ok
SHA ok
SHA1 ok
SHA224 ok
SHA256 ok
SHA384 ok
SHA512 ok

MD4 is preceded by the illegal access warning, so I assume that means it must come from Bouncy Castle. MDC2 does not appear to be supported by either OpenJDK or Bouncy Castle and likely has never worked.

It's also not clear to me why we need to get digest implementations from a specific provider; these are simple algorithms and could probably be instantiated directly.

There's another change we could, and perhaps should make to these SecurityHelper accesses: make them use my backport9 library to test if these classes are open before attempting to access their internals. In this case, it would mean the MD4 algorithm would not be supported on a Java 9+ OpenJDK (until we add it).

@headius headius deleted the direct_message_digest branch March 2, 2020 11:19
@kares kares added this to the 0.10.5 milestone Mar 17, 2020
@deivid-rodriguez
Copy link
Contributor

Hi!

I'm trying to get rid of this warning, which I believe has been fixed on jruby-openssl master but not yet released.

I tried to compile and install the master version of the gem locally, but getting this errors (I seem to be building an empty jar):

(master) deivid@chaba ~/Code/jruby-openssl $ mvn package -Dmaven.test.skip=true
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.google.inject.internal.cglib.core.$ReflectUtils$1 (file:/usr/share/maven/lib/guice.jar) to method java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int,java.security.ProtectionDomain)
WARNING: Please consider reporting this to the maintainers of com.google.inject.internal.cglib.core.$ReflectUtils$1
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
[INFO] Scanning for projects...
[WARNING] 
[WARNING] Some problems were encountered while building the effective model for rubygems:jruby-openssl:gem:0.10.5.dev-SNAPSHOT
[WARNING] 'build.plugins.plugin.(groupId:artifactId)' must be unique but found duplicate declaration of plugin org.apache.maven.plugins:maven-clean-plugin @ line 300, column 15
[WARNING] 'build.plugins.plugin.(groupId:artifactId)' must be unique but found duplicate declaration of plugin org.apache.maven.plugins:maven-dependency-plugin @ line 346, column 15
[WARNING] 
[WARNING] It is highly recommended to fix these problems because they threaten the stability of your build.
[WARNING] 
[WARNING] For this reason, future Maven versions might no longer support building such malformed projects.
[WARNING] 
[INFO] 
[INFO] -----------------------< rubygems:jruby-openssl >-----------------------
[INFO] Building JRuby OpenSSL 0.10.5.dev-SNAPSHOT
[INFO] --------------------------------[ gem ]---------------------------------
[INFO] 
[INFO] --- gem-maven-plugin:1.1.8:initialize (default-initialize) @ jruby-openssl ---
[INFO] 
[INFO] --- maven-resources-plugin:3.1.0:resources (default-resources) @ jruby-openssl ---
[WARNING] Using platform encoding (UTF-8 actually) to copy filtered resources, i.e. build is platform dependent!
[INFO] skip non existing resourceDirectory /home/deivid/Code/jruby-openssl/src/main/resources
[INFO] 
[INFO] --- maven-compiler-plugin:3.8.1:compile (default-compile) @ jruby-openssl ---
[INFO] Nothing to compile - all classes are up to date
[INFO] 
[INFO] --- maven-jar-plugin:2.4:jar (default-jar) @ jruby-openssl ---
[WARNING] JAR will be empty - no content was marked for inclusion!
[INFO] 
[INFO] --- exec-maven-plugin:1.3.2:exec (invoker-generator) @ jruby-openssl ---
/home/deivid/Code/jruby-openssl/target/generated-sources/annotated_classes.txt - not found. skip generator.
[INFO] 
[INFO] --- build-helper-maven-plugin:1.9:add-source (default) @ jruby-openssl ---
[INFO] Source directory: /home/deivid/Code/jruby-openssl/target/generated-sources added.
[INFO] 
[INFO] --- maven-compiler-plugin:3.8.1:compile (compile-populators) @ jruby-openssl ---
[INFO] Nothing to compile - all classes are up to date
[INFO] 
[INFO] --- maven-dependency-plugin:2.8:copy-dependencies (default) @ jruby-openssl ---
[INFO] bctls-jdk15on-1.62.jar already exists in destination.
[INFO] bcpkix-jdk15on-1.62.jar already exists in destination.
[INFO] bcprov-jdk15on-1.62.jar already exists in destination.
[INFO] 
[INFO] --- maven-resources-plugin:3.1.0:testResources (default-testResources) @ jruby-openssl ---
[INFO] Not copying test resources
[INFO] 
[INFO] --- maven-compiler-plugin:3.8.1:testCompile (default-testCompile) @ jruby-openssl ---
[INFO] Not compiling test sources
[INFO] 
[INFO] --- maven-surefire-plugin:3.0.0-M4:test (default-test) @ jruby-openssl ---
[INFO] Tests are skipped.
[INFO] 
[INFO] --- runit-maven-plugin:1.1.8:test (default) @ jruby-openssl ---
[INFO] Skipping RUnit tests
[INFO] 
[INFO] --- maven-jar-plugin:2.4:jar (default) @ jruby-openssl ---
[WARNING] JAR will be empty - no content was marked for inclusion!
[INFO] 
[INFO] --- gem-maven-plugin:1.1.8:package (default-package) @ jruby-openssl ---
[INFO] include dependencies? true
[INFO] use repository layout? true
[INFO]  -- include -- org.bouncycastle:bcprov-jdk15on:jar:1.62:compile
[INFO]  -- include -- org.bouncycastle:bcpkix-jdk15on:jar:1.62:compile
[INFO]  -- include -- org.bouncycastle:bctls-jdk15on:jar:1.62:compile
[WARNING] WARNING: Illegal reflective access by com.headius.backport9.modules.Modules to method sun.nio.ch.NativeThread.signal(long)
WARNING: Illegal reflective access by com.headius.backport9.modules.Modules to method sun.nio.ch.NativeThread.current()
WARNING: Illegal reflective access by com.headius.backport9.modules.Modules to method java.lang.Object.finalize()
WARNING: Illegal reflective access by com.headius.backport9.modules.Modules to method java.lang.Object.clone()
WARNING: Illegal reflective access by com.headius.backport9.modules.Modules to field java.io.FileDescriptor.fd
ERROR:  Loading command: build (NameError)
	cannot load Java class org.jruby.ext.openssl.OpenSSL
ERROR:  While executing gem ... (NoMethodError)
    undefined method `invoke_with_build_args' for nil:NilClass

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  6.311 s
[INFO] Finished at: 2020-03-25T12:31:05+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal de.saumya.mojo:gem-maven-plugin:1.1.8:package (default-package) on project jruby-openssl: Execution default-package of goal de.saumya.mojo:gem-maven-plugin:1.1.8:package failed: Java returned: 1 -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginExecutionException

Any hints?

@headius
Copy link
Member Author

headius commented Mar 25, 2020

@deivid-rodriguez The first warning is not related to our code. I would guess it's a Maven plugin that has not been updated for Java 9+ reflection restrictions. Perhaps there's a newer plugin? Running with Java option --illegal-access=debug would show us a longer trace so we can figure out what plugin to update.

The stack of five warnings later in the build are indeed from JRuby but it's a JRuby version used by the gem-maven-plugin. Upgrading that plugin to a newer version of JRuby should eliminate the first four. The fifth might go away as well, but it's used by the native IO subsystem and compatibility may be altered.

@headius
Copy link
Member Author

headius commented Mar 25, 2020

@deivid-rodriguez Oh, FWIW this would be good to file as a separate issue, perhaps after digging up more details like plugin versions etc.

@deivid-rodriguez
Copy link
Contributor

@headius Thanks. What about the error? You mentioned five warnings, but not the actual error. Anyways, I opened a separate ticket.

@headius
Copy link
Member Author

headius commented Mar 26, 2020

Oh I misunderstood...I thought you were asking how to get rid of those warnings :-)

I'll have a look in the other issue.

@deivid-rodriguez
Copy link
Contributor

Yeah, my bad, not very well explained. When I said "this warning" I meant "the warning that presumabily got fixed by this PR" :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants