-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
JDK 8u102 changed JceSecurity#isRestricted to final #4101
Comments
... and the naive patch would be : diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java
index 9f56537..1c06db5 100644
--- a/core/src/main/java/org/jruby/Ruby.java
+++ b/core/src/main/java/org/jruby/Ruby.java
@@ -170,6 +170,7 @@ import java.lang.invoke.MethodHandle;
import java.lang.ref.WeakReference;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
import java.net.BindException;
import java.net.MalformedURLException;
import java.net.URL;
@@ -1288,10 +1289,23 @@ public final class Ruby implements Constantizable {
try {
Class jceSecurity = Class.forName("javax.crypto.JceSecurity");
Field isRestricted = jceSecurity.getDeclaredField("isRestricted");
- isRestricted.setAccessible(true);
- isRestricted.set(null, false);
- isRestricted.setAccessible(false);
- } catch (Exception e) {
+ if ( Boolean.TRUE.equals(isRestricted.get(null)) ) {
+ if ( Modifier.isFinal(isRestricted.getModifiers()) ) {
+ Field modifiers = Field.class.getDeclaredField("modifiers");
+ modifiers.setAccessible(true);
+ modifiers.setInt(isRestricted, isRestricted.getModifiers() & ~Modifier.FINAL);
+ }
+ isRestricted.setAccessible(true);
+ isRestricted.setBoolean(null, false); // isRestricted = false;
+ isRestricted.setAccessible(false);
+ }
+ }
+ catch (ClassNotFoundException e) { // not an OpenJDK ~ JVM
+ if (isDebug() || LOG.isDebugEnabled()) {
+ LOG.debug("unable to enable unlimited-strength crypto " + e);
+ }
+ }
+ catch (Exception e) {
if (isDebug() || LOG.isDebugEnabled()) {
LOG.debug("unable to enable unlimited-strength crypto", e);
}
-- |
I'm not sure if it's ok for JRuby to ship this. I mean, I'm not a lawyer. But JRuby users can easily adjust their Rail boot scripts to include the @kares thanks for figuring this out! |
@kares ah, i just realized the setAccessible stuff has been in there. TIL. But it seems that the custom code to set isRestricted is still needed, so I'm confused. Does this not get executed all the time? |
yes should get executed on runtime initialization, thus the "Rails initializer hack" (or whatever you're using) shouldn't be necessary. on 8u102 you should see a trace printed with JRuby in debug mode: |
It would be interesting to see what they do in the unlimited-strength crypto stuff now. They have to have a way to turn this off too. The code that existed before basically just emulated the same steps that they do. |
Here's the bug in OpenJDK: https://bugs.openjdk.java.net/browse/JDK-8149417 No other code changed. # HG changeset patch
# User igerasim
# Date 1460476747 -10800
# Tue Apr 12 18:59:07 2016 +0300
# Node ID 666ddde3fecf014cb3c2949542f317231b72db46
# Parent 7d49df0ab4f69bdb0c0751e37e37549e054b794d
8149417: Use final restricted flag
Reviewed-by: mullan, weijun, coffeys
diff -r 7d49df0ab4f6 -r 666ddde3fecf src/share/classes/javax/crypto/JceSecurity.java
--- a/src/share/classes/javax/crypto/JceSecurity.java Mon Apr 11 14:42:45 2016 +0300
+++ b/src/share/classes/javax/crypto/JceSecurity.java Tue Apr 12 18:59:07 2016 +0300
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -64,8 +64,7 @@
private final static Map<Provider, Object> verifyingProviders =
new IdentityHashMap<>();
- // Set the default value. May be changed in the static initializer.
- private static boolean isRestricted = true;
+ private static final boolean isRestricted;
/*
* Don't let anyone instantiate this. |
So @kares's trick seems fine to me. It would be better moved out to a separate utility at this point, think. The alternative, for those who question how kosher it is for us to do this, would be to improve our handling of the situation when these policy files do not exist. The logic in JDK basically just loads up two jar files from the crypto extension...jar files which contain little more than two small security policy files. It would not be difficult for us to present a warning when crypto gets booted with helpful links and/or utilities to install the policy files. Here's the code that actually does the policy file logic in JDK: https://gist.github.com/headius/84a4b41706523579f9829db0a66904f8 |
A tip from a SO user indicates that our trick doesn't entirely disable the restrictions. I'm leaning toward being noisy until the user downloads policy files or runs our utility.
This user's much larger hack, with the additional tweaks, is provided here: http://stackoverflow.com/questions/1179672/how-to-avoid-installing-unlimited-strength-jce-policy-files-when-deploying-an |
@kares We've never had anyone complain about the restricted key stuff outside of jruby-openssl use, have we? I'm thinking we could do the following if we wanted to remove the reflection hacks:
The key point is that we'd be proactively warning the user that they might run into crypto problems, rather than letting them fall into a cryptic (pun intended) JCE error message. |
@headius nope - no explicit complains for restricted. but most users might not know they can install a JCE pack for unlimited crypto (I wonder if packaged OpenJDK builds e.g. in Debian do setup unlimited policy - probably not and they might not provide explicit support to do so neither). recall there's been issues around max-allowed-key-length but I believe its worked around (BC ciphers handle longer keys). I do like your idea however - having jruby-openssl gem packed with a script to help installing JCE pack. |
+1 for adding a script to jruby-openssl to help the installing of missing bits. |
@kares For now, commit your patch. It's no worse than what we had. If you could, move it into a utility class somewhere, like Helpers. At least this will be working as well as it used to for 9.1.3.0 + 8u102. We'll deal with the better solution later. |
This also attempts to make this hackery happen only once, in case this logic is called from many places (e.g. many JRuby instances in the same JVM). See #4101.
@kares FYI, the pre-check of "isAccessible" was causing this code to never run, since it was never accessible to be read. I removed that check and instead used a static boolean to "guarantee" we only attempt this hack once. I am also going to remove the check for Oracle JDK vs OpenJDK. |
my bad - did not realize that - thought the hack will only be needed on OracleJDK (not on OpenJDK). @headius with your change under different JDKs there will be an info being logged ...
... maybe going back to debug shall do as its not that relevant? - whatever you feel is best. |
@kares Thanks for patching that warning 👍 |
will there be a 1.7 release with this fix? |
…inal. This code attempts to solve this issue. See also https://github.com/jruby/jruby/blob/0c345e1b186bd457ebd96143c0816abe93b18fdf/core/src/main/java/org/jruby/util/SecurityHelper.java and the relevant discussion at jruby/jruby#4101 In addition, code reformatting is executed during process-sources maven phase automatically.
Hi, When I change to Java 8u112, isRestricted.get(null) will also directly raise the IllegalAccessException, So following will works without the condition if ( Modifier.isFinal(isRestricted.getModifiers()) ) {
Field modifiers = Field.class.getDeclaredField("modifiers");
modifiers.setAccessible(true);
modifiers.setInt(isRestricted, isRestricted.getModifiers() & ~Modifier.FINAL);
}
isRestricted.setAccessible(true);
isRestricted.setBoolean(null, false); // isRestricted = false;
isRestricted.setAccessible(false); |
@xuanzhaopeng exactly the code we have in place already, I am not sure what you're suggesting here |
Environment
originally reported by @jkutner on the mailing list, the OpenJDK
javax.crypto.JceSecurity
internals changed: http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/rev/666ddde3fecfExpected Behavior
unlimited strength Java cryptography support out of the box (on OpenJDK derrivates)
Actual Behavior
limited cryptography strength - need to install Java Cryptography Extension (JCE) Unlimited Strength Jurisdiction Policy Files
The text was updated successfully, but these errors were encountered: