codeforamerica / clean

Apply for CalFresh in SF
http://demo.cleanassist.org
MIT License
20 stars 7 forks source link

Basic security audit #35

Open lippytak opened 10 years ago

lippytak commented 10 years ago

At some point...at least:

daguar commented 10 years ago

Is this v2? (I can do it, just wanna know.)

lippytak commented 10 years ago

Nope. Not IMO atleast. I think we should do this before strangers apply without our in person support, whenever that may be.

On Saturday, May 10, 2014, Dave Guarino notifications@github.com wrote:

Is this v2? (I can do it, just wanna know.)

— Reply to this email directly or view it on GitHubhttps://github.com/daguar/calfresh-and-so-clean/issues/35#issuecomment-42763195 .

Sent from thumbs

daguar commented 10 years ago

TIL:

All default appname.herokuapp.com domains are SSL-enabled already and can be accessed simply by using https, e.g., https://appname.herokuapp.com. https://devcenter.heroku.com/articles/ssl-endpoint

so I just pushed a change implementing forced HTTPS redirects when in any environment besides local development. This will need to be revisited should we use a custom (i.e., non-herokuapp.com) domain.

(I've also made your bullet to-do's checklist boxes, and checked the first one off.)

lippytak commented 10 years ago

TIL:

alanjosephwilliams commented 10 years ago

Let me know if other team members need to sign an NDA as well.

daguar commented 10 years ago

@migurski made the suggestion of considering EC2 with an encrypted /tmp folder holding the generated images, since Heroku's shared hosting which opens more theoretical attack vectors.

lippytak commented 10 years ago

Adding a couple more (not dev...)

daguar commented 10 years ago

I walked through the code with @migurski yesterday. Here are the key things that came out of it (overall seems like we're good, just need some minor tinkering to make it even more secure):

daguar commented 10 years ago

Checking off the "use Phaxio's HTTPS endpoint" because the Ruby API wrapper we use does that, per https://github.com/gristmill/phaxio/blob/master/lib/phaxio/client.rb#L3

daguar commented 10 years ago

Checking off "Get one dev to sanity check the code/config setup" because @migurski is a dev I think.

daguar commented 10 years ago

I've added 2 new items to the list above:

migurski commented 10 years ago

So good. Regarding the 303 + data URI, the response should include finely-tuned cache-control response headers. Do you plan to serve over SSL?

daguar commented 10 years ago

Do you plan to serve over SSL?

Most definitely (and we are currently) because we're dealing with some sensitive inputs.

So good.

Feel free to expand on what you like about the code/project, Mike. :smile_cat:

migurski commented 10 years ago

Most definitely (and we are currently) because we're dealing with some sensitive inputs.

Great, that solves a lot right there.