Hi Brian, On 09/10/19 11:52 am, Brian May wrote: > My current understanding based on discussions in > https://github.com/openid/ruby-openid/issues/122 is that the following > patch should entirely fix this problem in ruby-openid. > > The discussion seems to be highly confused, and at times the reporter > seems to reject this as being insufficient, but without providing a any > real details. > > As this patch from upstream applied cleanly to Jessie, I imagine it will > apply equally as easily to the other distributions. > https://github.com/openid/ruby-openid/pull/121 > > > diff -Nru ruby-openid-2.5.0debian/debian/changelog ruby-openid-2.5.0debian/debian/changelog > --- ruby-openid-2.5.0debian/debian/changelog 2014-03-15 02:04:12.000000000 +1100 > +++ ruby-openid-2.5.0debian/debian/changelog 2019-10-09 17:00:00.000000000 +1100 > @@ -1,3 +1,11 @@ > +ruby-openid (2.5.0debian-1+deb8u1) jessie-security; urgency=high > + > + * Non-maintainer upload by the LTS Team. > + * CVE-2019-11027 Avoid SSRF for claimed_id request. > + Patch source: https://github.com/openid/ruby-openid/pull/121 > + > + -- Brian May <bam@debian.org> Wed, 09 Oct 2019 17:00:00 +1100 > + > ruby-openid (2.5.0debian-1) unstable; urgency=medium > > * Imported Upstream version 2.5.0debian > diff -Nru ruby-openid-2.5.0debian/debian/patches/CVE-2019-11027.patch ruby-openid-2.5.0debian/debian/patches/CVE-2019-11027.patch > --- ruby-openid-2.5.0debian/debian/patches/CVE-2019-11027.patch 1970-01-01 10:00:00.000000000 +1000 > +++ ruby-openid-2.5.0debian/debian/patches/CVE-2019-11027.patch 2019-10-09 17:00:00.000000000 +1100 > @@ -0,0 +1,30 @@ > +From 8a4c31a6740a949cdc29d956c276ba3c4021dfa8 Mon Sep 17 00:00:00 2001 > +From: Vadim Shaulski <sh.vadim@gmail.com> > +Date: Tue, 16 Apr 2019 19:34:35 +0300 > +Subject: [PATCH] Avoid SSRF for claimed_id request > + > +`verify_discovery_results` sends a request to openid.claimed_id URL. > +Anybody can change claimed_id URL but request still will be sent. > +For example, sending a request to the internal network or localhost: > +https://myserver/callback?_method=post&openid.claimed_id=http://localhost:3000/do_method..... > + > +I think, we must check signature before use any data from the URL > +--- > + lib/openid/consumer/idres.rb | 2 +- > + 1 file changed, 1 insertion(+), 1 deletion(-) > + > +diff --git a/lib/openid/consumer/idres.rb b/lib/openid/consumer/idres.rb > +index 16c1d80..6c4e0a3 100644 > +--- a/lib/openid/consumer/idres.rb > ++++ b/lib/openid/consumer/idres.rb > +@@ -72,9 +72,9 @@ def signed_fields > + def id_res > + check_for_fields > + verify_return_to > +- verify_discovery_results > + check_signature > + check_nonce > ++ verify_discovery_results > + end > + > + def server_url > diff -Nru ruby-openid-2.5.0debian/debian/patches/series ruby-openid-2.5.0debian/debian/patches/series > --- ruby-openid-2.5.0debian/debian/patches/series 1970-01-01 10:00:00.000000000 +1000 > +++ ruby-openid-2.5.0debian/debian/patches/series 2019-10-09 17:00:00.000000000 +1100 > @@ -0,0 +1 @@ > +CVE-2019-11027.patch Just a quick question about this patch since I haven't really tested this at all (however aware of the CVE), Is checking signature before sending a request to openid.claimed_id URL strict enough? Whilst another option that seems doable is to disable Yardis discovery but it might reduce functionalities that others need and also, I don't really like disabling things if we can fix them the right way :) So if it is strict enough to check the signature before *actually* sending a request to URL (to avoid SSRFs), then it should probably be good to go! :D Best, Utkarsh
Attachment:
signature.asc
Description: OpenPGP digital signature