Skip to content

Commit 2d91787

Browse files
MariuszCwiklaMariuszCwikla
committed
Rework: Fix OpenSSL::SSL::SSLContext#min_version= failure
1 parent 4ddcdb0 commit 2d91787

File tree

5 files changed

+58
-98
lines changed

5 files changed

+58
-98
lines changed

‎src/main/java/org/jruby/ext/openssl/SSLContext.java

Lines changed: 17 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -97,44 +97,35 @@ public class SSLContext extends RubyObject {
9797

9898
// Mapping table for OpenSSL's SSL_METHOD -> JSSE's SSLContext algorithm.
9999
private static final HashMap<String, String> SSL_VERSION_OSSL2JSSE;
100-
// Inverse mapping (incomplete, does not map 1-1 with SSL_VERSION_OSSL2JSSE)
101-
private static final HashMap<String, String> SSL_VERSION_OSSL2JSSE_INV;
102100
// Mapping table for JSEE's enabled protocols for the algorithm.
103101
private static final Map<String, String[]> ENABLED_PROTOCOLS;
104102
// Mapping table from CRuby parse_proto_version(VALUE str)
105103
private static final Map<String, Integer> PROTO_VERSION_MAP;
106-
// same as METHODS_MAP from ssl.rb
107-
private static final Map<String, Integer> METHODS_MAP;
108-
private static final Map<Integer, String> METHODS_MAP_INV;
104+
105+
private static final Map<String, Integer> JSSE_TO_VERSION;
109106

110107
static {
111108
SSL_VERSION_OSSL2JSSE = new LinkedHashMap<String, String>(20, 1);
112109
ENABLED_PROTOCOLS = new HashMap<String, String[]>(8, 1);
113110

114-
SSL_VERSION_OSSL2JSSE_INV = new HashMap<String, String>();
115-
116111
SSL_VERSION_OSSL2JSSE.put("TLSv1", "TLSv1");
117112
SSL_VERSION_OSSL2JSSE.put("TLSv1_server", "TLSv1");
118113
SSL_VERSION_OSSL2JSSE.put("TLSv1_client", "TLSv1");
119114
ENABLED_PROTOCOLS.put("TLSv1", new String[] { "TLSv1" });
120-
SSL_VERSION_OSSL2JSSE_INV.put("TLSv1", "TLSv1_1");
121115

122116
SSL_VERSION_OSSL2JSSE.put("SSLv2", "SSLv2");
123117
SSL_VERSION_OSSL2JSSE.put("SSLv2_server", "SSLv2");
124118
SSL_VERSION_OSSL2JSSE.put("SSLv2_client", "SSLv2");
125119
ENABLED_PROTOCOLS.put("SSLv2", new String[] { "SSLv2" });
126-
SSL_VERSION_OSSL2JSSE_INV.put("SSLv2", "SSLv2");
127120

128121
SSL_VERSION_OSSL2JSSE.put("SSLv3", "SSLv3");
129122
SSL_VERSION_OSSL2JSSE.put("SSLv3_server", "SSLv3");
130123
SSL_VERSION_OSSL2JSSE.put("SSLv3_client", "SSLv3");
131124
ENABLED_PROTOCOLS.put("SSLv3", new String[] { "SSLv3" });
132-
SSL_VERSION_OSSL2JSSE_INV.put("SSLv3", "SSLv3");
133125

134126
SSL_VERSION_OSSL2JSSE.put("SSLv23", "SSL");
135127
SSL_VERSION_OSSL2JSSE.put("SSLv23_server", "SSL");
136128
SSL_VERSION_OSSL2JSSE.put("SSLv23_client", "SSL");
137-
SSL_VERSION_OSSL2JSSE_INV.put("SSL", "SSLv23");
138129

139130
ENABLED_PROTOCOLS.put("SSL", new String[] { "SSLv2", "SSLv3", "TLSv1", "TLSv1.1", "TLSv1.2" });
140131

@@ -152,8 +143,6 @@ public class SSLContext extends RubyObject {
152143
SSL_VERSION_OSSL2JSSE.put("TLSv1_1", "TLSv1.1"); // supported on MRI 2.x
153144
SSL_VERSION_OSSL2JSSE.put("TLSv1_2", "TLSv1.2"); // supported on MRI 2.x
154145
ENABLED_PROTOCOLS.put("TLSv1.2", new String[] { "TLSv1.2" });
155-
SSL_VERSION_OSSL2JSSE_INV.put("TLSv1.1", "TLSv1_1");
156-
SSL_VERSION_OSSL2JSSE_INV.put("TLSv1.2", "TLSv1_2");
157146

158147
SSL_VERSION_OSSL2JSSE.put("TLSv1.2", "TLSv1.2"); // just for completeness
159148
SSL_VERSION_OSSL2JSSE.put("TLSv1_2_server", "TLSv1.2");
@@ -167,19 +156,13 @@ public class SSLContext extends RubyObject {
167156
PROTO_VERSION_MAP.put("TLS1_2", SSL.TLS1_2_VERSION);
168157
PROTO_VERSION_MAP.put("TLS1_3", SSL.TLS1_3_VERSION);
169158

170-
METHODS_MAP = new HashMap<String, Integer>();
171-
METHODS_MAP.put("SSLv23", 0);
172-
METHODS_MAP.put("SSLv2", SSL.SSL2_VERSION);
173-
METHODS_MAP.put("SSLv3", SSL.SSL3_VERSION);
174-
METHODS_MAP.put("TLSv1", SSL.TLS1_VERSION);
175-
METHODS_MAP.put("TLSv1_1", SSL.TLS1_1_VERSION);
176-
METHODS_MAP.put("TLSv1_2", SSL.TLS1_2_VERSION);
177-
METHODS_MAP.put("TLSv1_3", SSL.TLS1_3_VERSION);
178-
179-
METHODS_MAP_INV = new HashMap<Integer, String>();
180-
for(Map.Entry<String, Integer> e: METHODS_MAP.entrySet()) {
181-
METHODS_MAP_INV.put(e.getValue(), e.getKey());
182-
}
159+
JSSE_TO_VERSION = new HashMap<String, Integer>();
160+
JSSE_TO_VERSION.put("SSLv2", SSL.SSL2_VERSION);
161+
JSSE_TO_VERSION.put("SSLv3", SSL.SSL3_VERSION);
162+
JSSE_TO_VERSION.put("TLSv1", SSL.TLS1_VERSION);
163+
JSSE_TO_VERSION.put("TLSv1.1", SSL.TLS1_1_VERSION);
164+
JSSE_TO_VERSION.put("TLSv1.2", SSL.TLS1_2_VERSION);
165+
JSSE_TO_VERSION.put("TLSv1.3", SSL.TLS1_3_VERSION);
183166
}
184167

185168
private static ObjectAllocator SSLCONTEXT_ALLOCATOR = new ObjectAllocator() {
@@ -305,10 +288,7 @@ public SSLContext(Ruby runtime, RubyClass type) {
305288
}
306289

307290
private String ciphers = CipherStrings.SSL_DEFAULT_CIPHER_LIST;
308-
private static final String DEFAULT_PROTOCOL = "SSL"; // SSLv23 in OpenSSL by default
309-
private static final int DEFAULT_PROTOCOL_VERSION = 0;
310-
private String protocol = null;
311-
private Integer protocolVersion = null;
291+
private String protocol = "SSL"; // SSLv23 in OpenSSL by default
312292
private boolean protocolForServer = true;
313293
private boolean protocolForClient = true;
314294
private int minProtocolVersion = 0;
@@ -489,8 +469,6 @@ public IRubyObject setup(final ThreadContext context) {
489469
}
490470
*/
491471

492-
setupProtocolVersion(context);
493-
494472
try {
495473
internalContext = createInternalContext(context, cert, key, store, clientCert, extraChainCert, verifyMode, timeout);
496474
}
@@ -501,37 +479,6 @@ public IRubyObject setup(final ThreadContext context) {
501479
return runtime.getTrue();
502480
}
503481

504-
private void setupProtocolVersion(final ThreadContext context) {
505-
if (protocolVersion == null) {
506-
String ssl_version = null;
507-
if (maxProtocolVersion != 0) {
508-
ssl_version = METHODS_MAP_INV.get(maxProtocolVersion);
509-
set_ssl_version(ssl_version);
510-
} else {
511-
ssl_version = getMaxSupportedProtocolVersion();
512-
set_ssl_version(ssl_version);
513-
}
514-
}
515-
516-
if (minProtocolVersion !=0 || maxProtocolVersion !=0 ) {
517-
if (minProtocolVersion != 0 && protocolVersion < minProtocolVersion) {
518-
throw newSSLError(context.runtime, "no protocols available");
519-
}
520-
if (maxProtocolVersion != 0 && protocolVersion > maxProtocolVersion) {
521-
throw newSSLError(context.runtime, "no protocols available");
522-
}
523-
}
524-
}
525-
526-
private String getMaxSupportedProtocolVersion() {
527-
//TODO: probably needs to be computed from
528-
//javax.net.ssl.SSLContext.getSupportedSSLParameters().getProtocols()
529-
//some changes prob. needed in dummySSLEngine and SecurityHelper.getSSLContext
530-
//Hardcoding now.
531-
//Nonetheless, TLS1.3 needs more changes in jruby-openssl
532-
return "TLSv1_2";
533-
}
534-
535482
@JRubyMethod
536483
public RubyArray ciphers(final ThreadContext context) {
537484
return matchedCiphers(context);
@@ -540,7 +487,6 @@ public RubyArray ciphers(final ThreadContext context) {
540487
private RubyArray matchedCiphers(final ThreadContext context) {
541488
final Ruby runtime = context.runtime;
542489
try {
543-
setupProtocolVersion(context);
544490
final String[] supported = getSupportedCipherSuites(protocol);
545491
final Collection<CipherStrings.Def> cipherDefs =
546492
CipherStrings.matchingCiphers(this.ciphers, supported, false);
@@ -594,19 +540,14 @@ public IRubyObject set_ssl_version(IRubyObject version) {
594540
} else {
595541
versionStr = version.convertToString().toString();
596542
}
597-
set_ssl_version(versionStr);
598-
return version;
599-
}
600-
601-
private void set_ssl_version(final String versionStr) {
602543
final String protocol = SSL_VERSION_OSSL2JSSE.get(versionStr);
603544
if ( protocol == null ) {
604545
throw getRuntime().newArgumentError("unknown SSL method `"+ versionStr +"'");
605546
}
606547
this.protocol = protocol;
607-
this.protocolVersion = METHODS_MAP.get(versionStr);
608548
protocolForServer = ! versionStr.endsWith("_client");
609549
protocolForClient = ! versionStr.endsWith("_server");
550+
return version;
610551
}
611552

612553
@JRubyMethod(name = "set_minmax_proto_version")
@@ -621,7 +562,7 @@ private int parseProtoVersion(IRubyObject version) {
621562
if (version.isNil())
622563
return 0;
623564
if (version instanceof RubyFixnum) {
624-
return (int) ((RubyFixnum) version).getLongValue();
565+
return RubyFixnum.fix2int(version);
625566
}
626567

627568
String string = version.asString().asJavaString();
@@ -634,7 +575,7 @@ private int parseProtoVersion(IRubyObject version) {
634575
return sslVersion;
635576
}
636577

637-
final String getProtocol() { return this.protocol; }
578+
final String getProtocol() { return this.protocol; }
638579

639580
@JRubyMethod(name = "session_cache_mode")
640581
public IRubyObject session_cache_mode() {
@@ -758,6 +699,10 @@ private String[] getEnabledProtocols(final SSLEngine engine) {
758699
final String[] engineProtocols = engine.getEnabledProtocols();
759700
final List<String> protocols = new ArrayList<String>(enabledProtocols.length);
760701
for ( final String enabled : enabledProtocols ) {
702+
int protocolVersion = JSSE_TO_VERSION.get(enabled);
703+
if (minProtocolVersion != 0 && protocolVersion < minProtocolVersion) continue;
704+
if (maxProtocolVersion != 0 && protocolVersion > maxProtocolVersion) continue;
705+
761706
if (((options & OP_NO_SSLv2) != 0) && enabled.equals("SSLv2")) continue;
762707
if (((options & OP_NO_SSLv3) != 0) && enabled.equals("SSLv3")) continue;
763708
if (((options & OP_NO_TLSv1) != 0) && enabled.equals("TLSv1")) continue;

‎src/test/ruby/ssl/test_context.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,12 @@ def test_context_set_ssl_version
104104
end
105105
assert_raises(TypeError) { context.ssl_version = 12 }
106106
end
107-
107+
108108
def test_context_minmax_version
109109
context = OpenSSL::SSL::SSLContext.new
110110
context.min_version = OpenSSL::SSL::TLS1_VERSION
111111
context.max_version = OpenSSL::SSL::TLS1_2_VERSION
112-
end
112+
end if RUBY_VERSION > '2.3'
113113

114114
def test_context_ciphers
115115
self.class.disable_security_restrictions

‎src/test/ruby/ssl/test_helper.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
require File.expand_path('../test_helper', File.dirname(__FILE__))
2+
require 'openssl'
23

34
module SSLTestHelper
45

@@ -7,7 +8,7 @@ module SSLTestHelper
78
PORT = 20443
89
ITERATIONS = ($0 == __FILE__) ? 100 : 10
910

10-
def setup; require 'openssl'
11+
def setup;
1112

1213
@ca_key = OpenSSL::PKey::RSA.new TEST_KEY_RSA2048
1314
@svr_key = OpenSSL::PKey::RSA.new TEST_KEY_RSA1024

‎src/test/ruby/ssl/test_socket.rb

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -136,21 +136,6 @@ def test_connect_nonblock
136136
end
137137
end if RUBY_VERSION > '2.2'
138138

139-
def test_minmax_ssl_version
140-
# WIP: this test should setup it's own server to prove that SSLContext.min_version and SSLContext.max_version work as expected
141-
# However below line is failing with "no cipher match" even on master
142-
# When "no cipher match" is fixed, then this test need cleanup
143-
144-
#ssl_server = server
145-
begin
146-
ssl_client = client(443, host: "google.com", min_version: OpenSSL::SSL::TLS1_VERSION, max_version: OpenSSL::SSL::TLS1_1_VERSION, ciphers: nil)
147-
#ssl_client = client(server_port(ssl_server), min_version: OpenSSL::SSL::TLS1_VERSION, max_version: OpenSSL::SSL::TLS1_1_VERSION)
148-
assert_equal 'TLSv1.1', ssl_client.ssl_version
149-
ensure
150-
ssl_client.sysclose unless ssl_client.nil?
151-
end
152-
end if RUBY_VERSION > '2.3'
153-
154139
def test_inherited_socket; require 'socket'
155140
inheritedSSLSocket = Class.new(OpenSSL::SSL::SSLSocket)
156141

@@ -165,21 +150,18 @@ def test_inherited_socket; require 'socket'
165150

166151
private
167152

168-
def server(ssl_version: nil); require 'socket'
153+
def server; require 'socket'
169154
host = "127.0.0.1"; port = 0
170155
ctx = OpenSSL::SSL::SSLContext.new()
171-
ctx.ssl_version = ssl_version unless ssl_version.nil?
172-
ctx.ciphers = "ADH"
156+
ctx.ciphers = "ADH"
173157
server = TCPServer.new(host, port)
174158
OpenSSL::SSL::SSLServer.new(server, ctx)
175159
end
176160

177-
def client(port, host: "127.0.0.1", min_version: nil, max_version: nil, ciphers: "ADH")
178-
require 'socket'
161+
def client(port); require 'socket'
162+
host = "127.0.0.1"
179163
ctx = OpenSSL::SSL::SSLContext.new()
180-
ctx.min_version = min_version unless min_version.nil?
181-
ctx.max_version = max_version unless max_version.nil?
182-
ctx.ciphers = ciphers unless ciphers.nil?
164+
ctx.ciphers = "ADH"
183165
client = TCPSocket.new(host, port)
184166
ssl = OpenSSL::SSL::SSLSocket.new(client, ctx)
185167
ssl.connect

‎src/test/ruby/ssl/test_ssl.rb

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,38 @@ def test_ssl_version_tlsv1_2
137137
end
138138
end
139139

140+
# Ruby supports TLSv1.3 already. Java - TLSv1.2.
141+
MAX_SSL_VERSION = if defined? JRUBY_VERSION
142+
"TLSv1.2"
143+
else
144+
"TLSv1.3"
145+
end
146+
[
147+
[OpenSSL::SSL::TLS1_VERSION, nil, MAX_SSL_VERSION, "(TLSv1,)"],
148+
[OpenSSL::SSL::TLS1_1_VERSION, nil, MAX_SSL_VERSION, "(TLSv1.1,)"],
149+
[OpenSSL::SSL::TLS1_2_VERSION, nil, MAX_SSL_VERSION, "(TLSv1.2,)"],
150+
[nil, OpenSSL::SSL::TLS1_VERSION, "TLSv1", "(,TLSv1)"],
151+
[nil, OpenSSL::SSL::TLS1_1_VERSION, "TLSv1.1", "(,TLSv1.1)"],
152+
[nil, OpenSSL::SSL::TLS1_2_VERSION, "TLSv1.2", "(,TLSv1.2)"],
153+
[OpenSSL::SSL::TLS1_VERSION, OpenSSL::SSL::TLS1_VERSION, "TLSv1", "(TLSv1,TLSv1)"],
154+
[OpenSSL::SSL::TLS1_VERSION, OpenSSL::SSL::TLS1_1_VERSION, "TLSv1.1", "(TLSv1,TLSv1.1)"],
155+
[OpenSSL::SSL::TLS1_VERSION, OpenSSL::SSL::TLS1_2_VERSION, "TLSv1.2", "(TLSv1,TLSv1.2)"]
156+
].each do |min_version, max_version, expected_version, desc|
157+
define_method("test_ssl_minmax_#{desc}") do
158+
ctx_proc = Proc.new do |ctx|
159+
ctx.min_version = min_version unless min_version.nil?
160+
ctx.max_version = max_version unless max_version.nil?
161+
end
162+
start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc) do |server, port|
163+
sock = TCPSocket.new("127.0.0.1", port)
164+
ssl = OpenSSL::SSL::SSLSocket.new(sock)
165+
ssl.connect
166+
assert_equal(expected_version, ssl.ssl_version)
167+
ssl.close
168+
end
169+
end
170+
end if RUBY_VERSION > '2.3'
171+
140172
def test_read_nonblock_would_block
141173
start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true) do |server, port|
142174
sock = TCPSocket.new("127.0.0.1", port)

0 commit comments

Comments
 (0)