Skip to content

Commit

Permalink
asn1: do not treat EOC octets as part of content octets
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@rhenium
rhenium committed Apr 27, 2017
1 parent 27d36a5 commit cc985a2
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 24 deletions.
11 changes: 9 additions & 2 deletions ext/openssl/ossl_asn1.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -671,8 +671,9 @@ to_der_inner(int tag_class, int tag_number, int constructed, int indef_len,
ASN1_put_object(&p, indef_len ? 2 : constructed, clen, tag_number, tag_class);
memcpy(p, RSTRING_PTR(value), clen);
p += clen;
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;
}
Expand Down Expand Up @@ -831,13 +832,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) {
Expand Down Expand Up @@ -1190,6 +1191,12 @@ ossl_asn1cons_to_der(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);
Expand Down
44 changes: 22 additions & 22 deletions test/test_asn1.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -405,12 +408,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
Expand All @@ -434,12 +440,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
Expand Down Expand Up @@ -534,32 +540,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
Expand Down

0 comments on commit cc985a2

Please sign in to comment.