Project

General

Profile

Actions

Bug #4030

closed

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

Added by MartinBosslet (Martin Bosslet) over 13 years ago. Updated almost 13 years ago.

Status:
Closed
Target version:
-
ruby -v:
1.9.2-p0
Backport:
[ruby-core:33082]

Description

=begin
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(File.read("signature_file")).to_der
File.open("out_Again", "w") do |out|
out.print(der_string)
end

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:

if(j & V_ASN1_CONSTRUCTED){
/* 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
=end


Files

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
Actions #1

Updated by naruse (Yui NARUSE) over 13 years ago

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

=begin

=end

Actions #2

Updated by MartinBosslet (Martin Bosslet) over 13 years ago

=begin
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
=end

Actions #3

Updated by naruse (Yui NARUSE) over 13 years ago

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

Actions #4

Updated by MartinBosslet (Martin Bosslet) over 13 years ago

=begin
Ok, thanks for that!
=end

Actions #5

Updated by tenderlovemaking (Aaron Patterson) over 13 years ago

=begin
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.
=end

Actions #6

Updated by tenderlovemaking (Aaron Patterson) over 13 years ago

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

=begin

=end

Actions #7

Updated by MartinBosslet (Martin Bosslet) over 13 years ago

=begin
I changed the code to feature the 0 argument initialize!
=end

Actions #8

Updated by Anonymous over 13 years ago

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

=begin
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.

=end

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0