Skip to content

Commit

Permalink
Allow formatting of certificates that contain \r
Browse files Browse the repository at this point in the history
If an idp_cert contains a '\r' it can blow up upon response validation
with `OpenSSL::X509::CertificateError: nested asn1 error` even if the
cert is otherwise valid (or would have been post-formatting).

From the way `OneLogin::RubySaml::Utils.format_cert` is implemented it
would appear that it *is* expected for '\r's to be present since it
tries to strip them appropriately during the formatting below the guard
statement. Unfortunately, the guard statement at the top short circuits
the formatter when certificates contain '\r' since:
```
irb:0> "asldfkj\r".match(/\x0d/)
=> #<MatchData "\r">
```

Removing the `cert.match(/\x0d/)` doesn't actually break any specs but
from the comment it seems that it may have been intended to ensure
that encoded certs (i.e. .der) are not run through the formatter. I've
added a `.der` cert to `tests/certificates` and asserted that it isn't
changed when run through `format_cert` by checking for `ascii_only?`.
  • Loading branch information
id4ho committed Jan 26, 2018
1 parent 414d144 commit 508816f
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/onelogin/ruby-saml/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Utils
#
def self.format_cert(cert)
# don't try to format an encoded certificate or if is empty or nil
return cert if cert.nil? || cert.empty? || cert.match(/\x0d/)
return cert if cert.nil? || cert.empty? || !cert.ascii_only?

if cert.scan(/BEGIN CERTIFICATE/).length > 1
formatted_cert = []
Expand Down
Binary file added test/certificates/certificate.der
Binary file not shown.
5 changes: 5 additions & 0 deletions test/utils_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ class UtilsTest < Minitest::Test
assert_equal formatted_certificate, OneLogin::RubySaml::Utils.format_cert(invalid_certificate2)
end

it "returns the cert when it's encoded" do
encoded_certificate = read_certificate("certificate.der")
assert_equal encoded_certificate, OneLogin::RubySaml::Utils.format_cert(encoded_certificate)
end

it "reformats the certificate when there line breaks and no headers" do
invalid_certificate3 = read_certificate("invalid_certificate3")
assert_equal formatted_certificate, OneLogin::RubySaml::Utils.format_cert(invalid_certificate3)
Expand Down

0 comments on commit 508816f

Please sign in to comment.