Skip to content

Commit 508816f

Browse files
committed
Allow formatting of certificates that contain \r
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?`.
1 parent 414d144 commit 508816f

File tree

3 files changed

+6
-1
lines changed

3 files changed

+6
-1
lines changed

lib/onelogin/ruby-saml/utils.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class Utils
2222
#
2323
def self.format_cert(cert)
2424
# don't try to format an encoded certificate or if is empty or nil
25-
return cert if cert.nil? || cert.empty? || cert.match(/\x0d/)
25+
return cert if cert.nil? || cert.empty? || !cert.ascii_only?
2626

2727
if cert.scan(/BEGIN CERTIFICATE/).length > 1
2828
formatted_cert = []

test/certificates/certificate.der

590 Bytes
Binary file not shown.

test/utils_test.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ class UtilsTest < Minitest::Test
2929
assert_equal formatted_certificate, OneLogin::RubySaml::Utils.format_cert(invalid_certificate2)
3030
end
3131

32+
it "returns the cert when it's encoded" do
33+
encoded_certificate = read_certificate("certificate.der")
34+
assert_equal encoded_certificate, OneLogin::RubySaml::Utils.format_cert(encoded_certificate)
35+
end
36+
3237
it "reformats the certificate when there line breaks and no headers" do
3338
invalid_certificate3 = read_certificate("invalid_certificate3")
3439
assert_equal formatted_certificate, OneLogin::RubySaml::Utils.format_cert(invalid_certificate3)

0 commit comments

Comments
 (0)