RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1415298 - Possible regression in curl-7.19.7-CVE-2015-3148.patch
Summary: Possible regression in curl-7.19.7-CVE-2015-3148.patch
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: curl
Version: 6.8
Hardware: All
OS: Linux
unspecified
high
Target Milestone: rc
: ---
Assignee: Kamil Dudka
QA Contact: BaseOS QE Security Team
URL:
Whiteboard:
Depends On: CVE-2017-2628
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-01-20 19:37 UTC by Paulo Andrade
Modified: 2020-04-15 15:08 UTC (History)
3 users (show)

Fixed In Version: curl-7.19.7-53.el6_9
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-03-29 08:31:06 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
curl-7.19.7-sfdc01750392.patch (16.83 KB, patch)
2017-02-08 17:34 UTC, Paulo Andrade
no flags Details | Diff

Description Paulo Andrade 2017-01-20 19:37:50 UTC
The issue was started as the command:

curl --negotiate -u : -kv --location-trusted --connect-timeout 5 http://user_link

failing with an error
307 Temporary Redirect to the Active Resource Manager

  Initial check showed that curl-7.19.7-40.el6_6.4.x86_64
did not have the issue, and curl-7.19.7-52.el6_6.4.x86_64
had the problem.

  After a bisect testing packages, we found that the last
"good" one was curl-7.19.7-45.el6.x86_64.

  An extra test was done by building curl-7.19.7-46.el6.x86_64
with the patch curl-7.19.7-CVE-2015-3148.patch disabled, and
it also had the problem corrected.

  Current guess is that the patch is likely problematic,
given the commit comment:

"""
XX: Is Curl_http_done() the right place to do the final cleanup?
"""

Comment 2 Kamil Dudka 2017-01-23 14:56:49 UTC
Thanks for the analysis!  I see there was a regression fix upstream:

https://github.com/curl/curl/commit/curl-7_49_0~79

... which refers to the following issue:

https://github.com/curl/curl/pull/655

Could you please check whether the fix has any effect on the bug in question?

I can prepare a scratch build if needed...

Comment 3 Paulo Andrade 2017-01-24 12:37:04 UTC
(In reply to Kamil Dudka from comment #2)
> Thanks for the analysis!  I see there was a regression fix upstream:
> 
> https://github.com/curl/curl/commit/curl-7_49_0~79
> 
> ... which refers to the following issue:
> 
> https://github.com/curl/curl/pull/655
> 
> Could you please check whether the fix has any effect on the bug in question?

  Unfortunately the error happened again after enabling
back curl-7.19.7-CVE-2015-3148.patch and adding the
suggested patch.

> I can prepare a scratch build if needed...

  Patch was rediffed as:
"""
+diff -up curl-7.19.7/lib/http.c.orig curl-7.19.7/lib/http.c
+--- curl-7.19.7/lib/http.c.orig        2017-01-23 15:51:19.810640755 -0200
++++ curl-7.19.7/lib/http.c     2017-01-23 15:54:57.269206226 -0200
+@@ -1922,8 +1922,10 @@ CURLcode Curl_http_done(struct connectda
+      data->state.negotiate.state == GSS_AUTHSENT) {
+     /* add forbid re-use if http-code != 401/407 as a WA only needed for
+      * 401/407 that signal auth failure (empty) otherwise state will be RECV
+-     * with current code */
+-    if((data->req.httpcode != 401) && (data->req.httpcode != 407))
++     * with current code.
++     * Do not close CONNECT_ONLY connections. */
++    if((data->req.httpcode != 401) && (data->req.httpcode != 407) &&
++       !data->set.connect_only)
+       conn->bits.close = TRUE; /* Negotiate transfer completed */
+     Curl_cleanup_negotiate(data);
+   }
"""

Comment 4 Kamil Dudka 2017-01-24 12:41:01 UTC
Thanks for checking!  I guess you have some local environment to test it.  Could you please check whether the issue occurs also with RHEL-7 or Fedora (lib)curl?

Comment 5 Paulo Andrade 2017-01-24 18:35:05 UTC
  I do not have a test environment. But I build a rhel6 backport
of latest rawhide curl, and asked the user to test it, and revert
back after testing to a rhel6 supported version.
  It did work.
  Build used this patch:
"""
--- curl.spec.orig	2017-01-24 16:30:03.634274220 -0200
+++ curl.spec	2017-01-24 16:30:40.759173884 -0200
@@ -20,12 +20,14 @@
 BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(id -nu)
 BuildRequires: groff
 BuildRequires: krb5-devel
+%if 0%{?fedora} != 0 || 0%{?rhel} > 6
 BuildRequires: libidn2-devel
 BuildRequires: libmetalink-devel
 BuildRequires: libnghttp2-devel
 BuildRequires: libpsl-devel
 BuildRequires: libssh2-devel
 BuildRequires: multilib-rpm-config
+%endif
 BuildRequires: nss-devel
 BuildRequires: openldap-devel
 BuildRequires: openssh-clients
@@ -35,8 +37,10 @@
 BuildRequires: stunnel
 BuildRequires: zlib-devel
 
+%if 0%{?fedora} != 0 || 0%{?rhel} > 6
 # nghttpx (an HTTP/2 proxy) is used by the upstream test-suite
 BuildRequires: nghttp2
+%endif
 
 # perl modules used in the test suite
 BuildRequires: perl(Cwd)
@@ -65,9 +69,11 @@
 
 Requires: libcurl%{?_isa} = %{version}-%{release}
 
+%if 0%{?fedora} != 0 || 0%{?rhel} > 6
 # require at least the version of libssh2 that we were built against,
 # to ensure that we have the necessary symbols available (#525002, #642796)
 %global libssh2_version %(pkg-config --modversion libssh2 2>/dev/null || echo 0)
+%endif
 
 %description
 curl is a command line tool for transferring data with URL syntax, supporting
@@ -82,9 +88,11 @@
 Group: Development/Libraries
 Requires: libssh2%{?_isa} >= %{libssh2_version}
 
+%if 0%{?fedora} != 0 || 0%{?rhel} > 6
 # libnsspem.so is no longer included in the nss package (#1347336)
 BuildRequires: nss-pem
 Requires: nss-pem
+%endif
 
 %description -n libcurl
 libcurl is a free and easy-to-use client-side URL transfer library, supporting
@@ -150,11 +158,13 @@
     --enable-threaded-resolver \
     --with-ca-bundle=%{_sysconfdir}/pki/tls/certs/ca-bundle.crt \
     --with-gssapi${KRB5_PREFIX} \
+%if 0%{?fedora} != 0 || 0%{?rhel} > 6
     --with-libidn2 \
     --with-libmetalink \
     --with-libpsl \
     --with-libssh2 \
     --with-nghttp2 \
+%endif
     --without-ssl --with-nss
 #    --enable-debug
 # use ^^^ to turn off optimizations, etc.
@@ -166,6 +176,7 @@
 
 make %{?_smp_mflags} V=1
 
+%if 0
 %check
 # we have to override LD_LIBRARY_PATH because we eliminated rpath
 LD_LIBRARY_PATH="$RPM_BUILD_ROOT%{_libdir}:$LD_LIBRARY_PATH"
@@ -177,6 +188,7 @@
 
 # run the upstream test-suite
 ./runtests.pl -a -p -v '!flaky'
+%endif
 
 %install
 rm -rf $RPM_BUILD_ROOT
@@ -193,8 +205,10 @@
 install -d $RPM_BUILD_ROOT%{_datadir}/aclocal
 install -m 644 docs/libcurl/libcurl.m4 $RPM_BUILD_ROOT%{_datadir}/aclocal
 
+%if 0%{?fedora} != 0 || 0%{?rhel} > 6
 # Make libcurl-devel multilib-ready (bug #488922)
 %multilib_fix_c_header --file %{_includedir}/curl/curlbuild.h
+%endif
 
 %clean
 rm -rf $RPM_BUILD_ROOT
"""

Comment 6 Kamil Dudka 2017-01-25 14:18:20 UTC
Thanks!  Hopefully it means that the regression has been already fixed upstream.  If we do not have any in house reproducer, I have no idea how to further debug it, except bisecting further to the potential fix.

Could you please check whether it works with curl-7.42.0-1.fc23 (the first upstream release including the fix for CVE-2015-3148)?

https://koji.fedoraproject.org/koji/buildinfo?buildID=629950
http://pkgs.fedoraproject.org/cgit/rpms/curl.git/tree/curl.spec?id=13074767

Comment 7 Kamil Dudka 2017-01-25 15:06:20 UTC
I suspect that this regression could be related to the following sentence from the security advisory [1]:

    "We decided to do this blunt short-term fix to avoid this current problem
    and then continue to work on a better fix."

[1] https://curl.haxx.se/docs/adv_20150422B.html

Comment 8 Paulo Andrade 2017-01-25 20:21:04 UTC
  It works with curl-7.42.0-1.fc23 backported to rhel6.

Comment 10 Kamil Dudka 2017-01-25 20:40:08 UTC
Cool.  This means that we must be missing some commit that landed _before_ the actual fix for CVE-2015-3148.  Could you please check it with curl-7.29.0-35.el7 (the latest RHEL-7 build)?

Comment 11 Paulo Andrade 2017-01-26 17:11:35 UTC
It works with curl-7.29.0-35.el7, rebuilt as
curl-7.29.0-35.el6.x86_64 as well.
Indeed, should be a single/minor missing patch in
rhel6 curl-7.19.7-46.el6.x86_64 or newer.

Comment 12 Paulo Andrade 2017-02-08 17:34:05 UTC
Created attachment 1248648 [details]
curl-7.19.7-sfdc01750392.patch

  ***Note, this is not a proposed patch but hopefully
can help in getting some insight of where else to
look for a possible patch to correct the problem***

  The attached patch was an attempt on backporting
some curl-7.29.0-35.el7 changes to curl-7.19.7-52.el6,
but still, known that backport of curl-7.29.0-35.el7
to rhel6 works, and last rhel6 package that works is
rhel6 curl-7.19.7-45.el6.x86_64

  The attached patch is in hackish mode, as was added
to test if it were the new code under NTLM_WB_ENABLED
preprocessor option that was the cause of the rhel7
package working.

Comment 13 Kamil Dudka 2017-02-09 17:24:38 UTC
Thanks for the patch!

Do I understand it correctly that the latest el6 curl works as expected if you apply attachment #1248648 [details] on top of our downstream patches?

Comment 14 Paulo Andrade 2017-02-09 17:58:26 UTC
  No, I am afraid it does work.

  I just added it to show what was my best guess of
what could resolve the problem, but still did not work,
but hopefully get some feedback about it :)

  My next step is to just run latest rhel6 curl under gdb
in a remote support session, and attempt to find out where
and why it fails.

Comment 15 Paulo Andrade 2017-02-10 16:07:46 UTC
Kamil,

Running under gdb, if I set a breakpoint at
curl-7.19.7/lib/http_negotiate.c:340
like this:

338:   else
339:     conn->allocptr.userpwd = userp;
340:   free(encoded);
341:   return (userp == NULL) ? CURLE_OUT_OF_MEMORY : CURLE_OK;

and in the second time it stops there I call
Curl_cleanup_negotiate like this:

(gdb) call Curl_cleanup_negotiate (conn->data)
(gdb) continue

it works.

  Trying to figure some condition, with some obfuscated data,
I cannot see of any condition to check, to make the call
conditional, other than it had a redirect...

Breakpoint 2, Curl_output_negotiate (conn=0x635500, proxy=false) at http_negotiate.c:340
340       free(encoded);
(gdb) p conn.data.state
$6 = {used_interface = Curl_if_easy, connc = 0x62f150, keeps_speed = {tv_sec = 0, tv_usec = 0}, lastconnect = 0,
  headerbuff = 0x62dad0 "\r\n", headersize = 256,
  buffer = "HTTP/1.1 307 TEMPORARY_REDIRECT\r\nCache-Control: no-cache\r\nExpires: Fri, 10 Feb 2017 15:29:35 GMT\r\nDate: Fri, 10 Feb 2017 15:29:35 GMT\r\nPragma: no-cache\r\nExpires: Fri, 10 Feb 2017 15:29:35 GMT\r\nDate: F"...,
  uploadbuffer = '\000' <repeats 16384 times>, current_speed = -1, this_is_a_follow = true,
  first_host = 0x630320 "EDITED.EXAMPLE.COM", session = 0x62f370, sessionage = 1, tempwrite = 0x0, tempwritesize = 0,
  tempwritetype = 0, scratch = 0x0, errorbuf = false, os_errno = 0, prev_signal = 0, allow_port = false, digest = {
    nonce = 0x0, cnonce = 0x0, realm = 0x0, algo = 0, stale = false, opaque = 0x0, qop = 0x0, algorithm = 0x0, nc = 0},
  proxydigest = {nonce = 0x0, cnonce = 0x0, realm = 0x0, algo = 0, stale = false, opaque = 0x0, qop = 0x0, algorithm = 0x0,
    nc = 0}, negotiate = {state = GSS_AUTHNONE, gss = false, protocol = 0x7ffff7dcf9e7 "Negotiate", status = 0,
    context = 0x631850, server_name = 0x630020, output_token = {length = 1463, value = 0x6343b0}}, proxyneg = {
    state = GSS_AUTHNONE, gss = false, protocol = 0x0, status = 0, context = 0x0, server_name = 0x0, output_token = {
      length = 0, value = 0x0}}, authhost = {want = 4, picked = 4, avail = 0, done = true, multi = false, iestyle = false},
  authproxy = {want = 1, picked = 1, avail = 0, done = true, multi = false, iestyle = false}, authproblem = false,
  expiretime = {tv_sec = 0, tv_usec = 0}, timenode = {smaller = 0x0, larger = 0x0, same = 0x0, key = {tv_sec = 0,
      tv_usec = 0}, payload = 0x0}, most_recent_ftp_entrypath = 0x0, ftp_trying_alternative = false, httpversion = 11,
  expect100header = false, pipe_broke = false, prev_block_had_trailing_cr = false, crlf_conversions = 0, shared_conn = 0x0,
  closed = false, pathbuffer = 0x62fd60 "/EDITED/PATH",
  path = 0x62fd60 "/EDITED/PATH", use_range = false, rangestringalloc = false,
  range = 0x0, resume_from = 0, proto = {http = 0x630290, https = 0x630290, ftp = 0x630290, file = 0x630290,
    telnet = 0x630290, generic = 0x630290, ssh = 0x630290}, current_conn = 0x635500}

  I believe this is not correct, at least in the sense of a backport:
https://github.com/curl/curl/commit/f78ae415d24b9bd89d6c121c556e411fdb21c6aa

and, not fully understanding the CVE, it looks like it is even making the
CVE condition worse:
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2015-3148

by keeping the authentication, but in the user case, failing, apparently
after the redirect.

  So, if reverting this chunk:
diff --git a/lib/http_negotiate.c b/lib/http_negotiate.c
index 535a427..b56e7d0 100644
--- a/lib/http_negotiate.c
+++ b/lib/http_negotiate.c
@@ -338,7 +338,6 @@ CURLcode Curl_output_negotiate(struct connectdata *conn, bool proxy)
   else
     conn->allocptr.userpwd = userp;
   free(encoded);
-  Curl_cleanup_negotiate (conn->data);
   return (userp == NULL) ? CURLE_OUT_OF_MEMORY : CURLE_OK;
 }
 
-- 
it works for the user.

Comment 23 Kamil Dudka 2017-03-29 08:31:06 UTC
Fixed as CVE-2017-2628 via RHSA-2017:0847:
https://rhn.redhat.com/errata/RHSA-2017-0847.html


Note You need to log in before you can comment on or make changes to this bug.