Skip to content

Commit 3fde14b

Browse files
committed
asn1: clean up OpenSSL::ASN1::Constructive#to_der
Remove a mysterious behavior in Constructive#to_der: if the 'tagging' attribute is set to :EXPLICIT and it is not an instance of universal tag class classes, it "searches" the original tag from the first value whose encoding is primitive. ary = [ OpenSSL::ASN1.Sequence([ OpenSSL::ASN1.OctetString("abc") ]) ] cons = OpenSSL::ASN1::Constructive.new(ary, 1, :EXPLICIT) cons.to_der #=> "\xA1\x09\x24\x07\x30\x05\x04\x03\x61\x62\x63" # ^ # This 4 comes from the OctetString This is really confusing and nobody seems to be using this behavior. Let's make it raise error instead.
1 parent 3effe62 commit 3fde14b

File tree

2 files changed

+32
-68
lines changed

2 files changed

+32
-68
lines changed

ext/openssl/ossl_asn1.c

Lines changed: 32 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,79 +1167,54 @@ ossl_asn1prim_to_der(VALUE self)
11671167
static VALUE
11681168
ossl_asn1cons_to_der(VALUE self)
11691169
{
1170-
int tag, tn, tc, explicit, constructed = 1;
1171-
int found_prim = 0, seq_len;
1172-
long length;
1170+
int tn, tc, explicit, constructed, value_length;
11731171
unsigned char *p;
11741172
VALUE value, str, inf_length;
11751173

11761174
tn = ossl_asn1_tag(self);
11771175
tc = ossl_asn1_tag_class(self);
11781176
inf_length = ossl_asn1_get_indefinite_length(self);
1179-
if (inf_length == Qtrue) {
1180-
VALUE ary, example;
1181-
constructed = 2;
1182-
if (rb_obj_class(self) == cASN1Sequence ||
1183-
rb_obj_class(self) == cASN1Set) {
1184-
tag = ossl_asn1_default_tag(self);
1185-
}
1186-
else { /* must be a constructive encoding of a primitive value */
1187-
ary = ossl_asn1_get_value(self);
1188-
if (!rb_obj_is_kind_of(ary, rb_cArray))
1189-
ossl_raise(eASN1Error, "Constructive value must be an Array");
1190-
/* Recursively descend until a primitive value is found.
1191-
The overall value of the entire constructed encoding
1192-
is of the type of the first primitive encoding to be
1193-
found. */
1194-
while (!found_prim){
1195-
example = rb_ary_entry(ary, 0);
1196-
if (rb_obj_is_kind_of(example, cASN1Primitive)){
1197-
found_prim = 1;
1198-
}
1199-
else {
1200-
/* example is another ASN1Constructive */
1201-
if (!rb_obj_is_kind_of(example, cASN1Constructive)){
1202-
ossl_raise(eASN1Error, "invalid constructed encoding");
1203-
return Qnil; /* dummy */
1204-
}
1205-
ary = ossl_asn1_get_value(example);
1206-
}
1207-
}
1208-
tag = ossl_asn1_default_tag(example);
1209-
}
1210-
}
1211-
else {
1212-
tag = ossl_asn1_default_tag(self);
1213-
if (tag == -1) /* neither SEQUENCE nor SET */
1214-
tag = ossl_asn1_tag(self);
1215-
}
12161177
explicit = ossl_asn1_is_explicit(self);
12171178
value = join_der(ossl_asn1_get_value(self));
1179+
value_length = RSTRING_LENINT(value);
1180+
constructed = RTEST(inf_length) ? 2 : 1;
1181+
1182+
if (explicit) {
1183+
int length, inner_length, default_tag;
1184+
1185+
default_tag = ossl_asn1_default_tag(self);
1186+
/*
1187+
* non-universal tag class class && explicit tagging; this is not
1188+
* possible because the inner tag number is unknown.
1189+
*/
1190+
if (default_tag == -1)
1191+
ossl_raise(eASN1Error, "explicit tagging of unknown tag");
1192+
1193+
inner_length = ASN1_object_size(constructed, value_length, default_tag);
1194+
length = ASN1_object_size(constructed, inner_length, tn);
1195+
str = rb_str_new(0, length);
1196+
p = (unsigned char *)RSTRING_PTR(str);
1197+
ASN1_put_object(&p, constructed, inner_length, tn, tc);
1198+
ASN1_put_object(&p, constructed, value_length, default_tag, V_ASN1_UNIVERSAL);
1199+
}
1200+
else {
1201+
int length;
12181202

1219-
seq_len = ASN1_object_size(constructed, RSTRING_LENINT(value), tag);
1220-
length = ASN1_object_size(constructed, seq_len, tn);
1221-
str = rb_str_new(0, length);
1222-
p = (unsigned char *)RSTRING_PTR(str);
1223-
if(tc == V_ASN1_UNIVERSAL)
1224-
ASN1_put_object(&p, constructed, RSTRING_LENINT(value), tn, tc);
1225-
else{
1226-
if(explicit){
1227-
ASN1_put_object(&p, constructed, seq_len, tn, tc);
1228-
ASN1_put_object(&p, constructed, RSTRING_LENINT(value), tag, V_ASN1_UNIVERSAL);
1229-
}
1230-
else{
1231-
ASN1_put_object(&p, constructed, RSTRING_LENINT(value), tn, tc);
1232-
}
1203+
length = ASN1_object_size(constructed, value_length, tn);
1204+
str = rb_str_new(0, length);
1205+
p = (unsigned char *)RSTRING_PTR(str);
1206+
ASN1_put_object(&p, constructed, value_length, tn, tc);
12331207
}
1234-
memcpy(p, RSTRING_PTR(value), RSTRING_LEN(value));
1235-
p += RSTRING_LEN(value);
1208+
1209+
memcpy(p, RSTRING_PTR(value), value_length);
1210+
p += value_length;
12361211

12371212
/* In this case we need an additional EOC (one for the explicit part and
12381213
* one for the Constructive itself. The EOC for the Constructive is
12391214
* supplied by the user, but that for the "explicit wrapper" must be
12401215
* added here.
12411216
*/
1242-
if (explicit && inf_length == Qtrue) {
1217+
if (explicit && RTEST(inf_length)) {
12431218
ASN1_put_eoc(&p);
12441219
}
12451220
ossl_str_adjust(str, p);

test/test_asn1.rb

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -507,17 +507,6 @@ def test_cons_implicit_tagging
507507
encode_test B(%w{ A1 80 05 00 00 00 }), seq3
508508
end
509509

510-
def test_octet_string_indefinite_length_explicit_tagging
511-
octets = [ OpenSSL::ASN1::OctetString.new('aaa'),
512-
OpenSSL::ASN1::EndOfContent.new() ]
513-
cons = OpenSSL::ASN1::Constructive.new(octets, 1, :EXPLICIT)
514-
cons.indefinite_length = true
515-
expected = %w{ A1 80 24 80 04 03 61 61 61 00 00 00 00 }
516-
raw = [expected.join('')].pack('H*')
517-
assert_equal(raw, cons.to_der)
518-
assert_equal(raw, OpenSSL::ASN1.decode(raw).to_der)
519-
end
520-
521510
def test_octet_string_constructed_tagging
522511
octets = [ OpenSSL::ASN1::OctetString.new('aaa') ]
523512
cons = OpenSSL::ASN1::Constructive.new(octets, 0, :IMPLICIT)

0 commit comments

Comments
 (0)