From 471caf802f3db64765a2b15cc95eea9a4dbf064e 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 a BER 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. However the end-of-contents are not really part of the content octets. 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 e6029aa81..667b71038 100644 --- a/ext/openssl/ossl_asn1.c +++ b/ext/openssl/ossl_asn1.c @@ -708,6 +708,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); @@ -741,7 +747,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; @@ -751,7 +757,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; @@ -763,8 +770,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; } @@ -897,13 +905,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 179ce451f..1cb489f26 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 @@ -407,12 +410,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 @@ -436,12 +442,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 @@ -536,32 +542,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