Security/Reviews/BrowserIDCAPI/Code Review
From MozillaWiki
< Security | Reviews | BrowserIDCAPI
Note: The C code review has been completed and the issues resolved
Contents
Background
- Authentication Workflow
- Security review bug 684085
- Implementing the SASL based authentication workflow will allow users to log into certain LDAP protected sites by using a browserid assertion rather than a username / password.
- Websites initially slated for this functionality are mozillians.org and the intranet phonebook.
- Browserid integration requires the the frontend webapp to be modified to support SASL. This is currently possible with django sites.
Testing
Scope
- Plugiun code in sasl-browserid/plugins was reviewed
- The server and client plugin code are in browserid.c
- Server functions begin with browserid_server*
- Client functions begin with browserid_client*
- Files reviewed
- browserid.c
- browserid_init.c
- session.c
- session.h
- verifier.c
- verifier.h
- plugin_common.[ch] were not reviewed due to time constraints
- The files are part of the CMU SASL implementation
- Security of the browserid protocol was not covered in this review
- The plugin was tested directly without a django app in front
Methodology
- The codebase was small enough that a manual code review was performed
- The code was reviewed for common C security issues such as buffer overruns, arithmetic overflows, off-by-one errors, use-after-free
- Proof of concept testcases were not created for these issues.
- Issues were filled in bugzilla and triaged
- No specific security tests were created for the plugin. There exists some tests that exercise cases such as large/NULL inputs
Setup VM
- git clone https://github.com/ozten/sasl-browserid.git
- cd sasl-browserid
- vagrant up
- this may take a while, the VM has to be copied and configured
- vagrant ssh
You should be SSHed into the VM now
VM configuration
- mkdir ~/openldap_db
- cd ~/sasl-browserid/test
- cp config.py-dist config.py
- folder/file configurations are located in sasl-browserid/test/config.py
You should be able to run tests at this point. If you encounter an error similar to ldap_sasl_interactive_bind_s: Internal (implementation specific) error (80) additional info: SASL(-1): generic failure:
then you may have to update the browserid endpoint
- Edit the file ~/sasl-browserid/configs/slapd.conf
- Change browserid_endpoint to
- browserid_endpoint: http://localhost/success.json
- Save file and retry the tests
- The file will be copied to /usr/lib/sasl2/slapd.conf on next test run
Updating plugin
- git pull
- This can be done from the host or VM, though you will likely need to compile on the VM
- sasl-browserid is set up as a shared host-VM folder
- Connect to the VM if you haven't
- cd ~/sasl-browserid
- make
- sudo make install
- sudo /etc/init.d/slapd restart
Running tests
- Connect to the VM
- cd sasl-browserid/test
- python unit_privileged_test_suite.py
- sudo python functional_test.py
- The above test needs sudo due to some file copying / service configuration
Findings
- Issues are attached as blockers to bug 684085
- There were some potential buffer overruns, arithmetic overflows and error handling cases in the code.