Page MenuHomePhabricator

Remove requirement for security review of well maintained third party libraries
Closed, ResolvedPublic

Description

Not sure if this is going to be a more formal TechCom-RFC or just a decision made within the security team.

This has been brought up by T191638 and the security team wondering if time can be better spent on other tasks

Currently, when people want to add new third party the rules say the library needs security review.

This is looking to change this if the library meets some conditions, and security review can be skipped:

  • Library is actively maintained, by a well known group or developer (like Symfony) with known security policies (things like Symfonys Security Policy are advantageous) and responsive developers
  • Library has no known security issue (disclosed but not fixed)
  • Whoever adds the library has some responsibility for maintaining it in the MediaWiki ecosystem (if there is a security release, updating the usages in your code, and bumping it in MediaWiki-Vendor as necessary)

Exceptions

  • Requests for smaller sections of code (part of larger libraries) to be reviewed, due to specific concerns

If things like the above cannot be resolved, we should look to fork the library if we can't get patches back in upstream (which we have done in some cases in the past). Or, where appropriate, use something else

We should look at getting things like T180278 implemented to remove some of the human intervention. And then updating documentation like https://www.mediawiki.org/wiki/Manual:External_libraries as appropriate

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

I would say additionally, that it should depend on the type of library. While security issues can happen anywhere, I think it makes more sense to be careful with a library that's doing "dangerous" things (e.g. A library responsible for sanitization of html/templating. Or a library that's responsible for creating SQL queries, etc).

In other words, it might still make sense to ask the security team, we just need to give you the flexibility to say "whatever" without a full review:)

chasemp triaged this task as Medium priority.Sep 4 2018, 4:03 PM
chasemp edited projects, added Security-team-backlog; removed Security-Team.

@Reedy and I threw together this bit of documentation, which has a few more specific conditions from what's mentioned within the task description regarding reviews, a good portion of which could probably be automated. IMO, this seems like a good baseline of due-diligence efforts for security reviews performed by the Security-Team of third-party libraries.

Regarding T180278, since php-composer-security-docker is a thing, could that just plug into gerrit for most/all php repos?

@Reedy and I threw together this bit of documentation, which has a few more specific conditions from what's mentioned within the task description regarding reviews, a good portion of which could probably be automated. IMO, this seems like a good baseline of due-diligence efforts for security reviews performed by the Security-Team of third-party libraries.

Regarding T180278, since php-composer-security-docker is a thing, could that just plug into gerrit for most/all php repos?

+1

I vote we resolve this with https://www.mediawiki.org/wiki/Wikimedia_Security_Team/Third_Party_Code_Review_Checklist for now

chasemp claimed this task.