I have had back and forth with Trail of Bits regarding the V1 audit scope and timing, here’s a rundown:
Start Date: Sept 30th Total Effort: 9 person-weeks Dedicated Personal: 3 senior resources Total Timeline: 3 weeks (plus however long it takes us to fix potential issues) Broken Out Details:
Security review of the Customer source code through a combination of manual and automated review. The review will include, but is not limited to the following activities, with a rough estimate of their requisite level of effort:
Best-effort guidance after the project. After the project concludes, Trail of Bits will make its best efforts to address security questions that arise via email.
If there is something you feel is missing/needs changing/needs removed from this, now is the time to bring it up as we have not signed anything yet.
I’m curious to know how we’ll manage branches for the audit and the development that happens in parallel. Will we create a separate instance of develop for audit on Sept. 30, and then continue building on it as usual?
We have a kick-off call Monday, September 30th at 3 PM EDT with Trail of Bits to start the audit. The scope is the same as the OP, here are the engineer details:
Sefan Edwards and Sam Moelius will begin the review going 9/30 - 10/11
Josselin, Sam, and Dominik will then complete the review from 10/15 - 11/1
Each week will have a summary report which will allow us to start working on found issues during the audit. I hope to set up a channel in the new discord with all auditors and relevant Status CCs for quick communication on issues found.
I’m working on their plans w.r.t. @rachel’s question about branch management. Will update with details there when I get them.
So for comms during the Audit, ToB would like to create a specific channel in their slack and invite members to it. There reasoning is a-few-fold:
they can bring in resources of experience easily when needed, as everyone in the company uses that slack as their main comms
they want to maintain chat history of the review
they asked first
The question now is who would like to be included into this? Your goals would be to keep up with the discussion and real-time findings, ask appropriate questions as well as answer any clarification needs they may have when doing things.
I know we all LOVE having extra chat clients open and watching them, but this is a short term thing, and quite important.
Might be overstepping my bounds here, but this is how I’ve always done branches and it’s always worked well:
master is a branch from where you can always run the latest and greatest. It should also always be green. On the case of GitHub it needs to be a protected branch.
branches and WIP PRs are where the magic happens. I like to push early and often and open PRs as soon as possible to start a discussion around what may well be wrong implementation.
tags should be used judiciously and should be immutable. Anything that requires a specific version on any point in time can work off of a tag, knowing it will always be the same.
develop branches are mostly an anti-pattern, but I could be convinced otherwise.
In the past audits with ToB, they cloned a repo to a private repo within the Trail of Bits · GitHub org to keep discussion private until we publicize the finalized report. The only thing here would be to decide which exact branches they do this with, or if it’s even a reasonable thing to do.
Tags are a snapshot at a specific point in time, so they’re perfect for this use case. “We need you to help us test v1.0.5” means they can check out the v1.0.5 tag and know that will never change.
This way they can use our repository, and we can create a separate private repo under the status-im organization where only issues are created to track security concerns (and, as a result, we control access so we can invite whomever we find pertinent.)
Edit: I should say that tags should be used as a snapshot as a specific point in time. As long as we treat tags as immutable, we’re good.
For now, the team is grabbing the latest HEAD and will get setup from that. Do you have strong opinions or ideas on which commit/tag they should base their security tests from?
For status-protocol-go HEAD is fine, some parts of the codebase are not used by v1 by status-react, only by status-console-client (different client), so those strictly speaking don’t need to be audited, I can provide more details if necessary.
To make sure that we test exactly what we’re shipping, I agree it’s best to work backwards from status-react in terms of SHAs. As such, the following SHAs were picked:
Not sure I understand this, but the commit above is not even merged in the develop branch nor is ready, it turns out there’s more work to do as we currently store the master key and that needs changing. (This might require a change in status-go as well as we need to store the address of the key 1581).
That’s my bad. I thought, as it had been approved, that it was good to go.
As mentioned in channel, storing the new key path has security implications that need to be considered in the audit, so we 100% have to include this functionality.
Not trying to put pressure, but do you have an estimate on when we can give Trail of Bits a commit hash to work from?
The PR is ready now https://github.com/status-im/status-react/pull/9096 , I need some input from someone from the keycard team, as I don’t own one myself and can’t test the functionality with the keycard (as far as I can see, it should still work, but without testing it I can’t be sure), no change in status-go was necessary.
The commit hash can only be used though once merged in develop, as there might be some extra changes + we rebase against it, so it’s likely to change.
It sounds like there’s another dependency implication for the audit. There’s a PR which updates status-im/status-go to use Geth 1.9.5 that needs merging, and I’m trying to understand what the testing/time implications of this are.
