openssl icon indicating copy to clipboard operation
openssl copied to clipboard

Certificate.new vs Certificate.new(string)

Open no6v opened this issue 3 years ago • 3 comments

require "openssl"

cert1 = OpenSSL::X509::Certificate.new
cert1.version = 2
cert1.serial = 1
cert1.subject = cert1.issuer = OpenSSL::X509::Name.new([["CN", "CN"]])
t = Time.now
cert1.not_before = t - 3600
cert1.not_after = t + 3600
cert1.public_key = key = OpenSSL::PKey::EC.generate("prime256v1")
cert1.sign(key, "sha256")

cert2 = OpenSSL::X509::Certificate.new(cert1.to_der)
p cert1 == cert2                      # => true

# cert1: from Certificate.new
p cert1.verify(key)                   # => true
cert1.serial = 2
p cert1.verify(key)                   # => false

# cert2: from Certificate.new(string)
p cert2.verify(key)                   # => true
cert2.serial = 2
p cert2.verify(key)                   # => true (this should be false)

I'm wondering this is why? Confirmed with:

  • RUBY_DESCRIPTION
    • ruby 3.2.0dev (2022-06-10T01:10:27Z master e75cb61d46) [x86_64-linux]
  • OpenSSL::VERSION
    • 3.0.0
  • OpenSSL::OPENSSL_VERSION
    • OpenSSL 3.1.0-dev
    • OpenSSL 1.1.1n 15 Mar 2022
    • LibreSSL 3.5.2

no6v avatar Jun 21 '22 08:06 no6v

If I understand correctly, the cause of this is what is described here: https://www.openssl.org/docs/man3.0/man3/i2d_X509.html

Any function which encodes a structure (i2d_TYPE(), i2d_TYPE() or i2d_TYPE()) may return a stale encoding if the structure has been modified after deserialization or previous serialization. This is because some objects cache the encoding for efficiency reasons.

cert = OpenSSL::X509::Certificate.new(ARGF.read)
der, pem, text = cert.to_der, cert.to_pem, cert.to_text
cert.serial += 1
p der == cert.to_der   # => true
p pem == cert.to_pem   # => true
p text == cert.to_text # => false

This let me confused.

It would be nice to have a method to call i2d_re_X509_tbs().

https://www.openssl.org/docs/man3.0/man3/i2d_re_X509_tbs.html

If, after modification, the X509 object is re-signed with X509_sign(), the encoding is automatically renewed. Otherwise, the encoding of the TBSCertificate portion of the X509 can be manually renewed by calling i2d_re_X509_tbs().

no6v avatar Jun 23 '22 09:06 no6v

I agree with your analysis.

It's also confusing since the behavior is inconsistent between types, e.g., X509_REQ_set_version() updates the modified bit while X509_set_version() doesn't.

It would be nice to have a method to call i2d_re_X509_tbs().

Invalidating cache seems like a side effect, but I think we can add something like #tbs_to_der (just an example) to appropriate classes since it's a feature not present and is nice to have anyway.

rhenium avatar Jul 26 '22 14:07 rhenium

I agree with your analysis.

Thank you.

It's also confusing since the behavior is inconsistent between types, e.g., X509_REQ_set_version() updates the modified bit while X509_set_version() doesn't.

Thanks for the information.

Invalidating cache seems like a side effect, but I think we can add something like #tbs_to_der (just an example) to appropriate classes since it's a feature not present and is nice to have anyway.

It sounds reasonable to me.

And for example, in the case of X509 Certificate, the TBSCertificate portion itself is one of a requisite information when those who want to manually validate SCT (Signed Certificate Timestamp) signatures embedded in ct_precert_scts extension.

Currently Certificate#extensions is immutable and Certificate#extensions= does not invalidate the cache, so it is a bit difficult to extract TBSCertificate portion other than that extension. If we have #tbs_to_der etc., it will be easy to do that.

no6v avatar Jul 28 '22 10:07 no6v

X509_REQ_set_version() updates the modified bit while X509_set_version() doesn't.

This has been fixed in OpenSSL (independently) in https://github.com/openssl/openssl/pull/19271 which I think went to OpenSSL 3.2.0.

Also, https://github.com/ruby/openssl/pull/753 added Certificate#tbs_bytes as a wrapper around i2d_re_X509_tbs().

rhenium avatar Jun 08 '24 12:06 rhenium