search

backdrop-contrib / stripe

This module provides a simple abstraction to use the Stripe PHP SDK.
GNU General Public License v2.0
2 stars 2 forks source link

Including Stripe SDK lays too much security responsibility on Backdrop #7

Closed jlfranklin closed 8 months ago

jlfranklin commented 7 years ago

The Stripe SDK includes their copy of a CA bundle, originally copied from Mozilla, if the comments can be believed. The CA bundle is the linchpin of trust for SSL, and a compromised bundle destroys that trust. This gives rise to two major issues:

  1. By packaging it as part of a module distributed by Backdrop, we inherit responsibility to maintain the CA bundle. Updates due to new CA certs and revocations must be done in a timely manner, as recent events demonstrate that CA certs may get revoked from time to time.

  2. Including the CA bundle in the github repo makes it a juicy target. It's easy for someone to get backdrop-contrib access and include a commit adding their CA cert to the bundle. Done during a busy period or as part of a "refresh" of the SDK, we may not notice for months. Another attack vector is to upload a custom bundle to a hacked Backdrop+Stripe site. Both of these compromise the bundle and make MitM attacks much easier.

The root of the second issue is a problem with the Stripe SDK itself (they should be using the system CA bundle, not their own), but I don't think we want the responsibility of maintaining and distributing a copy, nor do we want to become a target.

The SDK should be removed from the module and site builders should be directed to download a copy of the SDK from Stripe and drop it in the libraries directory.

laryn commented 7 years ago

@jenlampton what are your thoughts? (related to https://github.com/backdrop-contrib/stripe/issues/1 )

jenlampton commented 7 years ago

By packaging it as part of a module distributed by Backdrop, we inherit responsibility to maintain the CA bundle.

I'm okay with this, as we'll have to maintain & pull in the stripe SDK updates anyway.

Including the CA bundle in the github repo makes it a juicy target. It's easy for someone to get backdrop-contrib access and include a commit adding their CA cert to the bundle.

I don't see how this is different than any other contrib project, a malicious contributor could commit anything naughty to any repo.

Another attack vector is to upload a custom bundle to a hacked Backdrop+Stripe site.

This would be the same weather we bundle the library or not, no?

site builders should be directed to download a copy of the SDK from Stripe and drop it in the libraries directory.

There shouldn't be a libraries directory on any Backdrop site, so having a one-off module that requires one is going to be a strange U/DX (and require a lot of explaining).

It sounds like the risk will remains whether we make it painful for our users or not. I'm tempted to sort out the issue upstream (fixing the Stripe SDK to the system CA bundle) and leave the SDK in the module. But I'd love some other feedback from the Backdrop security team. @serundeputy @quicksketch @matt2000 ?

papercrate commented 6 years ago

I know this is a bit old, but curious about how this was resolved (if at all). I noticed that when implementing the stripe_api and stripe_checkout modules, the master php bundle is still called for an comes up in error on the status report. Adding the files to the module > libraries directory or a libraries directory still results in a Missing error, as seen here...

screen shot 2017-10-27 at 9 47 12 pm
jenlampton commented 6 years ago

@papercrate the latest -dev version of the module should bundle the SDK, as far as I'm aware. The latest stable release may not. It could be that the requirements check is still looking for the library in a funny place, we should double check that....

laryn commented 8 months ago

Closing this for now as discussion has dried up and we have a new release.