Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tests failing when using OpenSSL library v3 #417

Closed
santiagorodriguez96 opened this issue Dec 29, 2023 · 4 comments · Fixed by #424
Closed

Tests failing when using OpenSSL library v3 #417

santiagorodriguez96 opened this issue Dec 29, 2023 · 4 comments · Fixed by #424

Comments

@santiagorodriguez96
Copy link
Contributor

santiagorodriguez96 commented Dec 29, 2023 •
edited
Loading

Some specs are failing for me when using OpenSSL library v3:

Failures:

  1) Packed attestation #valid? x5c attestation when the attestation certificate doesn't meet requirements because version is invalid fails
     Failure/Error: expect(statement.valid?(authenticator_data, client_data_hash)).to be_falsy

       expected: falsey value
            got: ["Basic_or_AttCA", [#<OpenSSL::X509::Certificate: subject=#<OpenSSL::X509::Name CN=CN,OU=Authenticato...SL::BN:0x000000012125db50>, not_before=2023-12-29 16:12:48 UTC, not_after=2023-12-29 16:13:49 UTC>]]
     # ./spec/webauthn/attestation_statement/packed_spec.rb:196:in `block (6 levels) in <top (required)>'

  2) TPM attestation statement #valid? AttCA attestation when the AIK certificate doesn't meet requirements because version is invalid returns false
     Failure/Error: expect(statement.valid?(authenticator_data, client_data_hash)).to be_falsy

       expected: falsey value
            got: ["AttCA", [#<OpenSSL::X509::Certificate: subject=#<OpenSSL::X509::Name >, issuer=#<OpenSSL::X509::Nam...SL::BN:0x00000001212970a8>, not_before=2023-12-29 16:12:49 UTC, not_after=2023-12-29 16:13:50 UTC>]]
     # ./spec/webauthn/attestation_statement/tpm_spec.rb:367:in `block (6 levels) in <top (required)>'
     # ./spec/webauthn/attestation_statement/tpm_spec.rb:144:in `block (5 levels) in <top (required)>'
     # ./spec/spec_helper.rb:91:in `silence_warnings'
     # ./spec/webauthn/attestation_statement/tpm_spec.rb:141:in `block (4 levels) in <top (required)>'

Finished in 1.87 seconds (files took 0.72988 seconds to load)
303 examples, 2 failures

Failed examples:

rspec ./spec/webauthn/attestation_statement/packed_spec.rb:195 # Packed attestation #valid? x5c attestation when the attestation certificate doesn't meet requirements because version is invalid fails
rspec ./spec/webauthn/attestation_statement/tpm_spec.rb:366 # TPM attestation statement #valid? AttCA attestation when the AIK certificate doesn't meet requirements because version is invalid returns false

Digging around a little, it seems that the issue is that we set up the certificate to have an invalid version (version is 1) but the certificate is later signed which, in newer versions of OpenSSL, updates the version to a valid one (version changes to be 2).

require "bundler/inline"

gemfile(true) do
  ruby "3.2.2"
  source "https://rubygems.org"
  gem "activesupport", "7.1.2"
  gem "minitest", "5.20.0"
  gem "openssl", "3.2.0"
end

require "openssl"
require "active_support"
require "minitest/autorun"

class BugTest < ActiveSupport::TestCase
  def test_1
    puts "======================================================"
    puts "OpenSSL library version is: #{OpenSSL::OPENSSL_VERSION}"
    puts "======================================================"
    puts

    key = OpenSSL::PKey::EC.generate("prime256v1")
    certificate = OpenSSL::X509::Certificate.new

    certificate.version = 1
    certificate.subject = OpenSSL::X509::Name.parse("CN=Root-#{rand(1_000_000)}")
    certificate.issuer = certificate.subject
    certificate.public_key = key

    extension_factory = OpenSSL::X509::ExtensionFactory.new
    extension_factory.subject_certificate = certificate
    extension_factory.issuer_certificate = certificate

    certificate.extensions = [
      extension_factory.create_extension("basicConstraints", "CA:TRUE", true),
      extension_factory.create_extension("keyUsage", "keyCertSign,cRLSign", true),
    ]

    assert_no_difference -> { certificate.version } do
      certificate.sign(key, "SHA256")
    end
  end
end

<<-OUTPUT
# Running:

======================================================
OpenSSL library version is: OpenSSL 3.0.7 1 Nov 2022
======================================================

F

Finished in 0.002432s, 411.1842 runs/s, 822.3685 assertions/s.

  1) Failure:
BugTest#test_1 [bug_report.rb:39]:
#<Proc:0x000000011c63c3e8 bug_report.rb:39 (lambda)> didn't change by 0, but by 1.
Expected: 1
  Actual: 2

1 runs, 2 assertions, 1 failures, 0 errors, 0 skips
OUTPUT

This does not happen when using OpenSSL v1 – that's the reason why in the CI the specs do not fail, as the Rubies installed by setup/ruby come with said version of OpenSSL:

image
@santiagorodriguez96
Copy link
Contributor Author

santiagorodriguez96 commented Dec 29, 2023 •
edited
Loading

This makes me wonder: should we test against the different versions of the OpenSSL gem? How hard would that be? 🤔

@bdewater
Copy link
Collaborator

This changed in OpenSSL 3.2, see openssl/openssl#19271. I see two possible ways to fix this:

  • manpage for X509_sign mentions "If the certificate information includes X.509 extensions, these two functions make sure that the certificate bears X.509 version 3." so we could try not adding extensions in the test.
  • since the check is done in Ruby a stub might also work?

@santiagorodriguez96
Copy link
Contributor Author

santiagorodriguez96 commented Jan 26, 2024 •
edited
Loading

This changed in OpenSSL 3.2, see openssl/openssl#19271

Hmm that's weird – it's failing for me in OpenSSL 3.0 (you can see that in the description) 🙃

  • manpage for X509_sign mentions "If the certificate information includes X.509 extensions, these two functions make sure that the certificate bears X.509 version 3." so we could try not adding extensions in the test.
  • since the check is done in Ruby a stub might also work?

That makes sense, I was thinking on stubbing but the extensions approach seems worth trying 👀

@santiagorodriguez96
Copy link
Contributor Author

Seems like the test does not fail with OpenSSL 3.0 in ubuntu:

https://github.com/santiagorodriguez96/webauthn-ruby/actions/runs/7673777484/job/20917106286?pr=1

@Earlopain Earlopain mentioned this issue Feb 11, 2024
Open
santiagorodriguez96 added a commit that referenced this issue Feb 16, 2024
@santiagorodriguez96
test: fix tests failing with OpenSSL v3
cdd432d 
Fixes #417.

Some tests fail when using OpenSSL v3. The issue is that, for testing
the `valid?` method for attestation statements, we set up a certificate
to have an invalid version (version is 1 initially) and expect the
attestation statement to not be valid. But when setting up the
certificate we have to sign it which, in newer versions of OpenSSL,
updates the version to a valid one (version changes to be 2), thus
causing the expectation to not be met and therefore the spec to fail.

This PR fixes the issue by changing the version after the certificate
has been signed.
@santiagorodriguez96 santiagorodriguez96 mentioned this issue Feb 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix tests failing with OpenSSL v3 cedarcode/webauthn-ruby
2 participants
@bdewater @santiagorodriguez96