Page MenuHomePhabricator

Security Review For KaiOS Wikipedia app
Closed, ResolvedPublic15 Estimated Story Points

Description

Project Information

Description of the tool/project

This is the Wikipedia application for KaiOS phones. Once accepted into the app stores (official KaiStore and/or JioStore), users will install the app and run it locally. We will send the app to the store in a zip file. It will not be hosted.

Description of how the tool will be used at WMF

Not used at WMF.

Dependencies

preact v10
preact-router v3
history v4
banana i18n v1

See package-lock.json in source repo for up to date dependencies.

Has this project been reviewed before?

Not reviewed before

Working test environment

Once cloned locally, npm install and npm run dev will allow running the app in a browser. Set your browser resolution to 240x290 to simulate a KaiOS phone.

Post-deployment

Inuka-Team / @SBisson

Related Objects

StatusSubtypeAssignedTask
Resolvedsbassett
DeclinedNone

Event Timeline

SBisson created this task.Dec 16 2019, 4:39 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 16 2019, 4:40 PM
SBisson moved this task from Backlog to Watching on the Inuka-Team board.Dec 16 2019, 4:40 PM
sbassett added a subscriber: sbassett.EditedDec 18 2019, 6:47 PM

Hey @SBisson -

The Security-Team would like to get at least a due-diligence review scheduled for this as soon as we can, though we had a few questions and concerns here (which are also most likely relevant for T240010), some directly-related to this review, some a bit tangential:

  1. Per our security review policy, we typically require at least 30 days of notice to schedule and turn around a review, which is technically met by this request. However given that a significant portion of the code has already been released (in a sense) at github, the upcoming winter holidays and our in-progress/backlog review requests, it may be a stretch for us to complete this request prior to the desired deployment date.
  2. Is there a reason the Inuka-Team seemingly chose to host this repo exclusively at github as opposed to gerrit mirroring to github? I'm not aware of any formal policy forbidding this, per se, but there are some concerns here around differences in process. As well as concerns about potential tests and security checks running in CI under gerrit which would be different or non-existent at github.
  3. While a lot of Wikimedia code is worked on publicly in gerrit prior to a security review, this code and T240010 have a slightly different context in that they are applications/libraries intended for third party (AIUI) use and have already been "released" simply by existing at github. As opposed to some MW core or extension code that was worked upon publicly but has yet to be deployed to Wikimedia production and would potentially be blocked by a bad security review. I'm not sure there's a good solution here (maybe private or restricted repos, but that can get annoying and flies a bit in the face of OSS) and the subtlety might not be great enough to demand a solution, but I wanted to mention this and see if the Inuka-Team has thought about this aspect when it comes to releasing code.
  4. Similar to what the WMF has done in the past for the existing iOS and Android Wikipedia mobile applications, it's probably a good idea to schedule a third party security assessment of this code. This would 1) get more eyes on the code from a security perspective and 2) likely provide for a higher quality review, given the Security-Team's dearth of KaiOS code review requests (even if it appears to be fairly standard NodeJS code). Of course, such third party assessments are not free, and I'm not sure what the Inuka-Team's budget might allow, but the Security-Team would strongly recommend this course of action here.

Hey @SBisson -

The Security-Team would like to get at least a due-diligence review scheduled for this as soon as we can, though we had a few questions and concerns here (which are also most likely relevant for T240010), some directly-related to this review, some a bit tangential:

  1. Per our security review policy, we typically require at least 30 days of notice to schedule and turn around a review, which is technically met by this request. However given that a significant portion of the code has already been released (in a sense) at github, the upcoming winter holidays and our in-progress/backlog review requests, it may be a stretch for us to complete this request prior to the desired deployment date.

We understand that everyone is very busy. Jan 22 is our internal date to be feature complete for the first release, we don't know how long the release process will take since this will be the first time we go through it. And we'll of course wait for you team's green light to go ahead.

  1. Is there a reason the Inuka-Team seemingly chose to host this repo exclusively at github as opposed to gerrit mirroring to github? I'm not aware of any formal policy forbidding this, per se, but there are some concerns here around differences in process. As well as concerns about potential tests and security checks running in CI under gerrit which would be different or non-existent at github.

Like the other apps, we host on github for CI reasons. zuul/jenkins is a good fit for MediaWiki but is unnecessary overhead here. Our CI is free, fast, and took minutes to setup. I'm not aware of any javascript tests or security checks that run in jenkins but if there are, we'll make sure to run those as well. And we can always migrate to gerrit without too much effort if it makes sense.

  1. While a lot of Wikimedia code is worked on publicly in gerrit prior to a security review, this code and T240010 have a slightly different context in that they are applications/libraries intended for third party (AIUI) use and have already been "released" simply by existing at github. As opposed to some MW core or extension code that was worked upon publicly but has yet to be deployed to Wikimedia production and would potentially be blocked by a bad security review. I'm not sure there's a good solution here (maybe private or restricted repos, but that can get annoying and flies a bit in the face of OSS) and the subtlety might not be great enough to demand a solution, but I wanted to mention this and see if the Inuka-Team has thought about this aspect when it comes to releasing code.

I hadn't thought about it since all code is indeed developed publicly. Nothing stops a 3rd party MediaWiki user from running the master branch other than the knowledge that they should stick with official releases. I see that being similar to the fact that our app hasn't been released to any store and a user really has to go out of their way to install it on a device. Do you think a big "work in progress - do not use" banner on the repo would help?

  1. Similar to what the WMF has done in the past for the existing iOS and Android Wikipedia mobile applications, it's probably a good idea to schedule a third party security assessment of this code. This would 1) get more eyes on the code from a security perspective and 2) likely provide for a higher quality review, given the Security-Team's dearth of KaiOS code review requests (even if it appears to be fairly standard NodeJS code). Of course, such third party assessments are not free, and I'm not sure what the Inuka-Team's budget might allow, but the Security-Team would strongly recommend this course of action here.

I'll bring this up to the team. Are you able to share some details from past experience (timeline, provider, cost, etc) by email?

Thanks!

sbassett claimed this task.Jan 9 2020, 4:48 PM
sbassett triaged this task as High priority.
sbassett moved this task from Waiting to In Progress on the Security Readiness Reviews board.
sbassett set the point value for this task to 15.
sbassett reassigned this task from sbassett to Reedy.Jan 14 2020, 6:12 PM
sbassett added a subscriber: Reedy.

Reassigning to @Reedy - if you'd like to pair up on this and explore some automated approaches for the review, let me know!

chasemp mentioned this in Unknown Object (Task).Jan 16 2020, 6:59 PM
chasemp mentioned this in Unknown Object (Task).Jan 16 2020, 7:15 PM

Deployment date changed due to partnership dependencies. We are now planning a couple of weeks of dev work to get the app launched.

Please let us know what it left to execute to be launch ready.

Jcross added a subscriber: Jcross.Feb 11 2020, 6:14 PM

Hi @AMuigai - do you have a specific stop date / date we could begin review yet? Our process is frozen so long as commits are frequently being made. Alternatively, if you could choose a specific commit we could begin reviewing at that point with the understanding that we would *only* be reviewing to that point and further commits would not be included in our assessment.

AMuigai added a subscriber: Arrbee.Feb 24 2020, 7:22 PM

Hi @Jcross. Sorry for the delay in responding back to you. Our plan is to share the app with your team on March 12th.

@AMuigai @Jcross - just to note: an external vendor pen-test and source code audit of the current Wikipedia KaiOS codebase is underway. It's still in the WMF procurement/legal approval process, so we could delay the start date until March 12th if that makes the most sense. Not sure if the March 12th date is due to ongoing development (in which case, we probably would want to delay the external vendor a bit) or not.

@sbassett Good to know that you found a vendor. Yes, there is ongoing development, hence the March 12 date.

@AMuigai - Ok, thanks for the update. We're still in the procurement and legal review phase with our vendor, which I'm hopeful will conclude by the end of this week or early next week. After that, we can request that the vendor delay the start of their review until March 12th. I don't think we'd want to push back on them much further though, so hopefully the codebase is stable from a development standpoint by that date.

@AMuigai - We've confirmed with the vendor that the earliest they'd be able to begin this review would be March 16, 2020. So we should be good with your team's March 12, 2020 completion date.

Assuming this task is about the KaiOS-Wikipedia-app project, hence adding that project tag so people who don't know or don't care about random WMF team tags can also find this task when searching for projects which they are interested in.

chasemp moved this task from Incoming to In Progress on the secscrum board.Mar 10 2020, 8:12 PM

Updating here (in case it was needed) that the team completed the feature set for security review by the 12th as earlier communicated. If the security review was waiting on an update from us, please go ahead and begin.

AMuigai updated the task description. (Show Details)Mar 16 2020, 10:01 AM

Updating here (in case it was needed) that the team completed the feature set for security review by the 12th as earlier communicated. If the security review was waiting on an update from us, please go ahead and begin.

For explicitness then, dd82962 will be used for the security review. It's three commits since the last commit on the 12th and only minimal changes

@Reedy Thanks for making this explicit. dd82962 is indeed a good place to start.

We are not doing a "code freeze" but we expect all the code to be merged between the commit above and launch date to be very low risk. We can organize a walk-through of those commits to validate that assumption.

SBisson moved this task from Backlog to MVP on the KaiOS-Wikipedia-app board.Mar 23 2020, 3:34 PM
SBisson edited projects, added KaiOS-Wikipedia-app (MVP); removed KaiOS-Wikipedia-app.
Reedy added a comment.Mar 31 2020, 3:52 PM

Some feedback

The testing is completed here and no major issues have been discovered. So, kudos to the development team there 🙂 Also, the application has a very narrow attack surface too which goes in favor.

:)

We should have the report to pass on soon

Thanks for the update @Reedy! Looking forward to seeing the report.

@AMuigai @SBisson -

Update: we have received the final security report from our vendor. They did not find very much, which is great. I'd like to distill the results of the vendor report here, along with the Security-Team's findings and then resolve this task soon.

sbassett claimed this task.Apr 21 2020, 2:53 AM
sbassett lowered the priority of this task from High to Medium.
sbassett added a project: user-sbassett.
sbassett moved this task from Backlog to In Progress on the user-sbassett board.

@sbassett I've filed follow-up tasks for the minor findings in the report.

We'll try to address the npm audit issues soon but since those are all related to dev dependencies, it's hard to see how that can be a real risk for the app in production.

Let us know if anything else needs to happen before this task can be closed.

@SBisson - Sounds good, thanks. I'm going to update T253141 with some more specifics from internal tests (assuming there's worthwhile information to add). And I'll look into WKM-006 a bit more and get a response to you on that.

sbassett moved this task from In Progress to Waiting on the secscrum board.Jun 1 2020, 9:39 PM
sbassett moved this task from In Progress to Waiting on the user-sbassett board.Jun 8 2020, 9:26 PM
SBisson changed the status of subtask Restricted Task from Open to Stalled.Jun 9 2020, 7:39 PM
sbassett closed this task as Resolved.Jun 11 2020, 4:51 PM
sbassett moved this task from Waiting to Our Part Is Done on the secscrum board.
sbassett moved this task from Waiting to Done on the user-sbassett board.
SBisson changed the status of subtask Restricted Task from Stalled to Open.Sep 10 2020, 3:34 PM
AMuigai closed subtask Restricted Task as Resolved.Oct 8 2020, 1:49 PM