[tracker] Security checklist #286

Closed
jvehent opened this Issue Oct 4, 2016 · 4 comments

Comments

Projects
None yet
4 participants
@jvehent
Contributor

jvehent commented Oct 4, 2016

Risk Management

  • The service must have performed a Rapid Risk Assessment and have a Risk Record bug (SVC-RRA).

Infrastructure rules

  • Use Intermediate TLS (INFRA-TLS)
  • Set HSTS to 31536000 (1 year) (INFRA-HSTS)
  • Set HPKP to 5184000 (60 days) (INFRA-HPKP)
    • Public-Key-Pins: max-age=5184000; pin-sha256="WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18="; pin-sha256="r/mIkG3eEpVdm+u/ko/cwxzOMo1bk4TyHIlByibiA5E="; pin-sha256="YLh1dUR9y6Kja30RrAn7JKnbQG/uEtLMkBgFF2Fuihg="; pin-sha256="sRHdihwgkaib1P1gxX8HFszlD+7/gTfNvuAybgLPNis=";
      • Start with max-age set to 5 minutes and increase gradually
      • Pin to the EV and DV roots of Digicert
    • If the service is not hosted under services.mozilla.com, it must be manually added to Firefox's preloaded pins.
  • If service has an admin panels, it must:
    • only be available behind Mozilla VPN (which provides MFA) (INFRA-ADMINVPN)
    • require LDAP authentication (INFRA-ADMINLDAP)
    • enforce a two-man rule on sensitive changes (INFRA-2MANRULE)

Coding rules

The following rules apply to all web applications: api and websites.

  • Sign all commits (APP-COMMITSIG)
  • Detailed logging in mozlog format (APP-MOZLOG)
    • Business logic must be logged with app specific codes (errno)
    • Access control failures must be logged at WARN level
  • All SQL queries must be parameterized, not concatenated (APP-SQL)
  • User data must be escaped for the right context prior to reflecting it (APP-ESCAPE)
  • Apply sensible limits to user inputs, see input validation (APP-INPUTVAL)
  • Enforce Access Controls server-side (APP-ACL)
  • Set the Secure flag on Cookies, and use sensible Expiration and HTTPOnly (APP-SECCOOKIE)
  • Keep 3rd-party libraries up to date (APP-DEPS)
  • If handling cryptographic keys, must have a mechanism to handle monthly key rotations (APP-KEYROT)
    • All keys must be rotated quarterly.
    • Keys used to sign sessions don't need a rotation mechanism if destroying all sessions is acceptable during.

Additional websites requirements

The following coding rules only apply to websites, not web apis.

  • Never store passwords, use Firefox Accounts (APP-IDP)
  • Forbid Mixed content, always use HTTPS (APP-MIXCONTENT)
  • Must have a CSP with (APP-CSP)
    • a report-uri pointing to the service's own /__cspreport__ endpoint
    • if default-src is not self, child-src should be none or only allow specific origins
    • no use of unsafe-inline or unsafe-eval
  • Must have CSRF tokens and manually excluded specific forms (APP-CSRF)
  • Should consider having checksums for 3rd-party content via SRI (APP-SRI).
    • Trusted 3rd parties, like Google Analytics, don't need SRI. Use your best judgment to decide if a 3rd party script is trustworthy (and assume it is not).
  • Consider Security headers as appropriate (APP-HEADERS)
    • X-Content-Type-Options
    • X-Frame-Options
    • X-XSS-Protection

Data rules

  • When storing sensitive user data (like browsing history) on Mozilla servers:
    • Anonymize it (similar to Tiles) (DATA-ANON)
    • Encrypt it client-side (similar to Sync) (DATA-CRYPT)
    • If user data must be stored non-anonymized and in clear text, you must talk to the security and legal teams about it.
  • If the service pushes data to Firefox, like when distributing blacklists or pushing updates, cryptographic signatures must be used. (DATA-SIGN)
    • Addons must use standard AMO signing (APP-SIGNING)
    • Code & Conf must use Content-Signature via Autograph (DATA-SIGNING)
@rugk

This comment has been minimized.

Show comment
Hide comment
@rugk

rugk Oct 6, 2016

If user data must be stored non-anonymized and in clear text,

So is this actually the case?

BTW: Is this already deployed or still work in progress?

rugk commented Oct 6, 2016

If user data must be stored non-anonymized and in clear text,

So is this actually the case?

BTW: Is this already deployed or still work in progress?

@mythmon

This comment has been minimized.

Show comment
Hide comment
@mythmon

mythmon Oct 6, 2016

Member

We don't currently store any user data. I was told that items that don't apply should be checked off.

Normandy is currently deployed at (among other places) https://self-repair.mozilla.org and https://normandy-admin.prod.mozaws.net (VPN required).

Member

mythmon commented Oct 6, 2016

We don't currently store any user data. I was told that items that don't apply should be checked off.

Normandy is currently deployed at (among other places) https://self-repair.mozilla.org and https://normandy-admin.prod.mozaws.net (VPN required).

@Osmose Osmose added priority: P3 and removed priority: P3 labels Nov 1, 2016

@Osmose Osmose removed their assignment Nov 1, 2016

@Osmose Osmose self-assigned this Feb 8, 2017

@Osmose Osmose added priority: P1 and removed priority: P2 labels Feb 8, 2017

@Osmose Osmose added this to the Sprint 5 - February 6th - February 18th, 2017 milestone Feb 8, 2017

Osmose added a commit to Osmose/normandy that referenced this issue Feb 8, 2017

mythmon added a commit that referenced this issue Feb 8, 2017

Osmose added a commit to Osmose/normandy that referenced this issue Feb 13, 2017

Osmose added a commit to Osmose/normandy that referenced this issue Feb 14, 2017

Osmose added a commit to Osmose/normandy that referenced this issue Feb 14, 2017

Osmose added a commit to Osmose/normandy that referenced this issue Feb 14, 2017

#286: Add CSP via django-csp.
Because CSP prevents the use of eval, we cannot use the eval-based
sourcemap support in Webpack. The other alternative, inline-source-map,
seems to trigger errors in the Django development server due to URLs
that are too long, so the only remaining alternative is to use
external source maps, which seem to work fine.

Osmose added a commit to Osmose/normandy that referenced this issue Feb 15, 2017

Osmose added a commit to Osmose/normandy that referenced this issue Feb 15, 2017

#286: Add CSP via django-csp.
Because CSP prevents the use of eval, we cannot use the eval-based
sourcemap support in Webpack. The other alternative, inline-source-map,
seems to trigger errors in the Django development server due to URLs
that are too long, so the only remaining alternative is to use
external source maps, which seem to work fine.

Osmose added a commit to Osmose/normandy that referenced this issue Feb 16, 2017

Osmose added a commit to Osmose/normandy that referenced this issue Feb 16, 2017

@Osmose

This comment has been minimized.

Show comment
Hide comment
@Osmose

Osmose Feb 21, 2017

Member

All the items in this checklist are in-progress or not relevant. There's two items from the list that have separate issues filed:

  • LDAP Auth: #526
  • Two-man rule for sensitive changes: #287

Given that Github is awful for representing the relationship between issues, I think we can call this issue closed and track the remaining work in those two issues.

Member

Osmose commented Feb 21, 2017

All the items in this checklist are in-progress or not relevant. There's two items from the list that have separate issues filed:

  • LDAP Auth: #526
  • Two-man rule for sensitive changes: #287

Given that Github is awful for representing the relationship between issues, I think we can call this issue closed and track the remaining work in those two issues.

@Osmose Osmose closed this Feb 21, 2017

@jvehent

This comment has been minimized.

Show comment
Hide comment
@jvehent

jvehent Feb 21, 2017

Contributor

Awesome! Thanks a lot for going through it.

Contributor

jvehent commented Feb 21, 2017

Awesome! Thanks a lot for going through it.

Osmose added a commit that referenced this issue Feb 21, 2017

Osmose added a commit to Osmose/normandy that referenced this issue Feb 23, 2017

Osmose added a commit to Osmose/normandy that referenced this issue Mar 1, 2017

Osmose added a commit that referenced this issue Mar 1, 2017

Merge pull request #511 from Osmose/subresource-integrity
#286: Use subresource integrity for static assets.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment