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.

In T240869#5752090, @sbassett wrote:

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!

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.

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.

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.

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.

In T240869#5971461, @AMuigai wrote:

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.

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 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.