From 508816fc65856660913faf0e38a3c5d441003685 Mon Sep 17 00:00:00 2001
From: jackvnimble <jack.wilson.v@gmail.com>
Date: Fri, 26 Jan 2018 14:58:44 -0800
Subject: [PATCH] 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?`.
---
 lib/onelogin/ruby-saml/utils.rb   |   2 +-
 test/certificates/certificate.der | Bin 0 -> 590 bytes
 test/utils_test.rb                |   5 +++++
 3 files changed, 6 insertions(+), 1 deletion(-)
 create mode 100644 test/certificates/certificate.der

diff --git a/lib/onelogin/ruby-saml/utils.rb b/lib/onelogin/ruby-saml/utils.rb
index 4a2f363fd..96836a4bb 100644
--- a/lib/onelogin/ruby-saml/utils.rb
+++ b/lib/onelogin/ruby-saml/utils.rb
@@ -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 = []
diff --git a/test/certificates/certificate.der b/test/certificates/certificate.der
new file mode 100644
index 0000000000000000000000000000000000000000..756d248e21c609e7d894ea8d3c6fd567ac8c089e
GIT binary patch
literal 590
zcmXqLV)8O*V%)rdnTe5!iIKs8myJ`a&7<u*FC!x_D}#ZPA-4f18*?ZNn=n&pF^t2(
z!{qE}C}6+`lHuZEPpwGIEyzhV6fqD2i7@kULq+tG^K%X4#CZ)Z3=ECTjEsyejZLF~
zTuUS_RqSY-kL+?rRtDzAUIv54PNv32hHXrTg~P0TmcDbEZgVZ~&XzCRoHm|V$CLSf
zv2^JdCWZC!{9kXrIiZo>wP(I&?fm<48)lWUOk>(wRQ54V{o-_&hX?vf8@paGO+9<W
zu9N?$=T(un_L591?w<Ir^kiz;tu%9&huZfzS2T%Tnyw)uVapi)eASxp2W|OVqkjH1
zuW2*NW@2V!U|bwv;AbEU3>{fM7BLnPTj_%bmd(`px}9%^N$|g<BCR>ILk#3W(#k9n
z24W4^74U!*2(z#nFf%g#NA@r<s+fU3jr!1)dnr)n&eJ68Te%XxEg>6LZaK46D{I=p
zwVVMrUWY2jhe#UTn(^p~K;Oq~<s5sLPnu^pxLo9}ox1RDncA~e|NCdXI6YtLn4yf|
z@{1GV6i-V{{2^5ob3`mLvDU=S@AITjtE3}UVl!s;$Cus<aI5`t;z0U}2ln+T%k{n;
I4c^rZ0IX%rPXGV_

literal 0
HcmV?d00001

diff --git a/test/utils_test.rb b/test/utils_test.rb
index 2bb592e99..7307132ba 100644
--- a/test/utils_test.rb
+++ b/test/utils_test.rb
@@ -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)