MozillaWiki

WebAppSec/Secure Coding Guidelines: Difference between revisions

Page Discussion

WebAppSec/Secure Coding Guidelines (view source)

Revision as of 12:57, 20 April 2012

5,484 bytes added ,  20 April 2012
→‎Session Management
 
(48 intermediate revisions by 9 users not shown)
Line 20: Line 20:
==Authentication==
==Authentication==
'''Attacks of Concern'''
'''Attacks of Concern'''
* brute force password guessing
* online & offline brute force password guessing
* user enumeration  
* user enumeration  
* mass account lockout
* mass account lockout (Account DoS)
* time trade-off hash cracking
* offline hash cracking (time trade-off)
* lost passwords
* lost passwords


===Password Complexity===
===Password Complexity===
====Critical Sites ====
All sites should have the following base password policy:
Examples: any admin/power user login or for all users on a website that has sensitive user data


Implement the following policy:
* Passwords must be 8 characters or greater
* Passwords must be 8 characters or greater
* Passwords must require numbers and letters and a special character
* Passwords must require letters and numbers
* Blacklisted passwords should be implemented (contact infrasec for the list)


==== Non-Critical Sites ====
====Critical Sites ====
Examples: No personnel user data stored, minimal impact of compromised account
Examples: addons.mozilla.org, bugzilla.mozilla.org, or other critical
 
sites.
Note:
* account lockout must still be in place for 3 failed attempts
* This policy is only for basic accounts. Any admin accounts must still adhere to the strict password policy identified above.
 
Implement the following policy:
* Passwords must be 6 characters or greater
* Password must require any two of the following three categories of characters:
** numbers
** letters
** special character
* Disallow the following passwords (case insensitive):
** password1 (or any number at the end)
** abc123
** trustno1
** ncc1701
** rush2112
** thx1138
** Note: This list was created by identifying passwords from the [http://www.whatsmypass.com/the-top-500-worst-passwords-of-all-time top 100 most common passwords] that would be possible with the identified password complexity policy.
* plain dictionary words blocking should also be considered


Critical sites should add the following requirements to the password policy:
* Besides the base policy, passwords should also require at least one or more special characters.


===Password Rotation===
===Password Rotation===
Password rotations have proven to be a little tricky and this should only be used if there is lack of monitoring with-in the applications and there is a mitigating reason to use rotations. Reasons being short password, or lack of password controls.  
Password rotations have proven to be a little tricky and this should only be used if there is lack of monitoring within the applications and there is a mitigating reason to use rotations. Reasons being short password, or lack of password controls.  
* Privileged accounts - Password for privileged accounts should be rotated every: 90 days.  
* Privileged accounts - Password for privileged accounts should be rotated every: 90 to 120 days.  
* General User Account - It is also recommended to implement password rotations for general users if possible.
* General User Account - It is also recommended to implement password rotations for general users if possible.
* Log Entry - an application log entry for this event should be generated.
* Log Entry - an application log entry for this event should be generated.


===Account Lockout and Failed Login===
===Account Lockout and Failed Login===
Account Lockouts vs login failures should be evaluated based on the application. In both cases, the application should be able to determine if the password being used is the same one over and over, or a different password being used which would indicate an attack.  
Account Lockouts vs login failures should be evaluated based on the application. In either case, the application should be able to determine if the password being used is the same one over and over, or a different password being used which would indicate an attack.  


The error message for both cases should be generic such as:  
The error message for both cases should be generic such as:  
Line 72: Line 54:
   The username or password you entered is not valid
   The username or password you entered is not valid


Logging will be critical for these events as they will feed up into our security event system and we can then take action based on these events. The application should also take action. Example would be in the case that the user is being attacked, the application should stop and/or slow down that user progress by either presenting a captcha or by doing a time delay for that IP address.
Logging will be critical for these events as they will feed up into our security event system and we can then take action based on these events. The application should also take action. Example would be in the case that the user is being attacked, the application should stop and/or slow down that user progress by either presenting a captcha or by doing a time delay for that IP address. Captcha's should be used in all cases when a limit of failed attempts has been reached.  


=== Password Reset Functions ===
=== Password Reset Functions ===
Line 84: Line 66:


We do not want to provide any information that would allow an attacker to determine if an entered username/email address is valid or invalid. Otherwise an attacker could enumerate valid accounts for phishing attacks or brute force attack.
We do not want to provide any information that would allow an attacker to determine if an entered username/email address is valid or invalid. Otherwise an attacker could enumerate valid accounts for phishing attacks or brute force attack.
=== Email Change and Verification Functions ===
Email verification links should not provide the user with an authenticated session.
Email verification codes must expire after the first use or expire after 8 hours if not used.


===Password Storage===
===Password Storage===
* Passwords stored in a database should using the following format that leverages secure hashing and a per user salt.
Separate from the password policy, we should have the following standards when it comes to storing passwords:
  * Every new password stored in a form like {algo}-{salt}-{hash}
* Passwords stored in a database should using the hmac+bcrypt function.
    * {algo} is {SHA-512},
 
    * {salt} is a salt unique per-user,
The purpose of hmac and bcrypt storage is as follows:
    * {hash} is algo(salt + password)
* bcrypt provides a hashing mechanism which can be configured to consume sufficient time to prevent brute forcing of hash values even with many computers
* bcrypt can be easily adjusted at any time to increase the amount of work and thus provide protection against more powerful systems
* The nonce for the hmac value is designed to be stored on the file system and not in the databases storing the password hashes. In the event of a compromise of hash values due to SQL injection, the nonce will still be an unknown value since it would not be compromised from the file system. This significantly increases the complexity of brute forcing the compromised hashes considering both bcrypt and a large unknown nonce value
* The hmac operation is simply used as a secondary defense in the event there is a design weakness with bcrypt that could leak information about the password or aid an attacker
 
A sample of this code is here: https://github.com/fwenzel/django-sha2
 
Keep in mind that while bcrypt is secure you should still enforce good passwords.
As slow as an algorithm may be if a password is "123" it still would only take a
short amount of time before somebody figures it out.
 
==== Old Password Hashes ====


* Password hashes older than a year should be deleted from the system.
* After a password hash migration, old hashes should be removed within 3 months if user has yet to log in for the conversion process.


====Migration====
====Migration====
The following process can be used to migrate an application that is using a different hashing algorithm than the standard hash listed above. The benefits of this approach is that it instantly upgrades all hashes to the strong, recommended hashing algorithm and it does not require user's to reset their passwords.
 
The following process can be used to migrate an application that is using a different hashing algorithm than the standard hash listed above. The benefits of this approach is that it instantly upgrades all hashes to the strong, recommended hashing algorithm and it does not require users to reset their passwords.


'''Migration Process'''<br>
'''Migration Process'''<br>
Migrate all password hashes entries in the database as follows. This is a one time, offline migration.
Migrate all password hashes entries in the database as follows. This is a one time, offline migration.


Stored in databases in form: {algo}-{salt}-{migration_hash}
Stored in databases in form: {algo}${salt}${migration_hash}
     * {algo} is {SHA512+MD5},
     * {algo} is {sha512+MD5},
     * {salt} is a salt unique per-user,
     * {salt} is a salt unique per-user,
     * {migration_hash} is SHA512(salt + existingPasswordHash)
     * {migration_hash} is SHA512(salt + existingPasswordHash)
Line 108: Line 109:


'''New Login Process'''<br>
'''New Login Process'''<br>
1. Attempt to login user with migration hash. This involves performing the old password hash procedure then adding the salt and finally performing the SHA512.
1. Attempt to login user with migration hash. This involves performing the old password hash procedure then adding the salt and finally performing the sha512.
  Example: Old password hash process is md5
  Example: Old password hash process is md5
  Migration Hash = SHA512(perUserSalt + md5(user supplied password))
  Migration Hash = sha512(perUserSalt + md5(user supplied password))
2. If authentication via migration hash is successful:
2. If authentication via migration hash is successful:
- Use the user's provided password and calculate the New Hash per the algorithm defined above.
- Use the user's provided password and calculate the New Hash per the algorithm defined above.
- Overwrite the Migration Hash with the New Hash
- Overwrite the Migration Hash with the New Hash
3. If authentication via migration hash is NOT successful:  
3. If authentication via migration hash is NOT successful:  
- The user may already be on the New Hash. Attempt to directly authenticate using the new hash. If this fails, then the password provided by the user is wrong.
- The user may already be on the New Hash. Attempt to directly authenticate using the new hash. If this fails, then the password provided by the user is wrong.
<hr>
<old process below - to be removed>
* If upgrading from an existing scheme such as md5 hashing with no-salt, then a conversion process can be established. This conversion will occur whenever a user logs into the system.  First attempt to authenticate the user via the above scheme (e.g. unique salt and sha-512). If that fails then authenticate via the old scheme (e.g. md5) and create a new hash in the above scheme(e.g. unique salt and sha-512). Remember to also clear out the old hash value after the conversion is complete for the user.
* After a six month period of changing from md5 to sha-512, all md5 hash entries should be cleared from the system.
* As part of the password storage and user table, the last password change and last login should be included in the table for tracking.


==Session Management==
==Session Management==
Line 134: Line 130:
The session tokens should be handled by the web server if possible or generated via a cryptographically secure random number generator.  
The session tokens should be handled by the web server if possible or generated via a cryptographically secure random number generator.  
===Inactivity Time Out===
===Inactivity Time Out===
Authenticated sessions should timeout after determined period of inactivity - 15 minutes is recommended
Authenticated sessions should timeout after determined period of inactivity - 15 minutes is recommended.
 
===Secure Flag===
===Secure Flag===
The "Secure" flag should be set during every set-cookie. This will instruct the browser to never send the cookie over HTTP. The purpose of this flag is to prevent the accidental exposure of a cookie value if a user follows an HTTP link.
The "Secure" flag should be set during every set-cookie. This will instruct the browser to never send the cookie over HTTP. The purpose of this flag is to prevent the accidental exposure of a cookie value if a user follows an HTTP link.
Line 142: Line 139:
===HTTP-Only Flag===
===HTTP-Only Flag===
The "HTTP-Only" flag should be set to disable malicious script access to the session ID (e.g. XSS)
The "HTTP-Only" flag should be set to disable malicious script access to the session ID (e.g. XSS)
===Login===
New session IDs should be created on login (to prevent session fixation via XSS on sibling domains or subdomains).
===Logout===
===Logout===
Upon logout the session ID should be invalidated on the server side and deleted on the client via expiration/overwriting the value.
Upon logout the session ID should be invalidated on the server side and deleted on the client via expiration/overwriting the value.
Line 177: Line 178:
'''Examples of Good Input Validation Approaches'''
'''Examples of Good Input Validation Approaches'''
For each field define the types of acceptable characters and an acceptable number of characters for the input
For each field define the types of acceptable characters and an acceptable number of characters for the input
* Username: Letters, numbers, 3 to 10 characters
* Username: Letters, numbers, certain special characters, 3 to 10 characters
* Firstname: Letters, single apostrophe, 1 to 30 characters
* Firstname: Letters, single apostrophe, dash, 1 to 30 characters
* Simple Zipcode: Numbers, 5 characters
* Simple US Zipcode: Numbers, 5 characters
 
Note: These are just examples to illustrate the idea of whitelist input validation. You'll need to adjust based on the type of input you expect.


===JavaScript vs Server Side Validation===
===JavaScript vs Server Side Validation===
Line 220: Line 223:
===Preventing SQL Injection===
===Preventing SQL Injection===
* String concatenation to build any part of a SQL statement with user controlled data creates a SQL injection vulnerability.
* String concatenation to build any part of a SQL statement with user controlled data creates a SQL injection vulnerability.
* Parameterized queries are the sure fire way to prevent SQL injection.
* Parameterized queries are a guaranteed approach to prevent SQL injection.
* It's not realistic to always know if a piece of data is user controlled, therefore parameterized queries should be used whenever a method/function accepts data and uses this data as part of the SQL statement.


Further Reading: [http://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet SQL Injection Prevention Cheat Sheet]
Further Reading: [http://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet SQL Injection Prevention Cheat Sheet]
Line 228: Line 232:
* Ensure that a robust escaping routine is in place to prevent the user from adding additional characters that can be executed by the OS ( e.g. user appends | to the malicious data and then executes another OS command). Remember to use a positive approach when constructing escaping routinges. [http://code.google.com/p/owasp-esapi-java/source/browse/trunk/src/main/java/org/owasp/esapi/codecs/UnixCodec.java Example]
* Ensure that a robust escaping routine is in place to prevent the user from adding additional characters that can be executed by the OS ( e.g. user appends | to the malicious data and then executes another OS command). Remember to use a positive approach when constructing escaping routinges. [http://code.google.com/p/owasp-esapi-java/source/browse/trunk/src/main/java/org/owasp/esapi/codecs/UnixCodec.java Example]


Further Reading: [http://www.owasp.org/index.ph/Reviewing_Code_for_OS_Injection Reviewing Code for OS Injection]
Further Reading: [http://www.owasp.org/index.php/Reviewing_Code_for_OS_Injection Reviewing Code for OS Injection]


===Preventing XML Injection===
===Preventing XML Injection===
Line 235: Line 239:
* If accepting raw XML then more robust validation is necessary. This can be complex. Please contact the [mailto:infrasec@mozilla.com infrastructure security team] for additional discussion
* If accepting raw XML then more robust validation is necessary. This can be complex. Please contact the [mailto:infrasec@mozilla.com infrastructure security team] for additional discussion


==Cross Domain / Unintended User Actions==
==Cross Domain ==
'''Attacks of Concern''': Cross Site Request Forgery (CSRF), Malicious Framing (Clickjacking), 3rd Party Scripts
'''Attacks of Concern''': Cross Site Request Forgery (CSRF), Malicious Framing (Clickjacking), 3rd Party Scripts, Insecure Interaction with 3rd party sites


===Preventing CSRF===
===Preventing CSRF===
Line 244: Line 248:
* Characteristics of a CSRF Token
* Characteristics of a CSRF Token
** Unique per user & per user session
** Unique per user & per user session
** Tied to a single user session
** Large random value
** Large random value
** Generated by a cryptographically secure random number generator
** Generated by a cryptographically secure random number generator
Line 269: Line 274:
* Careful consideration should be used when using third party scripts. While I am sure everybody would do an initial review, updates to scripts should be reviewed with the same due diligence.
* Careful consideration should be used when using third party scripts. While I am sure everybody would do an initial review, updates to scripts should be reviewed with the same due diligence.
* Ensure any scripts that are used are hosted locally and not dynamically referenced from a third party site.
* Ensure any scripts that are used are hosted locally and not dynamically referenced from a third party site.
===Connecting with Twitter, Facebook, etc===
* If using OAuth make sure the entire chain of communication is over HTTPS. This includes the initial OAuth request and any URLs passed as parameters.
* If redirecting to a login page for the app itself, ensure that URL is HTTPS and also that the selected URL does not simply redirect to a HTTP version
* Ensure the "tweet this" or "like this" button does not generate a request to the 3rd party site simply by loading the Mozilla webpage the button is on (e.g. no requests to third party site without user's intent via clicking on the button)


==Secure Transmission==
==Secure Transmission==
Line 288: Line 298:


==Content Security Policy (CSP)==
==Content Security Policy (CSP)==
Develop sites without inline JavaScript so adoption of CSP is easier
https://developer.mozilla.org/en/Introducing_Content_Security_Policy


==Logging==
==Logging==
Line 297: Line 310:
1. Controls are in place to prevent brute force attacks<br>
1. Controls are in place to prevent brute force attacks<br>
Options (any of these are fine):  
Options (any of these are fine):  
* Admin page behind ssl vpn (most popular option)
* Account Lockout
* Account Lockout
* CAPTCHA's after 5 failed logins
* CAPTCHA's after 5 failed logins
Line 307: Line 321:
4. The session id uses the HTTPOnly flag
4. The session id uses the HTTPOnly flag


[https://wiki.mozilla.org/WebAppSec/Secure_Coding_Details#Word_Press Configuring Worpress Admin Pages Securely]
[https://wiki.mozilla.org/WebAppSec/Secure_Coding_Details#Word_Press Configuring Wordpress Admin Pages Securely]
 
== Uploads  ==
 
'''Attacks of Concern''': Malformed user uploads containing JavaScript, HTML or other executable code, Arbitrary file overwrite
 
=== General Uploads  ===
 
'''Upload Verification'''
 
*Use input validation to ensure the uploaded filename uses an expected extension type
*Ensure the uploaded file is not larger than a defined maximum file size
 
'''Upload Storage'''
 
*Use a new filename to store the file on the OS. Do not use any user controlled text for this filename or for the temporary filename.
*Store all user uploaded files on a separate domain (e.g. mozillafiles.net vs mozilla.org). Archives should be analyzed for malicious content (anti-malware, static analysis, etc)
 
'''Public Serving of Uploaded Content'''
 
*Ensure the image is served with the correct content-type (e.g. image/jpeg, application/x-xpinstall)
 
'''Beware of "special" files'''
 
* The upload feature should be using a whitelist approach to only allow specific file types and extensions. However, it is important to be aware of the following file types that, if allowed, could result in security vulnerabilities.
 
*"crossdomain.xml" allows cross-domain data loading in Flash, Java and Silverlight.  If permitted on sites with authentication this can permit cross-domain data theft and CSRF attacks.  Note this can get pretty complicated depending on the specific plugin version in question, so its best to just prohibit files named "crossdomain.xml" or "clientaccesspolicy.xml".
 
*".htaccess" and ".htpasswd" provides server configuration options on a per-directory basis, and should not be permitted.  See http://en.wikipedia.org/wiki/Htaccess
 
=== Image Upload  ===
 
'''Upload Verification'''
 
*Use image rewriting libraries to verify the image is valid and to strip away extraneous content.
*Set the extension of the stored image to be a valid image extension based on the detected content type of the image from image processing (e.g. do not just trust the header from the upload).
*Ensure the detected content type of the image is within a list of defined image types (jpg, png, etc)
 
=== Archive Uploads  ===
 
'''Upload Verification'''
 
*Ensure that the decompressed size of each file within the archive is not larger than a defined maximum size
*Ensure that an uploaded archive matches the type expected (e.g. zip, rar, gzip, etc)
*For structured uploads such as an add-on, ensure that the hierarchy within the archive contains the required files
 
== Error Handling  ==
'''Attacks of Concern''': Sensitive Information Disclosure, System Information Disclosure, Aiding exploitation of other vulnerabilities
 
=== User Facing Error Messages===
Error messages displayed to the user should not contain system, diagnostic or debug information.
 
=== Debug Mode===
Debug mode is supported by many applications and frameworks and is acceptable for Mozilla applications. However, debug mode should only be enabled in stage.
 
=== Formatting Error Messages===
Error messages are often logged to text files or files viewed within a web browser.
* text based log files: Ensure any newline characters (%0A%0C) are appropriately handled to prevent log forging
* web based log files: Ensure any logged html characters are appropriately encoded to prevent XSS when viewing logs
 
=== Recommended Error Handling Design ===
* Log necessary error data to a system log file
* Display a generic error message to the user
* If necessary provide an error code to the user which maps to the error data in the logfile. A user reporting an error can provide this code to help diagnose the issue


=Further Reading=
=Further Reading=
[http://www.owasp.org/index.php/File:OWASP_T10_-_2010_rc1.pdf OWASP Top 10]
* [http://www.owasp.org/index.php/File:OWASP_T10_-_2010_rc1.pdf OWASP Top 10]
 
* [https://www.owasp.org/index.php/Cheat_Sheets OWASP Cheat Sheets]
[http://phpsec.org/library/ Php Sec Library]
* [https://www.owasp.org/index.php/Category:OWASP_Guide_Project OWASP Guide Project]
* [http://phpsec.org/library/ Php Sec Library]
* [http://www.djangobook.com/en/2.0/chapter20/ Django Security]
* [http://guides.rubyonrails.org/security.html Ruby Security]


= Contributors =
= Contributors =
Michael Coates - mcoates [at] mozilla.com<br>
Michael Coates - mcoates [at] mozilla.com<br>
Chris Lyon - clyon [at] mozilla.com
Chris Lyon - clyon [at] mozilla.com<br>
Mark Goodwin - mgoodwin [at] mozilla.com
Mgoodwin
Confirmed users
81

edits

Retrieved from "https://wiki.mozilla.org/Special:MobileDiff/275488...422400"