* ext/openssl/ossl_asn1.c: Fix decoding of infinite length values.

Simplified ossl_asn1_decode0 by splitting it into three separate
functions. Add tests.
[Ruby 1.9 - Bug #4374][ruby-core:35123]


git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@31700 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

3 files changed, 308 insertions, 136 deletions

diff --git a/ChangeLog b/ChangeLog
index 977c67b98f..d905a538a1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+Mon May 23 05:58:14 2011  Martin Bosslet  <Martin.Bosslet@googlemail.com>
+
+	* ext/openssl/ossl_asn1.c: Fix decoding of infinite length values.
+	Simplified ossl_asn1_decode0 by splitting it into three separate
+	functions. Add tests.
+	[Ruby 1.9 - Bug #4374][ruby-core:35123]
+
 Mon May 23 04:03:46 2011  Martin Bosslet  <Martin.Bosslet@googlemail.com>
 
 	* ext/openssl/ossl_asn1.c (ossl_asn1_initialize): Allow creation of
diff --git a/ext/openssl/ossl_asn1.c b/ext/openssl/ossl_asn1.c
index aa531ba572..69c736c160 100644
--- a/ext/openssl/ossl_asn1.c
+++ b/ext/openssl/ossl_asn1.c
@@ -20,6 +20,10 @@ struct timeval {
 #endif
 
 static VALUE join_der(VALUE enumerable);
+static VALUE ossl_asn1_decode0(unsigned char **pp, long length, long *offset,
+			       int depth, int yield, long *num_read);
+static VALUE ossl_asn1_initialize(int argc, VALUE *argv, VALUE self);
+static VALUE ossl_asn1eoc_initialize(VALUE self);
 
 /*
  * DATE conversion
@@ -447,6 +451,16 @@ decode_time(unsigned char* der, int length)
     return ret;
 }
 
+static VALUE
+decode_eoc(unsigned char *der, int length)
+{
+    VALUE ret;
+    if (length != 2 || !(der[0] == 0x00 && der[1] == 0x00))
+	ossl_raise(eASN1Error, NULL);
+
+    return rb_str_new("", 0);
+}
+
 /********/
 
 typedef struct {
@@ -751,138 +765,206 @@ ossl_asn1data_to_der(VALUE self)
 }
 
 static VALUE
-ossl_asn1_decode0(unsigned char **pp, long length, long *offset, long depth,
-		  int once, int yield)
+int_ossl_asn1_decode0_prim(unsigned char **pp, long length, int hlen, int tag,
+			   VALUE tc, long *num_read)
 {
-    unsigned char *start, *p;
-    const unsigned char *p0;
-    long len, off = *offset;
-    int hlen, tag, tc, j, infinite = 0;
-    VALUE ary, asn1data, value, tag_class;
+    VALUE value, asn1data;
+    unsigned char *p;
+    long flag = 0;
 
-    ary = rb_ary_new();
     p = *pp;
-    while(length > 0){
-	start = p;
-	p0 = p;
-	j = ASN1_get_object(&p0, &len, &tag, &tc, length);
-	p = (unsigned char *)p0;
-	if(j & 0x80) ossl_raise(eASN1Error, NULL);
-	hlen = rb_long2int(p - start);
-	if(yield){
-	    VALUE arg = rb_ary_new();
-	    rb_ary_push(arg, LONG2NUM(depth));
-	    rb_ary_push(arg, LONG2NUM(off));
-	    rb_ary_push(arg, LONG2NUM(hlen));
-	    rb_ary_push(arg, LONG2NUM(len));
-	    rb_ary_push(arg, (j & V_ASN1_CONSTRUCTED) ? Qtrue : Qfalse);
-	    rb_ary_push(arg, ossl_asn1_class2sym(tc));
-	    rb_ary_push(arg, INT2NUM(tag));
-	    rb_yield(arg);
+
+    if(tc == sUNIVERSAL && tag < ossl_asn1_info_size) {
+	switch(tag){
+	case V_ASN1_EOC:
+	    value = decode_eoc(p, hlen+length);
+	    break;
+	case V_ASN1_BOOLEAN:
+	    value = decode_bool(p, hlen+length);
+	    break;
+	case V_ASN1_INTEGER:
+	    value = decode_int(p, hlen+length);
+	    break;
+	case V_ASN1_BIT_STRING:
+	    value = decode_bstr(p, hlen+length, &flag);
+	    break;
+	case V_ASN1_NULL:
+	    value = decode_null(p, hlen+length);
+	    break;
+	case V_ASN1_ENUMERATED:
+	    value = decode_enum(p, hlen+length);
+	    break;
+	case V_ASN1_OBJECT:
+	    value = decode_obj(p, hlen+length);
+	    break;
+	case V_ASN1_UTCTIME:           /* FALLTHROUGH */
+	case V_ASN1_GENERALIZEDTIME:
+	    value = decode_time(p, hlen+length);
+	    break;
+	default:
+	    /* use original value */
+	    p += hlen;
+	    value = rb_str_new((const char *)p, length);
+	    break;
 	}
-	length -= hlen;
-	off += hlen;
-	if(len > length) ossl_raise(eASN1Error, "value is too short");
-	if((tc & V_ASN1_PRIVATE) == V_ASN1_PRIVATE)
-	    tag_class = sPRIVATE;
-	else if((tc & V_ASN1_CONTEXT_SPECIFIC) == V_ASN1_CONTEXT_SPECIFIC)
-	    tag_class = sCONTEXT_SPECIFIC;
-	else if((tc & V_ASN1_APPLICATION) == V_ASN1_APPLICATION)
-	    tag_class = sAPPLICATION;
-	else
-	    tag_class = sUNIVERSAL;
-	if(j & V_ASN1_CONSTRUCTED){
-	    if((j == 0x21) && (len == 0)){
-		long lastoff = off;
-                infinite = 1;
-		value = ossl_asn1_decode0(&p, length, &off, depth+1, 0, yield);
-		len = off - lastoff;
-	    }
-	    else value = ossl_asn1_decode0(&p, len, &off, depth+1, 0, yield);
+    }
+    else {
+	p += hlen;
+	value = rb_str_new((const char *)p, length);
+    }
+
+    *pp += hlen + length;
+    *num_read = hlen + length;
+
+    if (tc == sUNIVERSAL && tag < ossl_asn1_info_size && ossl_asn1_info[tag].klass) {
+	VALUE klass = *ossl_asn1_info[tag].klass;
+	VALUE args[4];
+	args[0] = value;
+	args[1] = INT2NUM(tag);
+	args[2] = Qnil;
+	args[3] = ID2SYM(tc);
+	asn1data = rb_obj_alloc(klass);
+	ossl_asn1_initialize(4, args, asn1data);
+	if(tag == V_ASN1_BIT_STRING){
+	    rb_ivar_set(asn1data, sivUNUSED_BITS, LONG2NUM(flag));
 	}
-	else{
-	    if ((j & 0x01) && (len == 0)) {
-		ossl_raise(eASN1Error, "Infinite length for primitive value");
-	    }
-	    value = rb_str_new((const char *)p, len);
-	    p += len;
-	    off += len;
+    }
+    else {
+	asn1data = rb_obj_alloc(cASN1Data);
+	ossl_asn1data_initialize(asn1data, value, INT2NUM(tag), ID2SYM(tc));
+    }
+
+    return asn1data;
+}
+
+static VALUE
+int_ossl_asn1_decode0_cons(unsigned char **pp, long length, long *offset,
+			   int depth, int yield, int j, int tag, VALUE tc,
+			   long *num_read)
+{
+    VALUE value, asn1data, ary;
+    int infinite;
+    long off = *offset;
+
+    infinite = (j == 0x21);
+    ary = rb_ary_new();
+
+    while (length > 0 || infinite) {
+	long inner_read = 0;
+	value = ossl_asn1_decode0(pp, length, &off, depth + 1, yield, &inner_read);
+	*num_read += inner_read;
+	rb_ary_push(ary, value);
+	length -= inner_read;
+	
+	if (infinite && NUM2INT(ossl_asn1_get_tag(value)) == V_ASN1_EOC)
+	    break;
+    }
+
+    if (tc == sUNIVERSAL && (tag == V_ASN1_SEQUENCE || V_ASN1_SET)) {
+	VALUE args[4];
+	VALUE klass = *ossl_asn1_info[tag].klass;
+	if (infinite && tag != V_ASN1_SEQUENCE && tag != V_ASN1_SET) {
+	    asn1data = rb_obj_alloc(cASN1Constructive);
 	}
-	if(tag_class == sUNIVERSAL &&
-	   tag < ossl_asn1_info_size && ossl_asn1_info[tag].klass){
-	    VALUE klass = *ossl_asn1_info[tag].klass;
-	    long flag = 0;
-	    if(!rb_obj_is_kind_of(value, rb_cArray)){
-		int l = rb_long2int(hlen + len);
-		switch(tag){
-		case V_ASN1_BOOLEAN:
-		    value = decode_bool(start, l);
-		    break;
-		case V_ASN1_INTEGER:
-		    value = decode_int(start, l);
-		    break;
-		case V_ASN1_BIT_STRING:
-		    value = decode_bstr(start, l, &flag);
-		    break;
-		case V_ASN1_NULL:
-		    value = decode_null(start, l);
-		    break;
-		case V_ASN1_ENUMERATED:
-		    value = decode_enum(start, l);
-		    break;
-		case V_ASN1_OBJECT:
-		    value = decode_obj(start, l);
-		    break;
-		case V_ASN1_UTCTIME:           /* FALLTHROUGH */
-		case V_ASN1_GENERALIZEDTIME:
-		    value = decode_time(start, l);
-		    break;
-		default:
-		    /* use original value */
-		    break;
-		}
-	    }
-            if (infinite && !(tag == V_ASN1_SEQUENCE || tag == V_ASN1_SET)){
-                asn1data = rb_funcall(cASN1Constructive,
-                                      rb_intern("new"),
-                                      4,
-                                      value,
-                                      INT2NUM(tag),
-                                      Qnil,
-                                      ID2SYM(tag_class));
-            }
-            else{
-                if (tag == V_ASN1_EOC){
-                    asn1data = rb_funcall(cASN1EndOfContent,
-                                          rb_intern("new"),
-                                          0);
-                }
-                else{
-                    asn1data = rb_funcall(klass, rb_intern("new"), 1, value);
-                }
-            }
-	    if(tag == V_ASN1_BIT_STRING){
-		rb_ivar_set(asn1data, sivUNUSED_BITS, LONG2NUM(flag));
-	    }
+	else {
+	    asn1data = rb_obj_alloc(klass);
 	}
-	else{
-            asn1data = rb_funcall(cASN1Data, rb_intern("new"), 3,
-				  value, INT2NUM(tag), ID2SYM(tag_class));
-        }
-
-        if (infinite)
-            ossl_asn1_set_infinite_length(asn1data, Qtrue);
-        else
-            ossl_asn1_set_infinite_length(asn1data, Qfalse);
-
-	rb_ary_push(ary, asn1data);
-	length -= len;
-        if(once) break;
+	args[0] = ary;
+	args[1] = INT2NUM(tag);
+	args[2] = Qnil;
+	args[3] = ID2SYM(tc);
+	ossl_asn1_initialize(4, args, asn1data);
+    }
+    else {
+	VALUE args[3];
+	args[0] = ary;
+	args[1] = INT2NUM(tag);
+	args[2] = ID2SYM(tc);
+	asn1data = rb_obj_alloc(cASN1Data);
+	ossl_asn1data_initialize(asn1data, ary, INT2NUM(tag), ID2SYM(tc));
     }
-    *pp = p;
+
+    if (infinite)
+	ossl_asn1_set_infinite_length(asn1data, Qtrue);
+    else
+	ossl_asn1_set_infinite_length(asn1data, Qfalse);
+
     *offset = off;
+    return asn1data;
+}
 
-    return ary;
+static VALUE
+ossl_asn1_decode0(unsigned char **pp, long length, long *offset, int depth,
+		  int yield, long *num_read)
+{
+    unsigned char *start, *p;
+    const unsigned char *p0;
+    long len, inner_read = 0, off = *offset;
+    int hlen, tag, tc, j;
+    VALUE asn1data, tag_class;
+
+    p = *pp;
+    start = p;
+    p0 = p;
+    j = ASN1_get_object(&p0, &len, &tag, &tc, length);
+    p = (unsigned char *)p0;
+    if(j & 0x80) ossl_raise(eASN1Error, NULL);
+    if(len > length) ossl_raise(eASN1Error, "value is too short");
+    if((tc & V_ASN1_PRIVATE) == V_ASN1_PRIVATE)
+	tag_class = sPRIVATE;
+    else if((tc & V_ASN1_CONTEXT_SPECIFIC) == V_ASN1_CONTEXT_SPECIFIC)
+	tag_class = sCONTEXT_SPECIFIC;
+    else if((tc & V_ASN1_APPLICATION) == V_ASN1_APPLICATION)
+	tag_class = sAPPLICATION;
+    else
+	tag_class = sUNIVERSAL;
+
+    hlen = p - start;
+
+    if(yield) {
+	VALUE arg = rb_ary_new();
+	rb_ary_push(arg, LONG2NUM(depth));
+	rb_ary_push(arg, LONG2NUM(*offset));
+	rb_ary_push(arg, LONG2NUM(hlen));
+	rb_ary_push(arg, LONG2NUM(len));
+	rb_ary_push(arg, (j & V_ASN1_CONSTRUCTED) ? Qtrue : Qfalse);
+	rb_ary_push(arg, ossl_asn1_class2sym(tc));
+	rb_ary_push(arg, INT2NUM(tag));
+	rb_yield(arg);
+    }
+
+    if(j & V_ASN1_CONSTRUCTED) {
+	long max_len = len == 0 ? length : len;
+	*pp += hlen;
+	off += hlen;
+	asn1data = int_ossl_asn1_decode0_cons(pp, max_len, &off, depth, yield, j, tag, tag_class, &inner_read);
+	inner_read += hlen;
+    }
+    else {
+    	if ((j & 0x01) && (len == 0)) ossl_raise(eASN1Error, "Infinite length for primitive value");
+	asn1data = int_ossl_asn1_decode0_prim(pp, len, hlen, tag, tag_class, &inner_read);
+	off += hlen + len;
+    }
+    if (num_read)
+	*num_read = inner_read;
+    if (len != 0 && inner_read != hlen + len) {
+	ossl_raise(eASN1Error,
+		   "Type mismatch. Bytes read: %ld Bytes available: %ld",
+		   inner_read, hlen + len);
+    }
+
+    *offset = off;
+    return asn1data;
+}
+
+static void
+int_ossl_decode_sanity_check(long len, long read, long offset)
+{
+    if (len != 0 && (read != len || offset != len)) {
+	ossl_raise(eASN1Error,
+		   "Type mismatch. Total bytes read: %ld Bytes available: %ld Offset: %ld",
+		   read, len, offset);
+    }
 }
 
 /*
@@ -910,14 +992,15 @@ static VALUE
 ossl_asn1_traverse(VALUE self, VALUE obj)
 {
     unsigned char *p;
-    long offset = 0;
     volatile VALUE tmp;
+    long len, read = 0, offset = 0;
 
     obj = ossl_to_der_if_possible(obj);
     tmp = rb_str_new4(StringValue(obj));
     p = (unsigned char *)RSTRING_PTR(tmp);
-    ossl_asn1_decode0(&p, RSTRING_LEN(tmp), &offset, 0, 0, 1);
-
+    len = RSTRING_LEN(tmp);
+    ossl_asn1_decode0(&p, len, &offset, 0, 1, &read);
+    int_ossl_decode_sanity_check(len, read, offset);
     return Qnil;
 }
 
@@ -938,15 +1021,15 @@ ossl_asn1_decode(VALUE self, VALUE obj)
 {
     VALUE ret, ary;
     unsigned char *p;
-    long offset = 0;
     volatile VALUE tmp;
+    long len, read = 0, offset = 0;
 
     obj = ossl_to_der_if_possible(obj);
     tmp = rb_str_new4(StringValue(obj));
     p = (unsigned char *)RSTRING_PTR(tmp);
-    ary = ossl_asn1_decode0(&p, RSTRING_LEN(tmp), &offset, 0, 1, 0);
-    ret = rb_ary_entry(ary, 0);
-
+    len = RSTRING_LEN(tmp);
+    ret = ossl_asn1_decode0(&p, len, &offset, 0, 0, &read);
+    int_ossl_decode_sanity_check(len, read, offset);
     return ret;
 }
 
@@ -966,17 +1049,26 @@ ossl_asn1_decode(VALUE self, VALUE obj)
 static VALUE
 ossl_asn1_decode_all(VALUE self, VALUE obj)
 {
-    VALUE ret;
+    VALUE ary, val;
     unsigned char *p;
-    long offset = 0;
+    long len, tmp_len = 0, read = 0, offset = 0;
     volatile VALUE tmp;
 
     obj = ossl_to_der_if_possible(obj);
     tmp = rb_str_new4(StringValue(obj));
     p = (unsigned char *)RSTRING_PTR(tmp);
-    ret = ossl_asn1_decode0(&p, RSTRING_LEN(tmp), &offset, 0, 0, 0);
-
-    return ret;
+    len = RSTRING_LEN(tmp);
+    tmp_len = len;
+    ary = rb_ary_new();
+    while (tmp_len > 0) {
+	long tmp_read = 0;
+	val = ossl_asn1_decode0(&p, tmp_len, &offset, 0, 0, &tmp_read);
+	rb_ary_push(ary, val);
+	read += tmp_read;
+	tmp_len -= tmp_read;
+    }
+    int_ossl_decode_sanity_check(len, read, offset);
+    return ary;
 }
 
 /*
diff --git a/test/openssl/test_asn1.rb b/test/openssl/test_asn1.rb
index 62d0eb476d..e29bf438b9 100644
--- a/test/openssl/test_asn1.rb
+++ b/test/openssl/test_asn1.rb
@@ -215,6 +215,34 @@ class  OpenSSL::TestASN1 < Test::Unit::TestCase
     end
   end
 
+  def test_decode_all
+    expected = %w{ 02 01 01 02 01 02 02 01 03 }
+    raw = [expected.join('')].pack('H*')
+    ary = OpenSSL::ASN1.decode_all(raw)
+    assert_equal(3, ary.size)
+    ary.each_with_index do |asn1, i|
+      assert_universal(OpenSSL::ASN1::INTEGER, asn1)
+      assert_equal(i + 1, asn1.value)
+    end
+  end
+
+  def test_create_inf_length_primitive
+    expected = %w{ 24 80 04 01 61 00 00 }
+    raw = [expected.join('')].pack('H*')
+    val = OpenSSL::ASN1::OctetString.new('a')
+    cons = OpenSSL::ASN1::Constructive.new([val,
+                                            OpenSSL::ASN1::EndOfContent.new],
+                                            OpenSSL::ASN1::OCTET_STRING,
+                                            nil,
+                                            :UNIVERSAL)
+    cons.infinite_length = true
+    assert_equal(nil, cons.tagging)
+    assert_equal(raw, cons.to_der)
+    asn1 = OpenSSL::ASN1.decode(raw)
+    assert(asn1.infinite_length)
+    assert_equal(raw, asn1.to_der)
+  end
+
   def test_seq_infinite_length
     begin
       content = [ OpenSSL::ASN1::Null.new(nil),
@@ -315,7 +343,7 @@ class  OpenSSL::TestASN1 < Test::Unit::TestCase
                   OpenSSL::ASN1::EndOfContent.new() ]
       seq = OpenSSL::ASN1::Sequence.new(content, 2, :EXPLICIT)
       seq.infinite_length = true
-      expected = %w{ A2 80 30 80 13 03 61 62 63 00 00 }
+      expected = %w{ A2 80 30 80 13 03 61 62 63 00 00 00 00 }
       raw = [expected.join('')].pack('H*')
       assert_equal(raw, seq.to_der)
       assert_equal(raw, OpenSSL::ASN1.decode(raw).to_der)
@@ -355,7 +383,7 @@ class  OpenSSL::TestASN1 < Test::Unit::TestCase
         1,
         :EXPLICIT)
       cons.infinite_length = true
-      expected = %w{ A1 80 24 80 04 03 61 61 61 00 00 }
+      expected = %w{ A1 80 24 80 04 03 61 61 61 00 00 00 00 }
       raw = [expected.join('')].pack('H*')
       assert_equal(raw, cons.to_der)
       assert_equal(raw, OpenSSL::ASN1.decode(raw).to_der)
@@ -439,5 +467,50 @@ class  OpenSSL::TestASN1 < Test::Unit::TestCase
     end
   end
 
+  def test_recursive_octet_string_parse
+    test = %w{ 24 80 24 80 04 01 01 00 00 24 80 04 01 02 00 00 04 01 03 00 00 }
+    raw = [test.join('')].pack('H*')
+    asn1 = OpenSSL::ASN1.decode(raw)
+    assert_equal(OpenSSL::ASN1::Constructive, asn1.class)
+    assert_universal(OpenSSL::ASN1::OCTET_STRING, asn1)
+    assert_equal(true, asn1.infinite_length)
+    assert_equal(4, asn1.value.size)
+    nested1 = asn1.value[0]
+    assert_equal(OpenSSL::ASN1::Constructive, nested1.class)
+    assert_universal(OpenSSL::ASN1::OCTET_STRING, nested1)
+    assert_equal(true, nested1.infinite_length)
+    assert_equal(2, nested1.value.size)
+    oct1 = nested1.value[0]
+    assert_universal(OpenSSL::ASN1::OCTET_STRING, oct1)
+    assert_equal(false, oct1.infinite_length)
+    assert_universal(OpenSSL::ASN1::EOC, nested1.value[1])
+    assert_equal(false, nested1.value[1].infinite_length)
+    nested2 = asn1.value[1]
+    assert_equal(OpenSSL::ASN1::Constructive, nested2.class)
+    assert_universal(OpenSSL::ASN1::OCTET_STRING, nested2)
+    assert_equal(true, nested2.infinite_length)
+    assert_equal(2, nested2.value.size)
+    oct2 = nested2.value[0]
+    assert_universal(OpenSSL::ASN1::OCTET_STRING, oct2)
+    assert_equal(false, oct2.infinite_length)
+    assert_universal(OpenSSL::ASN1::EOC, nested2.value[1])
+    assert_equal(false, nested2.value[1].infinite_length)
+    oct3 = asn1.value[2]
+    assert_universal(OpenSSL::ASN1::OCTET_STRING, oct3)
+    assert_equal(false, oct3.infinite_length)
+    assert_universal(OpenSSL::ASN1::EOC, asn1.value[3])
+    assert_equal(false, asn1.value[3].infinite_length)
+  end
+
+  private
+
+  def assert_universal(tag, asn1)
+    assert_equal(tag, asn1.tag)
+    if asn1.respond_to?(:tagging)
+      assert_nil(asn1.tagging)
+    end
+    assert_equal(:UNIVERSAL, asn1.tag_class)
+  end
+
 end if defined?(OpenSSL)