add high-level checklist of the process
s/yourself/the security team/
|Deletions are marked like this.||Additions are marked like this.|
|Line 46:||Line 46:|
|* unassign yourself||* unassign the security team from the bug|
The SecurityTeam is sometimes asked to perform source code auditing, typically during the MainInclusionProcess. Due to time constraints, only a high-level audit is performed for Main Inclusion Reports (MIRs).
While every effort is made to perform the audit within a timely fashion, please remember fixing vulnerabilities and proactive security development are the primary focus of the SecurityTeam.
When a source package needs an audit from the SecurityTeam, the bug is assigned to ubuntu-security with a comment asking for the package to be reviewed.
A member of the SecurityTeam will change the bug status to 'In Progress'. In addition, they will also create an appropriate card in the private Security Team trello board (if one does not already exist) and ensure members from the team that requested the MIR are also subscribed to the card, move this to the Work In Progress queue and comment on the card that they will be handling it.
When completed, the SecurityTeam member will change the bug back to 'Confirmed', unassign ubuntu-security, and add a comment as to the results of the audit. An appropriate comment should also be made on the trello card and the card should be moved to the DONE queue.
If the bug requires more information, the SecurityTeam member should mark the bug status as 'Incomplete', without changing who the bug is assigned to.
Doing the MIR
Once: clone https://launchpad.net/ubuntu-security-tools
Once: set up SecurityTeam/BuildEnvironment
Once: set up the Canonical VPN (if you're going to use Coverity results)
- Once: get Coverity credentials
- Once: install Coverity tools locally
select a package from the Trello lane
- read the MIR bug for the package in question to get context
- if the package still makes sense to audit, add yourself to the Trello card and move it to In Progress
uaudit <package name> -- finish reading email
inspect the files in the audits directory
use vim -q (or equivalent) on the language-specific files in audits; read much of the code
- post Coverity results to the bug, upstream bug trackers
- inspect Coverity results
- determine severity of issues discovered while reviewing code
- contact upstream developers with issues, privately if justified
- decide if the code appears to be written with discipline
- decide if the code appears to be testable
- decide if the code appears to represent sufficient value for users versus maintenance costs
- identify any changes that might reduce risks to more palatable levels
- write up a report using the template to communicate the status of the package
- post the report to the bug
- subscribe yourself to the bug
- unassign the security team from the bug
- move the Trello card from In Progress to DONE
Most of the process is driven by the uaudit script from https://launchpad.net/ubuntu-security-tools
$UST/audits/uaudit expects a lot of tools to be in your PATH:
exes = ['umt', 'audit-binaries.sh', 'audit-code.sh', 'audit-docs.sh', 'audit-log.sh', 'audit-packaging.sh', 'hardening-check', 'check-source-package', 'debuild', 'diffstat', 'fakeroot', 'licensecheck', 'lintian', 'suspicious-source', 'ctags', 'cppcheck', 'shellcheck', 'shellcheck.sh', ]
Many of these are in our repos, I set up symlinks in my ~/bin/ directory for the ones not already on my PATH.
uaudit will use umt to download the sources, rebuild the package, and run the different tools on the results.
If the Coverity analysis tools are available in PATH uaudit will also try and use these to perform additional analysis (this also requires the associated license.dat).
While it's building, it's a good time to change the bug assignee from the team to myself.
I start in ~/ubuntu/security/audits/ and run uaudit <source package name> and eventually there will be:
sarnold@hunt:~/ubuntu/security/audits/libfprint/cosmic$ ls -l total 588 drwxrwxr-x 2 sarnold sarnold 4096 Jul 5 18:32 audits drwxrwxr-x 4 sarnold sarnold 4096 Jun 27 18:15 binary drwxrwxr-x 8 sarnold sarnold 4096 Jun 15 20:29 libfprint-0.7.0 -rw-r--r-- 1 sarnold sarnold 7944 Jun 15 20:22 libfprint_0.7.0-1.debian.tar.xz -rw-r--r-- 1 sarnold sarnold 1239 Jun 15 20:22 libfprint_0.7.0-1.dsc -rw-r--r-- 1 sarnold sarnold 1728 Jun 15 20:22 libfprint_0.7.0-1_source.build -rw-r--r-- 1 sarnold sarnold 6289 Jun 15 20:22 libfprint_0.7.0-1_source.buildinfo -rw-r--r-- 1 sarnold sarnold 2396 Jun 15 20:22 libfprint_0.7.0-1_source.changes -rw-r--r-- 1 sarnold sarnold 550484 May 17 2017 libfprint_0.7.0.orig.tar.xz drwxrwxr-x 2 sarnold sarnold 4096 Jun 15 20:26 source
Almost all the work takes place in audits/ and libfprint-0.7.0/.
audits/REVIEW.txt contains the general 'plan' of audit. Focus on the top half, leave everything below https://wiki.ubuntu.com/MIRTeam for the rest of the MirTeam.
sarnold@hunt:~/ubuntu/security/audits/libfprint/cosmic/audits$ ls -l total 1684 -rw------- 1 sarnold sarnold 995 Jun 15 20:24 binaries.txt -rw-rw-r-- 1 sarnold sarnold 6004 Jun 27 19:48 bug.txt -rw------- 1 sarnold sarnold 79565 Jun 15 20:24 code-c-noextfilter.txt -rw------- 1 sarnold sarnold 50475 Jun 15 20:24 code-c.txt -rw------- 1 sarnold sarnold 52968 Jun 15 20:24 code-java-noextfilter.txt -rw------- 1 sarnold sarnold 462 Jun 15 20:24 code-java.txt -rw------- 1 sarnold sarnold 317066 Jun 15 20:24 code-perl-noextfilter.txt -rw------- 1 sarnold sarnold 436 Jun 15 20:24 code-perl.txt -rw------- 1 sarnold sarnold 239290 Jun 15 20:24 code-php-noextfilter.txt -rw------- 1 sarnold sarnold 459 Jun 15 20:24 code-php.txt -rw------- 1 sarnold sarnold 54866 Jun 15 20:24 code-python-noextfilter.txt -rw------- 1 sarnold sarnold 468 Jun 15 20:24 code-python.txt -rw------- 1 sarnold sarnold xxxxx Jun 15 20:26 coverity.txt -rw------- 1 sarnold sarnold 1119 Jun 15 20:24 cppcheck.txt -rw------- 1 sarnold sarnold 9208 Jun 15 20:26 csp-source.txt -rw------- 1 sarnold sarnold 278 Jun 15 20:25 docs.txt -rw-rw-r-- 1 sarnold sarnold xxxxx Jun 15 xx:xx libfprintd-xxxx.build -rw-rw-r-- 1 sarnold sarnold xxx Jun 15 xx:xx log.txt -rw------- 1 sarnold sarnold 10323 Jun 15 20:26 packaging.txt -rw-rw-r-- 1 sarnold sarnold 8507 Jun 15 20:22 REVIEW.txt -rw------- 1 sarnold sarnold 325486 Jun 15 20:25 shellcheck.txt -rw-rw-r-- 1 sarnold sarnold 481474 Jun 15 20:26 tags
I write my notes as I go in bug.txt, with the intention of pasting the whole thing into the bugreport once I'm done. The binaries.txt shows the hardening-check output, and we mostly expect *all* hardening to be applied to everything these days. code-* files are generated by language-specific grep patterns. cppcheck.txt is from cppcheck output, shellcheck.txt is from shellcheck, tags is from exuberant-ctags, csp-source.txt outputs a bunch of Debian packaging information (usually most useful for showing the lintian errors), docs.txt shows READMEs and similar in the tree, and packaging.txt includes some of the output of the commands listed in the REVIEW.txt file on the binary packages (found in the binary/ directory earlier).
When the Coverity analysis tools are available, an additional coverity.txt file is generated which lists the Coverity defects identified during the build.
The full build log is in a *.build file named after the source package. The log.txt is the output grepped for errors and warnings.
I skim docs, skim csp-source, skim the build logs, read binaries, and then use the language-specific files for most everything else.
vim's quickfix support is *fantastic* for this.
vim -q ../audits/code-c.txt vim -q ../audits/cppcheck.txt
Then you can use ^N and ^P to jump through the list of "errors" and read the surrounding code. (I also like to use :set so=99 in vim, to keep the cursor in the center of the terminal when it can.)
For cppcheck output I have this in my ~/.vimrc:
set errorformat+=[%f:%l]\ ->\ %m,[%f:%l]:%m
If you use emacs, file-local-variables are set in each appropriate .txt file to allow quick navigation to the associated source code via M-g M-n and M-g M-p for each next / previous error identified.
Around each line, look for error handling, input hygiene, etc.
There's a wide variety of how much time you can spend on this. For codebases that "feel" good I might actually look at each line for one or two seconds at most and look for "did they handle errors". Sometimes I'll spend a *long* time tracking up and down call sites to see if inputs come from untrusted sources or not, if they get filtered or sanitized or something similar elsewhere, etc.
ctags and cscope are really helpful for C and C++. ctags is nice for others.
Sometimes the code-*.txt files will be thousands of entries. You can't read them all. I tend to read the first fifty or hundred of each "type" of report and then start skipping.
If there's an extensive test suite that hits these greps, I'll usually grep -v the tests from the code-*.txt files I'm working with, to make it easier to focus on the "real" code. Test suites are sloppy.
Hopefully as you read you'll get a good feeling for the overall design of a project, whether or not the authors were writing with discipline or just fast and loose, etc.
Some higher-level things that are hard to spot in a snapshot of source code is overall project velocity. DPDK, nodejs, etc., move SO FAST that it's really difficult to backport fixes even six months. Bash, bc, awk, etc. move so slow that fixes can be backported decades without trouble. Sometimes it's difficult to guess the benefit / cost ratio.
In one MIR, I hated the *idea* of the package and didn't review it. I talked about it with the stakeholders and decided that we didn't need to support it. In another, I suggested replacement packages, but none met the needs of the requesting team, so we carried on with the audit. Ubuntu is an opinionated distribution: if something doesn't feel right, we can work with everyone else to find the right path forward.
Once I'm done reading the sources I usually have a good feeling if the package belongs in the distribution or not. I'll go through the REVIEW.txt and answer questions, and make notes of oddities found in the packaging that need to be fixed first.
We can make requests for changes. Sometimes I'll feel strongly enough to require the changes and without the ACK completely until it's done, sometimes it's enough to just make the request and follow up later. (I can't recall any specific cases of problems.)
Sometimes we'll ask the requesting team to put in writing that they'll help us prepare fixes or test new packages etc.
I like to be clear with "security team ACK for promoting ... to main" or "security team NAK for promoting ... to main". Not all bug readers know the whole process, so I like to be clear that I'm only speaking for the security team, not the final result.
Once I'm done, I unassign the bug, subscribe to all emails about the bug, and paste the bug.txt contents into the bug report.
Feel free to ask Seth Arnold (sarnold) any questions.