<a class="header-button" href="https://bugzilla.mozilla.org/home" title="Go to home page"> Bugzilla

Alex Buchanan [:abuchanan]

Comment 2

•

16 years ago

I like this idea. From my perspective, defaulting our code (PHP, Python, etc) to always escape/filter/strip would be nice (although extremely heavy-handed). A policy of (security) code reviews for all code should be mandatory. We have frontend code standards that mention security in passing, we should link from there: https://wiki.mozilla.org/WebDev:FrontendCodeStandards#Security_Standards I think we should have a meeting/brainstorm in person soon for this.

Comment 3

•

16 years ago

File uploads and archives are another common area to have security standards. What about recommended frameworks? Many come with filter/escape/protect libraries built in. Personas was built from scratch, and we've continuously payed for it. And of course, https://wiki.mozilla.org/Security/CSP which brings it's own set of code guidelines, such as no inline scripts. Processes are important too: When should have the security review of Personas happened? * before it hit production, Labs or not. * wayyyy before it became a main feature of 3.6 :) Is Labs responsible for producing secure code? * don't count on it If not, when was the formal hand-off to webdev? * document future hand-offs? * security review at this point Don't we have automated testing to catch the kinds of holes Personas had? (Sentinel?) How do we balance security reviews with the amount of new code produced each month? * patch reviews are great * full code review for new apps * periodic security reviews? ** frequency depends on age of codebase? * don't want over do it, security reviews are a lot of work

OS: Mac OS X → All

Hardware: x86 → All

Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)

Comment 4

•

16 years ago

+1 for frameworks I pay attention to the Django ecosystem, so when something stupid happens it usually affects everyone.

Comment 5

•

16 years ago

I agree with most of the sentiments towards Personas. We did scan it twice with a security tool (unfortunately neither bugzilla nor Mail can find the bug). I'm (now) not convinced a scanning tool will catch 100% of security issues. cc'ing QA, they are a huge help in testing for security issues and probably have an opinion.

James Socol [:jsocol, :james]

Comment 6

•

16 years ago

Found it, we used hyperscan for getpersonas.com

Comment 7

•

16 years ago

(In reply to comment #2) > I think we should have a meeting/brainstorm in person soon for this. +1, at least a few of us should get together and work something out. (Probably not everyone to avoid the gigantic bikeshed.)

Laura Thomson :laura

Comment 8

•

16 years ago

Agree on everything above, just one thing: >defaulting our code (PHP, Python, etc) >to always escape/filter/strip would be nice Let's make sure we do this at the app level, not dependent on config settings (for PHP anyway, not sure how this would apply to python). At one point we had an IT production acceptance document that we had to go through before something could go into prod, and that had "security review" as one of the compliance points. What happened to this process? I vote we bring it back.

Comment 9

•

16 years ago

(In reply to comment #8) > At one point we had an IT production acceptance document that we had to go > through before something could go into prod, and that had "security review" as > one of the compliance points. What happened to this process? I vote we bring > it back. It's still there, we/I didn't follow it.

Reed Loden [:reed]

Comment 10

•

16 years ago

(In reply to comment #6) > Found it, we used hyperscan for getpersonas.com As I mentioned in bug 542193, comment #3, Hyperscan is complete garbage. In my past experience with it, it has rarely offered useful finds and mostly only produces tons of false positives. Did it find anything when it scanned getpersonas.com? I filed bug 542188 on Tuesday to get getpersonas.com added to Sentinel, which is a much better tool, especially with WhiteHat Security backing it with actual human tests.

Comment 11

•

16 years ago

(In reply to comment #2) > I like this idea. From my perspective, defaulting our code (PHP, Python, etc) > to always escape/filter/strip would be nice (although extremely heavy-handed). Heavy handed in the way that it's probably one of the top performance drains for AMO, last I looked. The reason there is that *all* data passed to the view is escaped - even the keys of deeply-nested associative arrays from model query results - whether or not it ever gets included in output. If done selectively at the view level for only those strings to be included in output, it might not be so bad. But, of course, in native PHP templates, that means wrapping everything echo'd in htmlspecialchars() at least. FWIW, I started playing around with Twig templates for PHP (thanks jbalogh!) It seems to have both a sensibly concise way to invoke escaping, as well as a not-too-demanding auto-escaping feature. Additionally, it's pretty performant because it compiles on-the-fly into plain PHP for execution. And, it's based on Django template syntax, so... bonus! http://www.twig-project.org/

Comment 12

•

16 years ago

(In reply to comment #10) > As I mentioned in bug 542193, comment #3, Hyperscan is complete garbage. Doesn't bother me, let's just not use it again. Obviously it didn't find anything useful :(

Austin King [:ozten]

Comment 13

•

16 years ago

Some of this needs to be improved on an organizational level and not a technology solution. Ubiquity had some serious security risks, but that was fine as it was a labs experiment that only got highly technical users. For projects that have a good chance of transitioning to end-user products, say Bespin or Personas, webdev should be tapped early and often for the 'non-innovative' pieces. Admin code, authorization, and authentication stuff wasn't the innovative part of Personas and could have been built by webdev in a couple days to our normal standards. Plumbing out of the wya, labs can be left to innovate in the interesting parts of the project.

Comment 14

•

16 years ago

(In reply to comment #13) > Admin code, authorization, and authentication stuff wasn't the innovative part> > of Personas and could have been built by webdev in a couple days to our normal > standards. Plumbing out of the wya, labs can be left to innovate in the > interesting parts of the project. This needs to be formalized a bit as a relationship between Labs and Webdev, since I've heard this expressed by more than one person in both areas. Rather than just jumping to say "ship it!", we should have a stage of review and possible re-build after a Labs project is ready to "graduate". This could possibly call for retiring the prototype and building a more robust version against the planned or emergent specs/APIs produced during the course of the Labs investigations and experiments. Some of this stuff could be done up front by Webdev to provide "workbench" frameworks to Labs (eg. user authen/authz, admin, etc), but there should be a kind of hand-off at some point.

Reporter

Comment 15

•

16 years ago

(In reply to comment #7) > +1, at least a few of us should get together and work something out. (Probably > not everyone to avoid the gigantic bikeshed.) Agreed, I will schedule something up next week so we can discuss. (In reply to comment #14) > Rather than just jumping to say "ship it!", we should have a stage of review > and possible re-build after a Labs project is ready to "graduate". This is the main reason I created this bug. If we have guidelines for how we should code, we probably could avoid lots of issues. We also won't need to redo everything that comes out of Labs.

Comment 16

•

16 years ago

(In reply to comment #15) > > Rather than just jumping to say "ship it!", we should have a stage of review > > and possible re-build after a Labs project is ready to "graduate". > > This is the main reason I created this bug. If we have guidelines for how we > should code, we probably could avoid lots of issues. We also won't need to redo > everything that comes out of Labs. Well, it would be great if it happens that way, but I'd say that there's enough of a difference between these two things to cause trouble: * Exploring a new product, experimenting with features, trying out concepts * Launching a product that's secure, scalable, localizable, and maintainable Ideally, much of the latter can be worked into the process of the former, but that's not always the case and it can sometimes drag down the research.

Comment 17

•

16 years ago

> * Launching a product that's secure, scalable, localizable, and maintainable Oh, and cww would remind me: supportable :)

Stephen Donner [:stephend] Not actively reading bugmail

Reporter

Comment 18

•

16 years ago

(In reply to comment #10) > I filed bug 542188 on Tuesday to get getpersonas.com added to Sentinel, which > is a much better tool, especially with WhiteHat Security backing it with actual > human tests. Forgot to comment on the scanning... So we all know hyperscan is a pile.. I have used it once since I got here and compared it against Whitehat, have not used it since because it isn't a flexable tool. The biggest issue with Whitehat is the expense and hence why we only scan three sites. There is a significant cost for this service, yet even they miss stuff. Also keep in mind, it is a human reviewing their scanning tools results, not humans doing the manual scan. The tools used to scan are there for getting the obvious issues. There really isn't a substitute for manually trying to break things. Even tools like Whitehat don't get everything.

Comment 19

•

16 years ago

(Just as an ancillary: I'm hoping we can at least get a license for a roving WHS instance, so QA can feel a little more comfortable verifying bugs on staging that it's found in production, which is something Chris and I have briefly talked about already.)

Reporter

Comment 20

•

16 years ago

(In reply to comment #19) > (Just as an ancillary: I'm hoping we can at least get a license for a roving > WHS instance, so QA can feel a little more comfortable verifying bugs on > staging that it's found in production, which is something Chris and I have > briefly talked about already.) I am working on that with WHS. The bummer is they have limits to how it is used and it is more $ then just their standard SE. The kicker is that we can have one site in the system and once we put it in, we can't change it for 30 days. So it isn't that flexible. WHS isn't the silver bullet either as it has missed stuff in the past. It is great to QA stuff before it goes live but nothing beats a code review and manual testing.

Toby Elliott [:telliott]

Comment 21

•

16 years ago

(In reply to comment #15) > (In reply to comment #7) > (In reply to comment #14) > > Rather than just jumping to say "ship it!", we should have a stage of review > > and possible re-build after a Labs project is ready to "graduate". > > This is the main reason I created this bug. If we have guidelines for how we > should code, we probably could avoid lots of issues. We also won't need to redo > everything that comes out of Labs. Agree with this. By having an actual process and schedule for "graduation", it lets us make sure that all the bits are in place. That will sometimes involve recoding, since we aren't going to build a dev prototype for massive scalability (if we weren't going to replace personas with AMO, we'd need to rebuild it, since it wasn't designed to handle where it needs to go). The initial personas server was built in a week to test things out, and the core of it is still there. At the time, there were fewer than 30 personas. All but one of the security problems in personas was in the admin area. We knew there were likely issues there - that section had been hacked up and wasn't even close to feature-complete or checked - but only three people had access. Suddenly, without any warning, it was on the production cluster. I screwed up there by not circling back, but everything should have happened before that point.

Reporter

Updated

•

16 years ago

Assignee: nobody → clyon