curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, 0);

This line in commerce_authnet_aim_request() disables peer verification, which allows man-in-the-middle attacks between sites running this module and Authorize.Net's servers. See #1794680: HTTPS certificate verification and e.g. http://stackoverflow.com/questions/14914330/security-consequences-of-dis... for more information.

Comments

dwkitchen’s picture

Status: Active » Needs review
StatusFileSize
new2.02 KB

I think the best option is to add a option in the gateway settings, default to TRUE.

If the setting is not set then it will fallback to FALSE for upgrading user, maybe should consider a message to tell the site admin to set it.

rszrama’s picture

Priority: Critical » Major

Thought about this most of the morning, and while I like the compromise and think the chances of a MITM attack of this nature are low for any given site, I still don't like providing a UI to "opt" for insecurity. We did that with the example recurring payment method in UC Recurring and basically ended up with a bunch of live sites storing CC numbers in the database, some of whom even made the news for getting hacked and losing that CC data. : P

Soooo... here's what I'm thinking. Let's verifyhost / peer by default for all live transactions - it makes good sense, and the fact that we haven't been stretches back to my original development of the Ubercart payment method modules in a time when all the API docs were written without that verification (apparently to accommodate poor cURL maintenance). For test / sandbox transactions I don't see any real importance in verification, but until we start getting issues of people saying they aren't able to verify SSL certs on test servers, let's just go ahead and verify them, too. (The only reason I mention this is a test in the related Ubercart thread that failed for the PayPal sandbox; shouldn't affect this module regardless.)

For sites that get error messages pertaining to certificate verification, let's have the alternate option be to put the cert file into the module folder or the sites/all/libraries folder and reference that using the CURLOPT_CAINFO parameter. The question becomes how to label the option for that in the UI... perhaps with a checkbox right above the logging settings we just say "Use a local certificate bundle file." with a link to a documentation page explaining how to get the file (http://curl.haxx.se/docs/caextract.html) and where to put it on the server.

Also, for reference's sake, here's the source white paper: http://www.cs.utexas.edu/~shmat/shmat_ccs12.pdf

rszrama’s picture

Status: Needs review » Needs work

See my commit in #1931758-2: CURLOPT_SSL_VERIFYPEER should not be disabled for my eventual solution for PayPal. Basically, I opted not to clutter the user interface at all with options to disable peer verification. The vast majority of users will be fine, and what I do is make a variable available for those minority users who need to specify an alternate CA certificate bundle file. If the variable is set, I populate the CURLOPT_CAINFO setting with its value. Otherwise we trust the system to have up to date information. I'd recommend the same for Authorize.Net and any other Commerce module, obviously using alternate variable names (e.g. commerce_authnet_cacert).

travis-bradbury’s picture

Assigned: Unassigned » travis-bradbury
Issue summary: View changes
travis-bradbury’s picture

Status: Needs work » Needs review
StatusFileSize
new3.17 KB

I followed the commit to the paypal module, but I changed the comment a bit so there's other suggestions before downloading today's CA bundle and never updating it.

  // Commerce Authnet requires SSL peer verification, which may prevent out of
  // date servers from successfully processing API requests. If you get an error
  // related to peer verification, you may need to:
  // * Update libraries like openssl and libcurl.
  // * Use https://github.com/paragonie/certainty to get updated certificates.
  // * Update your php.ini to point to your CA certificate bundle with the
  //   curl.cainfo setting.
  // * Download the CA certificate bundle file from
  //   http://curl.haxx.se/docs/caextract.html, place it in a safe location on
  //   your web server, and update your settings.php to set the
  //   commerce_authnet_cacert variable to contain the absolute path of the
  //   file.
travis-bradbury’s picture

A note about compatibility:

Most sites should not notice anything.

Sites which are the victims of a man-in-the-middle attack may suddenly be able to connect, protecting credit card details from being sent to attackers.

Sites which are running in an environment that is unable to verify authorize.net's certificate, possibly from using a very out of date openssl or system-level CA cert bundle, may be unable to process payments. This is the correct behaviour. You don't want to send payment details until you trust it's really Authorize.net you're communicating with.

heddn’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for that update Travis. I was curious what this meant for backwards compatibility. It makes sense to do this. However, we should make sure this gets mentioned in release notes. But otherwise, this looks good to go. A straight forward solution to a long standing potential security vulnerability.

One might even argue this should use the security finding process, instead of the open forum.

  • mglaman committed 5ef65bc on 7.x-1.x authored by tbradbury
    Issue #1931760 by tbradbury, dwkitchen: CURLOPT_SSL_VERIFYPEER should...
mglaman’s picture

Status: Reviewed & tested by the community » Fixed

Thanks. Committed. Publishing the change record.

Status: Fixed » Closed (fixed)

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