From a5317e85e9fe47bd9a1cf34bc373476de173ca0b Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sun, 15 Jan 2017 02:14:49 +0900 Subject: [PATCH] asn1: do not treat EOC octets as part of content octets We currently treat end-of-contents octets as the encoding of a value whose tag is universal class and the number is zero, and require users to put one in the end of 'value' array. But, the end-of-contents are not really part of the content octets, and this is annoying. Do not require users to put an EOC object in the content when encoding, and don't produce an EOC object when decoding an encoding that uses indefinite length form. --- ext/openssl/ossl_asn1.c | 16 +++++++++++---- test/test_asn1.rb | 44 ++++++++++++++++++++--------------------- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/ext/openssl/ossl_asn1.c b/ext/openssl/ossl_asn1.c index 2c04df1ff..637c61a4e 100644 --- a/ext/openssl/ossl_asn1.c +++ b/ext/openssl/ossl_asn1.c @@ -725,6 +725,12 @@ ossl_asn1cons_encode_value(VALUE self) if (indef_len && rb_obj_is_kind_of(item, cASN1EndOfContent)) { if (i != RARRAY_LEN(ary) - 1) ossl_raise(eASN1Error, "illegal EOC octets in value"); + + /* + * EOC is not really part of the content, but we required to add one + * at the end in the past. + */ + break; } item = ossl_to_der_if_possible(item); @@ -758,7 +764,7 @@ ossl_asn1data_encode_value(VALUE self) static VALUE to_der_internal(VALUE self, int tag_class, int tag_number) { - int length, constructed; + int length, constructed, indef_len; VALUE encoded, value, str; unsigned char *p; @@ -768,7 +774,8 @@ to_der_internal(VALUE self, int tag_class, int tag_number) value = rb_ary_entry(encoded, 1); StringValue(value); - if (RTEST(ossl_asn1_get_indefinite_length(self))) { + indef_len = RTEST(ossl_asn1_get_indefinite_length(self)); + if (indef_len) { if (constructed == 0) ossl_raise(eASN1Error, "indefinite form used for primitive encoding"); constructed = 2; @@ -780,8 +787,9 @@ to_der_internal(VALUE self, int tag_class, int tag_number) ASN1_put_object(&p, constructed, RSTRING_LENINT(value), tag_number, tag_class); memcpy(p, RSTRING_PTR(value), RSTRING_LEN(value)); p += RSTRING_LEN(value); + if (indef_len) + ASN1_put_eoc(&p); ossl_str_adjust(str, p); - /* TODO: put EOC automatically even if it does not appear in value */ return str; } @@ -914,13 +922,13 @@ int_ossl_asn1_decode0_cons(unsigned char **pp, long max_len, long length, value = ossl_asn1_decode0(pp, available_len, &off, depth + 1, yield, &inner_read); *num_read += inner_read; available_len -= inner_read; - rb_ary_push(ary, value); if (indefinite && ossl_asn1_tag(value) == V_ASN1_EOC && ossl_asn1_get_tag_class(value) == sym_UNIVERSAL) { break; } + rb_ary_push(ary, value); } if (tc == sym_UNIVERSAL) { diff --git a/test/test_asn1.rb b/test/test_asn1.rb index e20c4f31d..1c66ffbc4 100644 --- a/test/test_asn1.rb +++ b/test/test_asn1.rb @@ -319,10 +319,8 @@ def test_sequence OpenSSL::ASN1::Sequence.new([]), OpenSSL::ASN1::OctetString.new(B(%w{ 00 })) ]) - expected = OpenSSL::ASN1::Sequence.new([ - OpenSSL::ASN1::OctetString.new(B(%w{ 00 })), - OpenSSL::ASN1::EndOfContent.new - ]) + + expected = OpenSSL::ASN1::Sequence.new([OpenSSL::ASN1::OctetString.new(B(%w{ 00 }))]) expected.indefinite_length = true encode_decode_test B(%w{ 30 80 04 01 00 00 00 }), expected @@ -334,6 +332,14 @@ def test_sequence ]) obj.indefinite_length = true assert_unencodable obj + + # The last EOC in value is ignored if indefinite length form is used + expected = OpenSSL::ASN1::Sequence.new([ + OpenSSL::ASN1::OctetString.new(B(%w{ 00 })), + OpenSSL::ASN1::EndOfContent.new + ]) + expected.indefinite_length = true + encode_test B(%w{ 30 80 04 01 00 00 00 }), expected end def test_set @@ -343,10 +349,7 @@ def test_set OpenSSL::ASN1::Sequence.new([]), OpenSSL::ASN1::OctetString.new(B(%w{ 00 })) ]) - expected = OpenSSL::ASN1::Set.new([ - OpenSSL::ASN1::OctetString.new(B(%w{ 00 })), - OpenSSL::ASN1::EndOfContent.new - ]) + expected = OpenSSL::ASN1::Set.new([OpenSSL::ASN1::OctetString.new(B(%w{ 00 }))]) expected.indefinite_length = true encode_decode_test B(%w{ 31 80 04 01 00 00 00 }), expected end @@ -406,12 +409,15 @@ def test_basic_asn1data encode_decode_test B(%w{ 41 81 80 } + %w{ AB CD } * 64), OpenSSL::ASN1::ASN1Data.new(B(%w{ AB CD } * 64), 1, :APPLICATION) encode_decode_test B(%w{ 41 82 01 00 } + %w{ AB CD } * 128), OpenSSL::ASN1::ASN1Data.new(B(%w{ AB CD } * 128), 1, :APPLICATION) encode_decode_test B(%w{ 61 00 }), OpenSSL::ASN1::ASN1Data.new([], 1, :APPLICATION) + obj = OpenSSL::ASN1::ASN1Data.new([OpenSSL::ASN1::ASN1Data.new(B(%w{ AB CD }), 2, :PRIVATE)], 1, :APPLICATION) + obj.indefinite_length = true + encode_decode_test B(%w{ 61 80 C2 02 AB CD 00 00 }), obj obj = OpenSSL::ASN1::ASN1Data.new([ OpenSSL::ASN1::ASN1Data.new(B(%w{ AB CD }), 2, :PRIVATE), OpenSSL::ASN1::EndOfContent.new ], 1, :APPLICATION) obj.indefinite_length = true - encode_decode_test B(%w{ 61 80 C2 02 AB CD 00 00 }), obj + encode_test B(%w{ 61 80 C2 02 AB CD 00 00 }), obj obj = OpenSSL::ASN1::ASN1Data.new(B(%w{ AB CD }), 1, :UNIVERSAL) obj.indefinite_length = true assert_unencodable obj @@ -435,12 +441,12 @@ def test_basic_constructed encode_test B(%w{ 21 00 }), OpenSSL::ASN1::Constructive.new([], 1, nil, :UNIVERSAL) encode_test B(%w{ A1 00 }), OpenSSL::ASN1::Constructive.new([], 1, nil, :CONTEXT_SPECIFIC) encode_test B(%w{ 21 04 04 02 AB CD }), OpenSSL::ASN1::Constructive.new([octet_string], 1) - obj = OpenSSL::ASN1::Constructive.new([ - octet_string, - OpenSSL::ASN1::EndOfContent.new - ], 1) + obj = OpenSSL::ASN1::Constructive.new([octet_string], 1) obj.indefinite_length = true encode_decode_test B(%w{ 21 80 04 02 AB CD 00 00 }), obj + obj = OpenSSL::ASN1::Constructive.new([octet_string, OpenSSL::ASN1::EndOfContent.new], 1) + obj.indefinite_length = true + encode_test B(%w{ 21 80 04 02 AB CD 00 00 }), obj end def test_prim_explicit_tagging @@ -535,32 +541,26 @@ def test_recursive_octet_string_parse assert_equal(OpenSSL::ASN1::Constructive, asn1.class) assert_universal(OpenSSL::ASN1::OCTET_STRING, asn1) assert_equal(true, asn1.indefinite_length) - assert_equal(4, asn1.value.size) + assert_equal(3, 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.indefinite_length) - assert_equal(2, nested1.value.size) + assert_equal(1, nested1.value.size) oct1 = nested1.value[0] assert_universal(OpenSSL::ASN1::OCTET_STRING, oct1) assert_equal(false, oct1.indefinite_length) - assert_universal(OpenSSL::ASN1::EOC, nested1.value[1]) - assert_equal(false, nested1.value[1].indefinite_length) nested2 = asn1.value[1] assert_equal(OpenSSL::ASN1::Constructive, nested2.class) assert_universal(OpenSSL::ASN1::OCTET_STRING, nested2) assert_equal(true, nested2.indefinite_length) - assert_equal(2, nested2.value.size) + assert_equal(1, nested2.value.size) oct2 = nested2.value[0] assert_universal(OpenSSL::ASN1::OCTET_STRING, oct2) assert_equal(false, oct2.indefinite_length) - assert_universal(OpenSSL::ASN1::EOC, nested2.value[1]) - assert_equal(false, nested2.value[1].indefinite_length) oct3 = asn1.value[2] assert_universal(OpenSSL::ASN1::OCTET_STRING, oct3) assert_equal(false, oct3.indefinite_length) - assert_universal(OpenSSL::ASN1::EOC, asn1.value[3]) - assert_equal(false, asn1.value[3].indefinite_length) end def test_decode_constructed_overread