upstream curl bug #1371: p12 client certificates code is broken

Bug #1556330 reported by Matthew Hall
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
curl (Ubuntu)
Fix Released
Medium
Unassigned
Trusty
Fix Released
Medium
Unassigned

Bug Description

[Impact]

The bug makes it impossible to use PKCS#12 secure storage of client certificates and private keys with any affected Ubuntu releases. The fix is one line fixing a broken switch statement and was already tested against Ubuntu 14.04 LTS with a rebuilt curl package.

This was fixed in upstream libcurl in the following bug:

https://sourceforge.net/p/curl/bugs/1371/

The bug fix consists of one missing break statement at the end of a case in a switch statement.

I personally patched the bug using source code release curl_7.35.0-1ubuntu2.6.dsc, used in Ubuntu 14.04 LTS, and verified it does indeed fix the bug and all of the package's tests still pass afterwards.

[Test Case]

The bug can be reproduced using the following libcurl parameters (even via CLI, pycurl, etc.).

CURLOPT_SSLCERTTYPE == "P12"
CURLOPT_SSLCERT = path to PKCS#12
CURLOPT_SSLKEY = path to PKCS#12
CURLOPT_SSLKEYPASSWD = key for PKCS#12 if needed

Basically, just use a PKCS#12 format client certificate and private key against some certificate protected web server.

[Regression Potential]

If it could possibly break anything, which is extraordinarily unlikely, it would break one of the three client certificate formats (most likely PKCS#12 but also PEM or DER). Note 1/3 formats is already broken due to the bug. Client certificates of all three types could be checked to prevent this.

Revision history for this message
Matthew Hall (mhall-9) wrote :
Revision history for this message
Matthew Hall (mhall-9) wrote :

Requested nomination for stable release update from Ubuntu Bug Control at 2016-03-12T00:08.

Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "official libcurl patch from Daniel Stenberg" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

tags: added: patch
Revision history for this message
Mathew Hodson (mhodson) wrote :

This is fixed in Wily. Subscribing ubuntu-sponsors.

Changed in curl (Ubuntu):
importance: Undecided → Medium
status: New → Fix Released
Mathew Hodson (mhodson)
description: updated
tags: added: trusty
C de-Avillez (hggdh2)
Changed in curl (Ubuntu):
milestone: none → trusty-updates
milestone: trusty-updates → none
Revision history for this message
Brian Murray (brian-murray) wrote :

The patch isn't really in sponsorable format yet as it is missing a debian/changelog and isn't a debdiff.

Changed in curl (Ubuntu Trusty):
status: New → Triaged
importance: Undecided → Medium
Revision history for this message
Matthew Hall (mhall-9) wrote :

Brian, I was hoping for some community assistance on the changelog and debdiff area because I am absolutely not expert at creating and maintaining custom Debian and Ubuntu packaging related files. If there are some things I should do to create these items documentation is of course welcome.

Revision history for this message
Gianfranco Costamagna (costamagnagianfranco) wrote :

1) go there https://launchpad.net/ubuntu/+source/curl
2) find the dsc link
3) dget -u https://launchpad.net/ubuntu/+archive/primary/+files/curl_7.35.0-1ubuntu2.6.dsc
4) wget your patch https://bugs.launchpad.net/ubuntu/+source/curl/+bug/1556330/+attachment/4596446/+files/libcurl_broken_pkcs12.patch
5) check if xenial is fixed (yes)
6) add-patch libcurl_broken_pkcs12.patch
7) explain changes in changelog
8) upload to ppa (dpkg-buildpackage -S -d && dput ppa:blah/blah-ppa filename_source.changes)
9) dput ubuntu filename_source.changes

I'm not a core-dev, so I can't do "9"
but I did instead:
10) debdiff curl_7.35.0-1ubuntu2.*.dsc > debdiff

and attached here

Revision history for this message
Gianfranco Costamagna (costamagnagianfranco) wrote :

build ongoing here
https://launchpad.net/~costamagnagianfranco/+archive/ubuntu/locutusofborg-ppa/

I'll leave for you, testing my ppa, and fille the paperwork
please read this, and convert this bug to look an SRU bug
https://wiki.ubuntu.com/StableReleaseUpdates
(section 3.1, but read all).

thanks for the patch :)

Revision history for this message
Gianfranco Costamagna (costamagnagianfranco) wrote :

unfortunately it doesn't seem to build.

Revision history for this message
Matthew Hall (mhall-9) wrote : Re: [Bug 1556330] Re: upstream curl bug #1371: p12 client certificates code is broken

> On Mar 12, 2016, at 8:55 AM, LocutusOfBorg <email address hidden> wrote:
>
> unfortunately it doesn't seem to build.

It built perfectly when I modified the source for 14.04 LTS.

Also thanks for the more detailed stable release diff procedures. I did read the stable release update page to write the original report but it didn't explain the commands to run as a community member. Who has access to add the additional procedures you wrote into the page? This would be very helpful for other technical users without knowledge of the special Debian and Ubuntu processes.

Matthew.

Revision history for this message
Rolf Leggewie (r0lf) wrote :

Thanks for the report, debdiff and update.

Quick comment, the upload should be to trusty-proposed, not trusty. The changelog would need to be adjusted accordingly once the FTBFS has been ironed out.

Mathew Hodson (mhodson)
tags: added: bitesize
Revision history for this message
Sebastien Bacher (seb128) wrote :

@Rolf, uploading the serie without "-proposed" is fine (that has been true for some years), on stable series uploads are automatically redirected

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

The debdiff looked fine, but needed fixing. Curl is a particular package as the last two patches get unapplied during the build to accommodate for different library backends. New patches need to get added earlier in the series file.

I've fixed the debdiff, made sure it builds properly, and have uploaded the package for processing by the SRU team.

Thanks!

Changed in curl (Ubuntu Trusty):
status: Triaged → In Progress
Revision history for this message
Gianfranco Costamagna (costamagnagianfranco) wrote :

thanks Marc!

How could I forget about the two quilt pop inside the rules file?

my memory is soooo bad :(

g.

Revision history for this message
Chris J Arges (arges) wrote : Please test proposed package

Hello Matthew, or anyone else affected,

Accepted curl into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/curl/7.35.0-1ubuntu2.7 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in curl (Ubuntu Trusty):
status: In Progress → Fix Committed
tags: added: verification-needed
Revision history for this message
Rolf Leggewie (r0lf) wrote :

Can somebody please provide more detailed test instructions?

Revision history for this message
Matthew Hall (mhall-9) wrote :

Hello,

I am very sorry for the delay in testing this.

I noticed that the package libcurl4-doc is missing. I wasn't sure if it was obsolete, or a mistake, or not included in the test packages only.

Otherwise the package is working perfectly and is ready for release to Ubuntu 14.04 LTS.

Thanks for all of your help to make this fix possible.

Sincerely,
Matthew.

tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package curl - 7.35.0-1ubuntu2.7

---------------
curl (7.35.0-1ubuntu2.7) trusty; urgency=medium

  [ Matthew Hall ]
  * debian/patches/libcurl_broken_pkcs12.patch:
    - fix p12 client certificates (LP: #1556330)

 -- Gianfranco Costamagna <email address hidden> Sat, 12 Mar 2016 17:22:33 +0100

Changed in curl (Ubuntu Trusty):
status: Fix Committed → Fix Released
Revision history for this message
Adam Conrad (adconrad) wrote : Update Released

The verification of the Stable Release Update for curl has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.