I had to look into this recently while undergoing PCI compliance. It turns out that when ubercart makes requests to PayPal, Authorize.net, and CyberSource it doesn't verify SSL certificates. The curl code has this line in all instances:

curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, 0);

The PHP docs on curl_setopt (http://us2.php.net/manual/en/function.curl-setopt.php) have this say about CURLOPT_SSL_VERIFYPEER: "FALSE to stop cURL from verifying the peer's certificate. Alternate certificates to verify against can be specified with the CURLOPT_CAINFO option or a certificate directory can be specified with the CURLOPT_CAPATH option."

I dug around and discovered than the example code for most payment processors sets CURLOPT_SSL_VERIFYPEER to false. But why? This explains it: http://curl.haxx.se/docs/sslcerts.html

tl;dr is that curl ships with it's own list of root CAs. This CA list used to suck and be way different than the list of CAs that ship with web browsers. Which means that https websites that got ssl certs signed by valid CAs still wouldn't verify in curl in many cases. But curl fixed their CA list and made it much better in version 7.18.0. Debian Squeeze currently ships with libcurl 7.21.0, so this isn't actually a problem anymore (for the most part). But still payment processor code all across the web (including in ubercart) isn't verifying SSL certificates.

Not verifying SSL certificates of payment processors is a security bug. It's pretty hard to exploit (the attacker needs to be sitting between your web server and your payment processor) but if it is exploited then your customer's credit card numbers get compromised.

I've attached a patch that just changes the CURLOPT_SSL_VERIFYPEER from 0 to 1, and this should fix the problem.

This should solve the problem, except that there's one small issue with it. I wrote a php script that makes curl requests to all of the payment processor URLs used in ubercart while verifying the peers (verifypeer_test.php_.txt). The output is:

https://test.authorize.net/gateway/transact.dll  ok 
https://secure.authorize.net/gateway/transact.dll  ok 
https://apitest.authorize.net/xml/v1/request.api  ok 
https://api.authorize.net/xml/v1/request.api  ok 
https://orderpagetest.ic3.com/hop/ProcessOrder.do  ok 
https://orderpage.ic3.com/hop/ProcessOrder.do  ok 
https://api-3t.sandbox.paypal.com/nvp  ok 
https://api-3t.paypal.com/nvp  ok 
https://www.sandbox.paypal.com/cgi-bin/webscr  failed 
SSL certificate problem, verify that the CA cert is OK. Details:
error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
https://www.paypal.com/cgi-bin/webscr  ok 

The connections all get made except for the one to www.sandbox.paypal.com. I'm not sure why, since www.paypal.com works fine and they both seem to use the same root CA and certificate chain. I haven't looked into it enough, but my suspicion is that it's a problem on PayPal's end, like maybe they're not serving the complete CA chain on their sandbox domain name, but they are on their main one, and curl doesn't cache intermediate CAs. Also, it doesn't look like ubercart actually makes requests to that URL, only to https://api-3t.sandbox.paypal.com/nvp and https://api-3t.paypal.com/nvp, though I could be wrong.

In any case, I'm applying this patch to my ubercart store. It makes production stores more secure, even if sandbox payments to PayPal might not work.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TR’s picture

Status: Active » Needs review

Yes, you're right about all the above. Moving to "needs review" to let the testbot have a look at the patch.

Do you think you could modify the patch so that the PayPal sandbox doesn't try to verify the certificate? I don't think that's necessary for the sandbox, and I don't want to get still more support questions for the sandbox because of this - the sandbox causes enough already. It would also be great if you could turn your script into a SimpleTest test case that way we could make this part of the automated tests.

micah’s picture

This patch should skip SSL verification for www.sandbox.paypal.com.

I haven't written these test before so it'll take me a little longer to figure it all out, but I'll give it a shot.

TR’s picture

Thanks! Because the test for a valid certificate doesn't involve interactions with an Ubercart site through a browers, I think a unit test case for uc_payment rather than a web test case is the right way to go. You can use modules/simpletest/tests/unicode.test as an example.

TR’s picture

Any progress on this micah? I'd be happy to help you get this done.

nedjo’s picture

longwave’s picture

After seeing Ubercart quoted in the paper at https://crypto.stanford.edu/~dabo/pubs/abstracts/ssl-client-bugs.html I figure we should actually do something about this.

I ran the test script from #0 and they are all reported as OK for me, so I am not sure we need an exception for the PayPal sandbox:

https://test.authorize.net/gateway/transact.dll ok
https://secure.authorize.net/gateway/transact.dll ok
https://apitest.authorize.net/xml/v1/request.api ok
https://api.authorize.net/xml/v1/request.api ok
https://orderpagetest.ic3.com/hop/ProcessOrder.do ok
https://orderpage.ic3.com/hop/ProcessOrder.do ok
https://api-3t.sandbox.paypal.com/nvp ok
https://api-3t.paypal.com/nvp ok
https://www.sandbox.paypal.com/cgi-bin/webscr ok
https://www.paypal.com/cgi-bin/webscr ok

However, I can imagine that on some hosts, cURL is not configured correctly or the root certificates are not available. We should not ship root certificates with Ubercart as this would add too much complexity and we could not easily handle revocation.

We can either simply commit the patch from #0 and wait for store owners to upgrade and see if it causes problems, or we could add a store-wide option to verify SSL peers and default it to on - making it the responsibility of store owners if they turn this option off. Comments on these proposals are welcome.

TR’s picture

Yeah, my comments about the PayPal sandbox were based on the OP's experience that he couldn't get the tests to work with the sandbox. If they work for you then great, we don't have to do anything different.

I vote for committing #0 as is, as a short-term fix. I'd still very much like to see the "verifypeer_test.php_.txt" attachment in #0 be turned into SimpleTest test cases - I volunteered to help do that back in April but the OP seems to have left the building ...

My preference though would be to change all payment methods in Ubercart core to use drupal_http_request() instead of cURL. That way the problem is offloaded to Drupal core. Plus, this is being addressed and will be fixed in D8 core (the fix does involve bundling root certs with Drupal core, BTW), and is slated for backport to D7 core.

It's a little unfair for that paper to single out Ubercart, as this is also a problem with Commerce and any other Drupal module that uses drupal_http_request() and/or cURL for SSL. An ethical security researcher would have contacted the Ubercart maintainers before publishing this sort of information in the public domain.

longwave’s picture

Status: Needs review » Fixed

Committed #0 to both branches, and reopened #511064: Replace cURL usage with Guzzle.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture