Skip to content

Commit 49fdea4

Browse files
kareskares
committed
do not der encode non-critical flag in X509::Extension (fixes #389)
1 parent 9074751 commit 49fdea4

File tree

4 files changed

+113
-32
lines changed

4 files changed

+113
-32
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ public IRubyObject to_text(final ThreadContext context) {
270270
Extension ext = (Extension) extensions.entry(i);
271271
ASN1ObjectIdentifier oiden = ext.getRealOid();
272272
text.append(IND12).append( ASN1.o2a(runtime, oiden) ).append(": ");
273-
if ( ext.getRealCritical() ) text.append("critical");
273+
if ( ext.isRealCritical() ) text.append("critical");
274274
text.append("\n");
275275
text.append(IND16).append( ext.value(context) ).append("\n");
276276
}
@@ -421,7 +421,7 @@ public IRubyObject sign(final ThreadContext context, final IRubyObject key, IRub
421421
try {
422422
for ( int i = 0; i < extensions.size(); i++ ) {
423423
Extension ext = (Extension) extensions.entry(i);
424-
generator.addExtension(ext.getRealOid(), ext.getRealCritical(), ext.getRealValueBytes());
424+
generator.addExtension(ext.getRealOid(), ext.isRealCritical(), ext.getRealValueBytes());
425425
}
426426
}
427427
catch (IOException e) { throw newCRLError(runtime, e); }

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ public IRubyObject sign(final ThreadContext context, final IRubyObject key, fina
427427
for ( X509Extensions.Extension ext : extensions ) {
428428
try {
429429
final byte[] bytes = ext.getRealValueBytes();
430-
generator.addExtension(ext.getRealOid(), ext.getRealCritical(), bytes);
430+
generator.addExtension(ext.getRealOid(), ext.isRealCritical(), bytes);
431431
}
432432
catch (IOException ioe) {
433433
throw runtime.newIOErrorFromException(ioe);

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

Lines changed: 40 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@
7474
import static org.jruby.ext.openssl.OpenSSLReal.debug;
7575
import static org.jruby.ext.openssl.OpenSSLReal.debugStackTrace;
7676
import static org.jruby.ext.openssl.OpenSSLReal.isDebug;
77-
import static org.jruby.ext.openssl.OpenSSLReal.warn;
7877

7978
/**
8079
* @author <a href="mailto:[email protected]">Ola Bini</a>
@@ -254,7 +253,7 @@ else if ( id.equals("2.5.29.37") ) { //extendedKeyUsage
254253

255254
ext.setRealOid(objectId);
256255
ext.setRealValue(value);
257-
ext.setRealCritical(critical.isTrue());
256+
ext.setRealCritical(critical.isNil() ? null : critical.isTrue());
258257

259258
return ext;
260259
}
@@ -514,28 +513,24 @@ public Extension(Ruby runtime, RubyClass type) {
514513

515514
private ASN1ObjectIdentifier oid;
516515
private Object value;
517-
private boolean critical;
518-
519-
void setRealOid(ASN1ObjectIdentifier oid) {
520-
this.oid = oid;
521-
}
522-
523-
void setRealValue(Object value) {
524-
this.value = value;
525-
}
526-
527-
void setRealCritical(boolean critical) {
528-
this.critical = critical;
529-
}
516+
private Boolean critical;
530517

531518
ASN1ObjectIdentifier getRealOid() {
532519
return oid;
533520
}
534521

522+
void setRealOid(ASN1ObjectIdentifier oid) {
523+
this.oid = oid;
524+
}
525+
535526
Object getRealValue() {
536527
return value;
537528
}
538529

530+
void setRealValue(Object value) {
531+
this.value = value;
532+
}
533+
539534
byte[] getRealValueBytes() throws IOException {
540535
if ( value instanceof RubyString ) {
541536
return ((RubyString) value).getBytes();
@@ -551,8 +546,20 @@ byte[] getRealValueBytes() throws IOException {
551546
}
552547
}
553548

554-
boolean getRealCritical() {
555-
return critical;
549+
boolean isRealCritical() {
550+
return critical == null ? Boolean.FALSE : critical.booleanValue();
551+
}
552+
553+
//Boolean getRealCritical() {
554+
// return critical;
555+
//}
556+
557+
void setRealCritical(boolean critical) {
558+
this.critical = Boolean.valueOf(critical);
559+
}
560+
561+
private void setRealCritical(Boolean critical) {
562+
this.critical = critical;
556563
}
557564

558565
@JRubyMethod(name = "initialize", rest = true, visibility = Visibility.PRIVATE)
@@ -594,8 +601,11 @@ public IRubyObject oid(final ThreadContext context) {
594601

595602
@JRubyMethod(name="oid=")
596603
public IRubyObject set_oid(final ThreadContext context, IRubyObject arg) {
597-
warn(context, "WARNING: unimplemented method called: Extension#oid=");
598-
return false ? arg : context.runtime.getNil();
604+
if ( arg instanceof RubyString ) {
605+
setRealOid( ASN1.getObjectIdentifier( context.runtime, arg.toString() ) );
606+
return arg;
607+
}
608+
throw context.runtime.newTypeError(arg, context.runtime.getString());
599609
}
600610

601611
private static final byte[] CA_ = { 'C','A',':' };
@@ -807,30 +817,31 @@ else if ( name.getTagNo() == GeneralName.iPAddress ) {
807817

808818
@JRubyMethod(name="value=")
809819
public IRubyObject set_value(final ThreadContext context, IRubyObject arg) {
810-
//warn(context, "WARNING: unimplemented method called: Extension#value=");
811-
//return false ? arg : context.runtime.getNil();
812-
setRealValue(arg); return arg;
820+
if ( arg instanceof RubyString ) {
821+
setRealValue(arg); return arg;
822+
}
823+
throw context.runtime.newTypeError(arg, context.runtime.getString());
813824
}
814825

815826
@JRubyMethod(name="critical?")
816827
public IRubyObject critical_p() {
817-
return getRuntime().newBoolean(critical);
828+
return getRuntime().newBoolean( isRealCritical() );
818829
}
819830

820831
@JRubyMethod(name="critical=")
821832
public IRubyObject set_critical(final ThreadContext context, IRubyObject arg) {
822-
//warn(context, "WARNING: unimplemented method called: Extension#critical=");
823-
//return false ? arg : context.runtime.getNil();
824-
setRealCritical(arg.isTrue()); return arg;
833+
setRealCritical( arg.isTrue() ); return arg;
825834
}
826835

827836
@JRubyMethod
828837
public IRubyObject to_der() {
829838
final ASN1EncodableVector vec = new ASN1EncodableVector();
830839
try {
831-
vec.add(getRealOid());
832-
vec.add(getRealCritical() ? DERBoolean.TRUE : DERBoolean.FALSE);
833-
vec.add(new DEROctetString(getRealValueBytes()));
840+
vec.add( getRealOid() );
841+
if ( critical != null ) { // NOTE: likely a hack Boolean.FALSE should also get skipped
842+
vec.add(critical.booleanValue() ? DERBoolean.TRUE : DERBoolean.FALSE);
843+
}
844+
vec.add( new DEROctetString(getRealValueBytes()) );
834845
return RubyString.newString(getRuntime(), new DLSequence(vec).getEncoded(ASN1Encoding.DER));
835846
}
836847
catch (IOException e) {

‎src/test/ruby/asn1/test_x509ext.rb

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
# coding: US-ASCII
2+
require File.expand_path('../test_helper', File.dirname(__FILE__))
3+
require 'jopenssl/load'
4+
5+
class TestX509Extension < Test::Unit::TestCase
6+
7+
def test_attrs
8+
ext = OpenSSL::X509::Extension.new('1.1.1.1.1.1', 'foo')
9+
assert_equal false, ext.critical?
10+
11+
ext.critical = nil
12+
assert_equal false, ext.critical?
13+
14+
ext.critical = true
15+
assert_equal true, ext.critical?
16+
17+
assert_equal 'foo', ext.value
18+
19+
ext.value = 'bar'
20+
assert_equal 'bar', ext.value
21+
22+
assert_equal '1.1.1.1.1.1', ext.oid
23+
24+
# TODO ASN1 needs to fallback to ASN1Registry
25+
#ext.oid = '1.2'
26+
#assert_equal 'member-body', ext.oid
27+
end
28+
29+
def test_to_der # reproducing #389
30+
ext = OpenSSL::X509::Extension.new('1.1.1.1.1.1', 'foo')
31+
32+
mri_to_der = "0\f\x06\x05)\x01\x01\x01\x01\x04\x03foo"
33+
assert_equal mri_to_der, ext.to_der
34+
# MRI 1.8 "0\f\006\005)\001\001\001\001\004\003foo"
35+
# MRI 2.x "0\f\x06\x05)\x01\x01\x01\x01\x04\x03foo"
36+
37+
dec = OpenSSL::ASN1.decode(ext.to_der)
38+
39+
assert_instance_of OpenSSL::ASN1::Sequence, dec
40+
assert_equal 2, ( value = dec.value ).size
41+
42+
assert_instance_of OpenSSL::ASN1::ObjectId, value[0]
43+
# assert_equal 4, value[0].tag
44+
assert_equal '1.1.1.1.1.1', value[0].value
45+
46+
assert_instance_of OpenSSL::ASN1::OctetString, value[1]
47+
# assert_equal 6, value[1].tag
48+
assert_equal 'foo', value[1].value
49+
end
50+
51+
def test_to_der_is_the_same_for_non_critical
52+
ext1 = OpenSSL::X509::Extension.new('1.1.1.1.1.1', 'foo')
53+
ext2 = OpenSSL::X509::Extension.new('1.1.1.1.1.1', 'foo', false)
54+
55+
assert_equal ext1.to_der, ext2.to_der
56+
57+
ext1.critical = nil
58+
59+
assert_equal ext1.to_der, ext2.to_der
60+
61+
ext1.critical = false
62+
63+
assert_equal ext1.to_der, ext2.to_der
64+
65+
ext1.critical = true
66+
67+
assert_not_equal ext1.to_der, ext2.to_der
68+
end
69+
70+
end

0 commit comments

Comments
 (0)