Page MenuHomePhabricator

Security Readiness Review For geoip2/geoip2
Closed, ResolvedPublic

Description

Project Information

Description of the tool/project:
This package provides an API for the GeoIP2 web services and databases. The API also works with the free GeoLite2 databases.

Description of how the tool will be used at WMF:
As part of the IP Info extension, we would like to use MaxMind's proprietary dataset. The format they provide is a pre-indexed single-file database which can be queried easily using the geoip2/geoip2 library which is their official PHP library.

Dependencies

Has this project been reviewed before?
Not that we are aware of.

Working test environment
TBD

Post-deployment
Anti-Harassment

Related Objects

StatusSubtypeAssignedTask
ResolvedSTran
ResolvedSTran
OpenNone
OpenNiharika
OpenNiharika
StalledNone
StalledNone
DeclinedNone
OpenNone
DeclinedNone
ResolvedFeatureNone
OpenNone
OpenNone
ResolvedNiharika
ResolvedTchanders
ResolvedNiharika
Resolved AGueyte
ResolvedSTran
ResolvedSTran
Resolved AGueyte
ResolvedNiharika
ResolvedSTran
ResolvedNiharika
ResolvedTchanders
InvalidNone
InvalidNone
InvalidNone
ResolvedSTran
ResolvedSTran
ResolvedSpikephuedx
ResolvedSTran
Resolved TThoabala
Resolvedphuedx
DeclinedTchanders
ResolvedSTran
ResolvedSTran
ResolvedTchanders
ResolvedTchanders
Resolvedsbassett
ResolvedDec 15 2020Tchanders
ResolvedTchanders
ResolvedTchanders
InvalidNone
ResolvedSep 22 2020Tchanders
ResolvedSep 22 2020Tchanders
ResolvedTchanders
Resolveddbarratt
ResolvedTchanders
Resolveddbarratt
ResolvedTchanders
Resolvedsbassett
ResolvedNiharika
InvalidNone
StalledNone
ResolvedSecurityUrbanecm
ResolvedUrbanecm

Event Timeline

We already use maxmind in production in a few places and have a subscription. The geoip2 lib for python seems in use, but I don't see the PHP libs.

The question would be, are these packaged in debian?

The question would be, are these packaged in debian?

I believe we bring PHP libraries into MediaWiki and extensions with composer.

The question would be, are these packaged in debian?

I believe we bring PHP libraries into MediaWiki and extensions with composer.

I had Alternatively, we could create a new web service in production that our extension could make a request to (from PHP). lingering in my brain when I made the packaging comment. Which was confusing on my part as I was doing a bit of a drive-by here.

Typically, the policy for established (and esp large) FLOSS project inclusion has been a cursory review and opinion from the appsec contingent within the security team as part of the Readiness SOP. Note this SOP usually requires a template for requesting service that involves more info than is here. I'll let those folks decide what's appropriate in this case.

I added that tag this should get noticed in the weekly triage.

I had Alternatively, we could create a new web service in production that our extension could make a request to (from PHP). lingering in my brain when I made the packaging comment. Which was confusing on my part as I was doing a bit of a drive-by here.

Apologies if that was confusing. I was thinking it may be more secure to create some sort of "microservice" that would accept an IP address (over the local network from MediaWiki) and return information about that IP address. It obviously wouldn't do any sort of access control (other than only being accessible over the local network). That could isolate the third party code away from MediaWiki.

On the flip side, if it's better to add the library to MediaWiki, that works too! :)

sbassett changed the task status from Open to Stalled.Sep 16 2020, 4:10 PM
sbassett triaged this task as Low priority.
sbassett moved this task from Incoming to Back Orders on the secscrum board.

Hi @dbarratt - we'd appreciate it if you could please provide all of the information requested on our RFS form located here: https://phabricator.wikimedia.org/maniphest/task/edit/form/79/ We are here to answer any questions and additional information regarding our process is available here: https://www.mediawiki.org/wiki/Security/SOP/Security_Readiness_Reviews

Thank you!

dbarratt renamed this task from Security review of geoip2/geoip2 to Security Readiness Review For geoip2/geoip2.Sep 17 2020, 6:34 PM
dbarratt added a project: Security.
dbarratt updated the task description. (Show Details)
dbarratt added a subscriber: Tchanders.

Thanks @Jcross, it looks like we've added the information to this task description. Do you need anything else from us before the review can go ahead?

It would also be great to have an estimate on when security would be able to look at this - thanks!

sbassett added a project: user-sbassett.
sbassett moved this task from Back Orders to In Progress on the secscrum board.

Hi @Tchanders - we've moved this ticket to In Progress and I believe we are planning on having feedback for you in the next two weeks. Please let us know if you have any questions as we move forward. Thanks!

Apologies for the delay on this - I hope to have this review completed by this Friday. So far I haven't seen anything extremely concerning within the geoip2 package.

sbassett changed the task status from Stalled to Open.Dec 3 2020, 8:42 PM
sbassett updated the task description. (Show Details)

Security Review Summary - T262963 - 2020-11-03

Overall, the current vendor code under consideration looks to be very good, which makes sense given that it's from maxmind, an organization whose components are already used within various Wikimedia production infrastructures. I would currently assign an overall risk rating of: low for this package.

geoip2/geoip2

General Security Information

Statistic/InfoValueRisk
Repositoryhttps://github.com/maxmind/GeoIP2-php none
Relevant tagv2.11.0 none
Last commit reviewed (if relevant)d01be5894a none
Recent contributions to code (6 months)21 low
Active developers with > 10 commits5 low
Current overall usage1700 stars, 222 forks low
Current open security issues0 none
Methods for reporting security issuesnone medium

Vulnerable Packages

  1. None found via composer security:check, snyk: low risk

Policy Compliance

  1. I'm assuming that since maxmind is already used within production (as @chasemp noted above) in a few different ways (though not currently at the MediaWiki application layer), that WMF-Legal would be fine with this additional use case.

General Issues

  1. In analyzing and manually reviewing the code, I did not find any major security weaknesses or vulnerabilities, though I would note that:
    1. At this point, given the number of disparate things using maxmind's databases within production, it might make sense to eventually consolodate such requirements into a centralized service. Of course that's well beyond the scope of this project and might involve more complication and frustration than any benefit provided.
    2. I'll likely have more things to say about this package's usage within the IPInfo extension, but I'll plan to keep those issues contextually isolated to the review of that extension (T260822).
sbassett moved this task from In Progress to Our Part Is Done on the secscrum board.
sbassett moved this task from In Progress to Done on the user-sbassett board.