Page MenuHomePhabricator

Security review for MachineVision libraries
Closed, DeclinedPublic

Description

Requesting a security review of the google/auth library used by the MachineVision extension to connect to the Google Cloud Vision API, if this hasn't already happened in the course of the security readiness review (T227346).

https://github.com/googleapis/google-auth-library-php
https://github.com/php-fig/cache

  • google/auth
  • psr/cache

Event Timeline

Reedy renamed this task from Security review for google/auth library to Security review for MachineVision libraries.Nov 7 2019, 3:07 AM
Reedy updated the task description. (Show Details)

google/auth is by far the most important here. That said, I intend to write up a substitute auth client this morning so that we're not blocked on this.

davedevelopment/stiphle I could drop immediately with little concern. Not sure where psr/cache comes in.

https://packagist.org/packages/google/auth

requires

php: >=5.4
firebase/php-jwt: ~2.0|~3.0|~4.0|~5.0
guzzlehttp/guzzle: ~5.3.1|~6.0
guzzlehttp/psr7: ^1.2
psr/http-message: ^1.0
psr/cache: ^1.0

I dropped the libraries from MachineVision in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MachineVision/+/549479/. Now that I know about this part of the process, I'll request security reviews nice and early for necessary libraries (and only strictly necessary libraries) in the future.

I am interested in your opinions on whether we'd be better off using the libraries (particularly google/auth) in the long run or the code is fine as-is. If there's no real gain from using the libraries, perhaps task this can be closed.

I am interested in your opinions on whether we'd be better off using the libraries (particularly google/auth) in the long run or the code is fine as-is. If there's no real gain from using the libraries, perhaps task this can be closed.

I guess it depends on how much the request/auth structure might change in future. The reason to use libraries is to make life easier for people, not re-inventing the wheel etc. With a library, you hopefully only need to bump versions, and maybe make minimal changes.

I guess it's mostly dealers choice. There's no particularly right or wrong answer

Maybe it makes most sense to wait a bit and see how much staying power this feature has. If the extension sticks around long-term, it probably makes sense to switch to the google/auth library, and maybe the full google/cloud-vision library. If not, it's probably not worth the trouble of reviewing them.

Actually, we probably wouldn't go for the full google/cloud-vision library at this point because it's too opinionated about what the runtime environment should look like, and doesn't mesh well with our production environment.

FWIW, there’s nothing to actually review on ‘psr/cache’, but listed because we don’t have it currently

sbassett triaged this task as Medium priority.
sbassett moved this task from Incoming to In Progress on the deprecated-security-team-reviews board.
Reedy changed the task status from Open to Stalled.Dec 10 2019, 8:02 PM

Maybe it makes most sense to wait a bit and see how much staying power this feature has. If the extension sticks around long-term, it probably makes sense to switch to the google/auth library, and maybe the full google/cloud-vision library. If not, it's probably not worth the trouble of reviewing them.

Marking stalled as per this

Jcross subscribed.

Marking declined as there is no actionable work at this time. Please open a new ticket when / if work is needed on this in the future.