Bug #4030

ext/openssl OpenSSL::ASN1::decode / to_der

Added by MartinBosslet (Martin Bosslet) over 7 years ago. Updated about 7 years ago.

Target version:


Hi all,

I have a PKCS#7 SignedData whose EncapContentInfo's content is BER-encoded using indefinite length. If I decode it and then encode it again, e.g. by

der_string = OpenSSL::ASN1::decode("signature_file")).to_der"out_Again", "w") do |out|

then, the resulting file will no longer use the previous encoding, but actually DER-encode the content using definite length.

I think I spotted the reason for this in ext/openssl/ossl_asn1.c:

/* TODO: if j == 0x21 it is indefinite length object. */
if((j == 0x21) && (len == 0)){
long lastoff = off;
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);

Could the encoding be preserved? A simple way would be to cache the initial encoding and the information that infinite length tags were used?

I'd love to write a library for digital signatures in Ruby that supports CAdES (RFC5126) signatures. For these signatures, elements of the SignedData are hashed and on the basis of these hashes, timestamps are generated. For recalculating these hashes it's very important that the encoding is exactly the same as the initial one. Unfortunately, with the current implementation, this would only be possible if the initial signature is already DER-encoded, but it would fail for BER-encoded signatures.

Best regards,
Martin Boßlet

asn1_inf_length.diff (18.1 KB) asn1_inf_length.diff MartinBosslet (Martin Bosslet), 12/01/2010 10:56 AM
asn1_inf_length_2.diff (18.4 KB) asn1_inf_length_2.diff MartinBosslet (Martin Bosslet), 12/12/2010 01:08 AM


#1 Updated by naruse (Yui NARUSE) over 7 years ago

  • Status changed from Open to Assigned
  • Assignee set to nahi (Hiroshi Nakamura)



#2 Updated by MartinBosslet (Martin Bosslet) over 7 years ago

I'd like to propose a patch that would resolve the issue for me.

After analyzing the signatures in more detail I found out that the only BER encodings used were infinite length constructed encodings e.g. for sequence, set and octet string (or bit string) structures. This makes sense in particular for CMS SignedData structures when "attached signatures" are used - encoding the EncapContentInfo using infinite length constructs is the only way to keep streaming support possible.
But the rest of the primitive encodings used DER encoding. Essentially this also means that there is not really a need to cache the entire encoding, it suffices to store the information whether a constructed encoding was encoded with infinite length or not.
To support this, I added "@infinite length" to each ASN1Data (with getter/setter) that is used to determine the encoding rule to be applied when serializing the object. This is only effective for sub-classes of ASN1Constructive, which is why #infinite_length= is explicitly undefined for all ASN1Primitives.
Infinite length octet strings and bit strings are represented as ASN1Constructives whose value is an array of octet strings or bit strings respectively, where these again can be constructed, in ASN1Constructive form, or ,ultimately, primitive.
To construct an infinite length encoding from scratch, an instance of the new class OpenSSL::ASN1::EndOfContent must be added at the end of the respective value array.
I also added a couple of tests that assert the expected behavior and also demonstrate the usage of infinite_length.

Best regards,
Martin Boßlet

#3 Updated by naruse (Yui NARUSE) over 7 years ago

I think, this proposal is reasonable, but the code should be reviewed.
So anyone can review this?

#4 Updated by MartinBosslet (Martin Bosslet) over 7 years ago

Ok, thanks for that!

#5 Updated by tenderlovemaking (Aaron Patterson) over 7 years ago

I've reviewed this patch. It seems fine except for one thing, OpenSSL::ASN1::EndOfContent#initialize is defined with -1, but then ignores the argv and argc. Can we just make the initialize 0 argument?

I haven't tried applying and running the tests yet. But if the change I suggested were made, and the tests all pass, I think this patch would be good.

#6 Updated by tenderlovemaking (Aaron Patterson) over 7 years ago

  • Assignee changed from nahi (Hiroshi Nakamura) to tenderlovemaking (Aaron Patterson)



#7 Updated by MartinBosslet (Martin Bosslet) over 7 years ago

I changed the code to feature the 0 argument initialize!

#8 Updated by Anonymous over 7 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r30178.
Martin, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


Also available in: Atom PDF