Security/Reviews/BrowserIDCAPI/Code Review

From MozillaWiki
Jump to: navigation, search
Note: The C code review has been completed and the issues resolved

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

  1. git clone https://github.com/ozten/sasl-browserid.git
  2. cd sasl-browserid
  3. vagrant up
    • this may take a while, the VM has to be copied and configured
  4. vagrant ssh

You should be SSHed into the VM now

VM configuration

  1. mkdir ~/openldap_db
  2. cd ~/sasl-browserid/test
  3. 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

  1. Edit the file ~/sasl-browserid/configs/slapd.conf
  2. Change browserid_endpoint to
  3. Save file and retry the tests
    • The file will be copied to /usr/lib/sasl2/slapd.conf on next test run

Updating plugin

  1. 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
  2. Connect to the VM if you haven't
  3. cd ~/sasl-browserid
  4. make
  5. sudo make install
  6. 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.