Closed Bug 655873 Opened 13 years ago Closed 12 years ago

mozilla.org Responsys id in config.php

Categories

(mozilla.org :: Security Assurance: Applications, task)

Product:
mozilla.org
Component:
Security Assurance: Applications
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: ozten, Unassigned)

References

Details

There are a couple issues with mozilla.org/phonebook

1) It's config is checked into SVN, instead of a config.php-dist
2) It has the sekret Responsys id in the config. This is an out of date value.

http://svn.mozilla.org/projects/mozilla.org/trunk/phonebook/includes/config.php
Blocks: 654124
Renamed config.php to config.php-dist on trunk.
r88559.

We need to add a deployment process to this project.
We need to loop in Operations to keep this Responsys key up to date.

David, any ideas here?
Adding Fred who is technical owner for www.mozilla.org and James who has experience with Responsys on mozilla.com (and since the sites are merging we should probably follow whatever mozilla.com does here so it's easy to integrate later).

Fred or James, any feedback on comment #1?
(In reply to comment #2)
The main question is how soon can we get Operations to own this config file?
(In reply to comment #1)
Well it turns out working in trunk wasn't the "safe" thing to do... that auto deploys to production...

We should document the mozilla.org workflow and show this wiki page to volunteers before they commit any code. Does this page exist? David, can you create it?

Once I am sure I understand the flow, I'll back-port this work to dev and stage.
> We should document the mozilla.org workflow and show this wiki page to 
> volunteers before they commit any code. Does this page exist?

https://wiki.mozilla.org/Mozilla.org/How_to_Work_with_Site
Back ported trunk commit to staging in r88600.
Filed Bug#656199 to get this config file copied over and updated on stage.
Depends on: 656199
The staging version of this system is now working correctly, using the copy-and-fill-in-key method. This will also be what happens in production. There are now no keys in current versions of files in SVN.

Gerv
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Sorry I didn't see this earlier, do you all still need any help with this? Do you need the new Responsys key?
(In reply to comment #9)
> Sorry I didn't see this earlier, do you all still need any help with this?
> Do you need the new Responsys key?

James, can you throw up a page under Websites on Mana, listing the current key and basically a short overview of wth it's used for? :)

Thanks!
(In reply to comment #8)

I see the current Responsys ID checked into SVN, tip of trunk

http://viewvc.svn.mozilla.org/vc/projects/mozilla.org/trunk/phonebook/includes/config.php?view=markup

@gerv - Can you rm and have Operations deploy this file
@Web Security - Do we need to change the Responsys ID again?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #11)
> @Web Security - Do we need to change the Responsys ID again?

Yes, any IDs that have been publicly exposed need to be disabled.
I didn't mean to check in that file (given that multiple people have now done this, perhaps we should consider whether this deployment pattern is actually a safe one?) But in fact, AIUI, although the bugs on this issue are a maze of twisty little passages all alike, and some of them are hidden from my eyes, that's not the current one, that's the (an) old one. Albeit one which still works.

Gerv
(In reply to comment #10)
> (In reply to comment #9)
> > Sorry I didn't see this earlier, do you all still need any help with this?
> > Do you need the new Responsys key?
> 
> James, can you throw up a page under Websites on Mana, listing the current
> key and basically a short overview of wth it's used for? :)
> 
> Thanks!

Done.

https://mana.mozilla.org/wiki/display/websites/Responsys+configuration
This key is the subject of bug 657794 regarding disabling it. According to a comment in that bug, the new key is (apparently) in bug 646627, which I can't see. I'm hoping that is the one Jabba used when he got the phonebook working on the main site.

Gerv
(In reply to comment #11)
> (In reply to comment #8)
> 
> I see the current Responsys ID checked into SVN, tip of trunk
> 
> http://viewvc.svn.mozilla.org/vc/projects/mozilla.org/trunk/phonebook/
> includes/config.php?view=markup
> 
> @gerv - Can you rm and have Operations deploy this file
> @Web Security - Do we need to change the Responsys ID again?

Sigh. config's should never be checked into a VCS.

I believe we are about to switch to using Responsys' SOAP API, so lets just remove it from the current config in SVN and not worry about it. In the next month we won't be using it anyway.
(In reply to comment #15)
> This key is the subject of bug 657794 regarding disabling it. According to a
> comment in that bug, the new key is (apparently) in bug 646627, which I
> can't see. I'm hoping that is the one Jabba used when he got the phonebook
> working on the main site.
> 
> Gerv

You have the new key already. See:

https://mana.mozilla.org/wiki/display/websites/Responsys+configuration
Ah :-| I was fooled by the fact that it seems like these keys are not random, but have the first few letters in common.

> Sigh. config's should never be checked into a VCS.

Indeed. But it's a very easy mistake to make. 

svn add .
svn ci -m "Wahey! I can finally ship my stuff after security review!"

The fact that this disclosure keeps happening suggests to me that this method of keeping things secret is too easy to get wrong.

Gerv
(In reply to comment #18)
> Ah :-| I was fooled by the fact that it seems like these keys are not
> random, but have the first few letters in common.
> 
> > Sigh. config's should never be checked into a VCS.
> 
> Indeed. But it's a very easy mistake to make. 
> 
> svn add .
> svn ci -m "Wahey! I can finally ship my stuff after security review!"

`svn add .` is dangerous and should only be used if you are pedantic about adding things to the ignore registry. Every project has files which shouldn't be committed to the shared repo, so either be careful and only add files you want to commit or monitor your ignores very regularly.

> The fact that this disclosure keeps happening suggests to me that this
> method of keeping things secret is too easy to get wrong.

This is the first accident. Previously we weren't aware that this key was even used for authentication, and it was even hardcoded into source code. So we recently moved it into the config and changed it.

How else are we supposed to keep authentication keys secret?
Re comment  #14, is mana.mozilla.org a public site?   I tried going to that page and didn't get asked to log in (but maybe it knows I'm accessing it from the Mozilla MV network?)
(In reply to comment #20)

> Re comment  #14, is mana.mozilla.org a public site?

No, since it is a repository of all our sekrit config values, etc.
mana.mozilla.org is not public; I was asked for a login.

jlongster: my suggestion would be a pre-commit hook which rejected all attempts to check in files named "config.php" (or some other chosen name). (Sadly, it seems SVN doesn't have support for global svn:ignore patterns which apply even to newly-created directories.)

Gerv
How did config.php leave the admin host and get out?
Gerv -- config files should have a distributable version in the source code filled with default values that can be edited upon deployment.  You shouldn't check in local parameters for an app that will deployed in different environments.  Has nothing to do with SVN or anything else.

It's a simple mistake, easily avoidable, most of us have done it once.  However, don't blame the RCS -- it shouldn't have to know what's sensitive or what's not.   That's a human's decision.

For future reference, things not to check in:
- Flash player
- Java
- Passwords
- Private API keys
- Private SSH keys
- Windows 7
- Live animals
Note that the usual solution is to never have a config.php anywhere in the source code and instead have a "config-dist.php" that gets copied over.  It fulfills two functions:

1. you do not get conflicts between the local config.php and the server config.php upon update
2. modified config.php files never get checked in with your current checkin/changeset
morgamic: I know this. comment 24/25 is exactly what we did. See phonebook/includes/config.php-dist. 

I accept that it was my mistake; but I do think that we could do more to help people not shoot themselves in the foot in this way.

shyam: I was testing the code on the staging branch and, in order to test it fully, I needed a config.php. So I made one, using what I thought was an older, soon-to-be-disabled but working responsys ID.

After I had verified my fixes, and got permission to ship the code, I did:

cp -r staging/phonebook production/phonebook
cd staging/phonebook
find . -name ".svn" -delete
svn add .
svn ci

That's how. :-| 

Although I'm confused about why it didn't work without jabba doing something - given that if I checked in config.php, he shouldn't have needed to create one. I was in bed whenever he did what he did, so I don't know what it was.

Gerv
(In reply to comment #26)
> I accept that it was my mistake; but I do think that we could do more to
> help people not shoot themselves in the foot in this way.

I'm open to hearing your suggested method of doing this.  (not committing that we will use it, but you are the first person I know of to think there is a better way)
Config files should usually be part of:

.gitignore -- in git
svn:ignore (property) -- in SVN

http://svnbook.red-bean.com/en/1.1/ch07s02.html#svn-ch-7-sect-2.3.3
QA Contact: chris → mcoates
QA Contact: mcoates → jstevensen
Status: REOPENED → RESOLVED
Closed: 13 years ago → 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.