* login in /\w+\.\-_@/ This allows (most) email addresses and is safe…

… for urls, database expressions (@, technically reserved in a url, will survive in most browsers) * put length constraints in migration too * password in 6, 40 * Trivial email validation * Added site key to generator, users.yml. * Made site key generation idempotent in the most crude and hackish way h4. Site key A Site key gives additional protection against a dictionary attack if your DB is ever compromised. With no site key, we store DB_password = hash(user_password, DB_user_salt) If your database were to be compromised you'd be vulnerable to a dictionary attack on all your stupid users' passwords. With a site key, we store DB_password = hash(user_password, DB_user_salt, Code_site_key) That means an attacker needs access to both your site's code *and* its database to mount an "offline dictionary attack":http://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/web-authentication.html It's probably of minor importance, but recommended by best practices: 'defense in depth'. Needless to say, if you upload this to github or the youtubes or otherwise place it in public view you'll kinda defeat the point. Your users' passwords are still secure, and the world won't end, but defense_in_depth -= 1. Please note: if you change this, all the passwords will be invalidated, so DO keep it someplace secure. Use the random value given or type in the lyrics to your favorite Jay-Z song or something; any moderately long, unpredictable text. * Stretch Repeated applications of the hash make brute force (even with a compromised database and site key) harder, and scale with Moore's law. bq. "To squeeze the most security out of a limited-entropy password or passphrase, we can use two techniques [salting and stretching]... that are so simple and obvious that they should be used in every password system. There is really no excuse not to use them. ... Choose stretching factor so computing K from (salt, passwd) takes 200-1000 ms. Store r with the user's password, and increase it as computers get faster.-- http://tinyurl.com/37lb73 Practical Security (Ferguson & Scheier) p350 Now, adding even a 0.2s delay to page requests isn't justifiable for most online applications, and storing r is unnecessary (at least on your first design iteration). But On a 1G Slicehost already under moderate load: irb(main):005:0> puts Time.now; (10**6).times{ secure_digest(Time.now, rand) }; puts Time.now Fri May 16 08:26:16 +0000 2008 Fri May 16 08:30:58 +0000 2008 => 280s/1M ~= 0.000_3 ms / digest A modest 10 (the default here) foldings makes brute forcing, even given the site key and database, 10 times harder at a 3ms penalty. An app that otherwise serves 100 reqs/s is reduced to 78 signin reqs/s; an app that does 10reqs/s is reduced to 9.7 signin reqs/s More: * http://www.owasp.org/index.php/Hashing_Java * "An Illustrated Guide to Cryptographic Hashes":http://www.unixwiz.net/techtips/iguide-crypto-hashes.html
hiphapis · May 18, 2008 · 53f3f3b · 53f3f3b
1 parent c855a16
commit 53f3f3b
Show file tree

Hide file tree

Showing 9 changed files with 339 additions and 46 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,3 +1,6 @@
+
+Changes since fork from technoweenie's branch
+
 I've made a few security changes based on best practices recommended in
 * "The OWASP Guide to Building Secure Web Applications":http://www.owasp.org/index.php/Category:OWASP_Guide_Project
 * "Secure Programming for Linux and Unix HOWTO":http://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/web-authentication.html
@@ -86,6 +89,70 @@ commented out.
 I'd like to get the advice of someone who understands CSRF better than I on this
 change.
 
+h3. Non-backwards compatible Changes
+
+* login in /\w+\.\-_@/  This allows (most) email addresses and is safe for urls, database expressions (@, technically reserved in a url, will survive in most browsers)
+  If you want to be more permissive:
+  "URL-legal characters":http://www.blooberry.com/indexdot/html/topics/urlencoding.htm are <nowiki>-_.!~*'()</nowiki>
+  "XML-legal characters":http://www.sklar.com/blog/archives/96-XML-vs.-Control-Characters.html are <nowiki>Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]</nowiki>
+  "Email-address legal characters":http://tools.ietf.org/html/rfc2822#section-3.4.1 are <nowiki>0-9a-zA-Z!#\$%\&\'\*\+_/=\?^\-`\{|\}~\.</nowiki> but see "this discussion of what is sane"http://www.regular-expressions.info/email.html (as opposed to legal)
+* put length constraints in migration too
+* password in 6, 40
+* Trivial email validation
+* Added site key to generator, users.yml.
+* Made site key generation idempotent in the most crude and hackish way
+
+h4. Site key
+
+A Site key gives additional protection against a dictionary attack if your
+DB is ever compromised.  With no site key, we store
+  DB_password = hash(user_password, DB_user_salt)
+If your database were to be compromised you'd be vulnerable to a dictionary
+attack on all your stupid users' passwords.  With a site key, we store
+  DB_password = hash(user_password, DB_user_salt, Code_site_key)
+That means an attacker needs access to both your site's code *and* its
+database to mount an "offline dictionary attack.":http://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/web-authentication.html
+
+It's probably of minor importance, but recommended by best practices: 'defense
+in depth'.  Needless to say, if you upload this to github or the youtubes or
+otherwise place it in public view you'll kinda defeat the point.  Your users'
+passwords are still secure, and the world won't end, but defense_in_depth -= 1.
+
+Please note: if you change this, all the passwords will be invalidated, so DO
+keep it someplace secure.  Use the random value given or type in the lyrics to
+your favorite Jay-Z song or something; any moderately long, unpredictable text.
+
+* Stretch
+
+Repeated applications of the hash make brute force (even with a compromised
+database and site key) harder, and scale with Moore's law.
+
+  bq. "To squeeze the most security out of a limited-entropy password or
+  passphrase, we can use two techniques [salting and stretching]... that are so
+  simple and obvious that they should be used in every password system.  There
+  is really no excuse not to use them. ... Choose stretching factor so computing
+  K from (salt, passwd) takes 200-1000 ms. Store r with the user's password, and
+  increase it as computers get faster." -- http://tinyurl.com/37lb73
+  Practical Security (Ferguson & Scheier) p350
+
+Now, adding even a 0.2s delay to page requests isn't justifiable for most online
+applications, and storing r is unnecessary (at least on your first design
+iteration).  But 
+  On a 1G Slicehost already under moderate load:
+  irb(main):005:0> puts Time.now; (10**6).times{ secure_digest(Time.now, rand) }; puts Time.now
+  Fri May 16 08:26:16 +0000 2008
+  Fri May 16 08:30:58 +0000 2008
+  => 280s/1M ~= 0.000_3 ms / digest
+A modest 10 (the default here) foldings makes brute forcing, even given the site
+key and database, 10 times harder at a 3ms penalty.  An app that otherwise
+serves 100 reqs/s is reduced to 78 signin reqs/s; an app that does 10reqs/s is
+reduced to 9.7 signin reqs/s
+
+More:
+* http://www.owasp.org/index.php/Hashing_Java
+* "An Illustrated Guide to Cryptographic Hashes":http://www.unixwiz.net/techtips/iguide-crypto-hashes.html
+
+
 h3. Views
 
 * Used escapes <%= %> in email templates (among other reasons, so courtenay's
@@ -98,3 +165,17 @@ h3. Specs
 h3. Stories
 
 * added them
+
+
+h3. Authentication security projects for a later date
+
+A couple little projects
+
+* Track 'failed logins this hour' and demand a captcha after say 5 failed logins
+  ("RECAPTCHA plugin.":http://agilewebdevelopment.com/plugins/recaptcha)
+  "De-proxy-ficate IP address": http://wiki.codemongers.com/NginxHttpRealIpModule
+
+* Make cookie spoofing a little harder: we set the user's cookie to
+  (remember_token), but store digest(remember_token, request_IP). A CSRF cookie
+  spoofer has to then at least also spoof the user's originating IP
+  (see "Secure Programs HOWTO":http://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/web-authentication.html)
diff --git a/generators/authenticated/authenticated_generator.rb b/generators/authenticated/authenticated_generator.rb
@@ -1,4 +1,5 @@
 require 'restful_authentication/rails_commands'
+require 'digest/sha1'
 class AuthenticatedGenerator < Rails::Generator::NamedBase
   default_options :skip_migration => false,
                   :skip_routes => false,
@@ -53,6 +54,8 @@ def initialize(runtime_args, runtime_options = {})
     else
       @model_controller_class_name = "#{@model_controller_class_nesting}::#{@model_controller_class_name_without_nesting}"
     end
+
+    load_or_initialize_site_keys()
   end
 
   def manifest
@@ -76,6 +79,7 @@ def manifest
       m.directory File.join('app/controllers', model_controller_class_path)
       m.directory File.join('app/helpers', model_controller_class_path)
       m.directory File.join('app/views', model_controller_class_path, model_controller_file_name)
+      m.directory File.join('config/initializers')
 
       if @rspec
         m.directory File.join('spec/controllers', controller_class_path)
@@ -121,6 +125,8 @@ def manifest
       m.template 'authenticated_test_helper.rb',
                   File.join('lib', 'authenticated_test_helper.rb')
 
+      m.template 'site_keys.rb', site_keys_file
+
       if @rspec
         # RSpec Specs
         m.template  'spec/controllers/users_controller_spec.rb',
@@ -282,6 +288,38 @@ def manifest
   def has_rspec?
     options[:rspec] || (File.exist?('spec') && File.directory?('spec'))
   end
+  def secure_digest(*args)
+    Digest::SHA1.hexdigest(args.flatten.join('&&'))
+  end
+  def make_token
+    secure_digest(Time.now, (1..10).map{ rand.to_s })
+  end
+  def password_digest(password, salt)
+    digest = $rest_auth_site_key_from_generator
+    $rest_auth_digest_stretches_from_generator.times do
+      digest = secure_digest(salt, digest, password, $rest_auth_site_key_from_generator)
+    end
+    digest
+  end
+
+  #
+  # Try to be idempotent:
+  # pull in the existing site key if any,
+  # seed it with reasonable defaults otherwise
+  #
+  def load_or_initialize_site_keys
+    begin 
+      require RAILS_ROOT + '/' + site_keys_file
+    rescue Exception ; true end # don't complain
+    $rest_auth_site_key_from_generator         = (
+      (defined? REST_AUTH_SITE_KEY)         ? REST_AUTH_SITE_KEY : make_token)
+    $rest_auth_digest_stretches_from_generator = (
+      (defined? REST_AUTH_DIGEST_STRETCHES) ? REST_AUTH_DIGEST_STRETCHES : 10)  
+    puts [$rest_auth_site_key_from_generator, $rest_auth_digest_stretches_from_generator]
+  end  
+  def site_keys_file
+    File.join("config", "initializers", "site_keys.rb")
+  end
 
   protected
     # Override with your own usage banner.

diff --git a/generators/authenticated/templates/authenticated_test_helper.rb b/generators/authenticated/templates/authenticated_test_helper.rb
@@ -5,7 +5,7 @@ def login_as(<%= file_name %>)
   end
 
   def authorize_as(<%= file_name %>)
-    @request.env["HTTP_AUTHORIZATION"] = <%= file_name %> ? ActionController::HttpAuthentication::Basic.encode_credentials(<%= table_name %>(<%= file_name %>).login, 'test') : nil
+    @request.env["HTTP_AUTHORIZATION"] = <%= file_name %> ? ActionController::HttpAuthentication::Basic.encode_credentials(<%= table_name %>(<%= file_name %>).login, 'monkey') : nil
   end
 
 <% if options[:rspec] -%>

diff --git a/generators/authenticated/templates/migration.rb b/generators/authenticated/templates/migration.rb
@@ -1,8 +1,9 @@
 class <%= migration_name %> < ActiveRecord::Migration
   def self.up
     create_table "<%= table_name %>", :force => true do |t|
-      t.column :login,                     :string
-      t.column :email,                     :string
+      t.column :login,                     :string, :limit => 40
+      t.column :name,                      :string, :limit => 100, :default => '', :null => true
+      t.column :email,                     :string, :limit => 100
       t.column :crypted_password,          :string, :limit => 40
       t.column :salt,                      :string, :limit => 40
       t.column :created_at,                :datetime

diff --git a/generators/authenticated/templates/model.rb b/generators/authenticated/templates/model.rb
@@ -1,21 +1,52 @@
 require 'digest/sha1'
+
+# Uncomment to suit
+# RE_LOGIN_OK   = /\A[[:alnum:]][[:alnum:]\.\-_@]+\z/     # Unicode, strict
+RE_LOGIN_OK     = /\A\w[\w\.\-_@]+\z/                     # ASCII, strict
+MSG_LOGIN_BAD   = "use only letters, numbers, and .-_@ please."
+
+RE_NAME_OK      = /\A[^[:cntrl:]\\<>\/&]*\z/              # Unicode, permissive
+MSG_NAME_BAD    = "avoid non-printing characters and \\&gt;&lt;&amp;/ please."
+
+# This is purposefully imperfect -- it's just a check for bogus input. See
+# http://www.regular-expressions.info/email.html
+#RE_EMAIL_NAME   = '0-9A-Z!#\$%\&\'\*\+_/=\?^\-`\{|\}~\.' # technically allowed by RFC-2822
+RE_EMAIL_NAME   = '[\w\.%\+\-]+'                          # what you actually see in practice
+RE_DOMAIN_HEAD  = '(?:[A-Z0-9\-]+\.)+'
+RE_DOMAIN_TLD   = '(?:[A-Z]{2}|com|org|net|gov|mil|biz|info|mobi|name|aero|jobs|museum)'
+RE_EMAIL_OK     = /\A#{RE_EMAIL_NAME}@#{RE_DOMAIN_HEAD}#{RE_DOMAIN_TLD}\z/i
+MSG_EMAIL_BAD   = "should look like an email address."
+
+
 class <%= class_name %> < ActiveRecord::Base
   # Virtual attribute for the unencrypted password
   attr_accessor :password
 
-  validates_presence_of     :login, :email
+  validates_presence_of     :login
+  validates_length_of       :login,    :within => 3..40
+  validates_uniqueness_of   :login,    :case_sensitive => false
+  validates_format_of       :login,    :with => RE_LOGIN_OK, :message => MSG_LOGIN_BAD
+
+  validates_format_of       :name,     :with => RE_NAME_OK,  :message => MSG_NAME_BAD, :allow_nil => true
+  validates_length_of       :name,     :maximum => 100
+
+  validates_presence_of     :email
+  validates_length_of       :email,    :within => 6..100 #r@a.wk
+  validates_uniqueness_of   :email,    :case_sensitive => false
+  validates_format_of       :email,    :with => RE_EMAIL_OK, :message => MSG_EMAIL_BAD
+
   validates_presence_of     :password,                   :if => :password_required?
   validates_presence_of     :password_confirmation,      :if => :password_required?
-  validates_length_of       :password, :within => 4..40, :if => :password_required?
   validates_confirmation_of :password,                   :if => :password_required?
-  validates_length_of       :login,    :within => 3..40
-  validates_length_of       :email,    :within => 3..100
-  validates_uniqueness_of   :login, :email, :case_sensitive => false
+  validates_length_of       :password, :within => 6..40, :if => :password_required?
   before_save :encrypt_password
+
   <% if options[:include_activation] && !options[:stateful] %>before_create :make_activation_code <% end %>
+
   # prevents a user from submitting a crafted form that bypasses activation
   # anything else you want your user to change should be added here.
-  attr_accessible :login, :email, :password, :password_confirmation
+  attr_accessible :login, :email, :name, :password, :password_confirmation
+
 <% if options[:stateful] %>
   acts_as_state_machine :initial => :pending
   state :passive
@@ -69,11 +100,6 @@ def self.authenticate(login, password)
     u && u.authenticated?(password) ? u : nil
   end
 
-  # Encrypts the password with the user salt
-  def encrypt(password)
-    self.class.password_digest(password, salt)
-  end
-
   def authenticated?(password)
     crypted_password == encrypt(password)
   end
@@ -126,7 +152,6 @@ def encrypt_password
       self.salt = self.class.make_token if new_record?
       self.crypted_password = encrypt(password)
     end
-
     def password_required?
       crypted_password.blank? || !password.blank?
     end
@@ -147,11 +172,32 @@ def do_activate
       self.deleted_at = self.activation_code = nil
     end<% end %>
 
-    def self.password_digest(password, salt)
+    # Encrypts the password with the user salt
+    def encrypt(password)
+      self.class.password_digest(password, salt)
+    end
+
+    # Backwards-compatible; replace call to "password_digest" with "old_password_digest"
+    def self.old_password_digest(password, salt)
       Digest::SHA1.hexdigest("--#{salt}--#{password}--")
     end
+
+    # This provides a modest increased defense against a dictionary attack if
+    # your db were ever compromised, but will invalidate existing passwords.
+    # See the README.
+    def self.password_digest(password, salt)
+      digest = REST_AUTH_SITE_KEY
+      REST_AUTH_DIGEST_STRETCHES.times do
+        digest = secure_digest(salt, digest, password, REST_AUTH_SITE_KEY)
+      end
+      digest
+    end
+
+    def self.secure_digest(*args)
+      Digest::SHA1.hexdigest(args.flatten.join('&&'))
+    end
 
     def self.make_token
-      Digest::SHA1.hexdigest([Time.now, (1..10).map{ rand.to_s }].flatten.join('&&'))
+      secure_digest(Time.now, (1..10).map{ rand.to_s })
     end 
 end
diff --git a/generators/authenticated/templates/site_keys.rb b/generators/authenticated/templates/site_keys.rb
@@ -0,0 +1,38 @@
+
+# A Site key gives additional protection against a dictionary attack if your
+# DB is ever compromised.  With no site key, we store
+#   DB_password = hash(user_password, DB_user_salt)
+# If your database were to be compromised you'd be vulnerable to a dictionary
+# attack on all your stupid users' passwords.  With a site key, we store
+#   DB_password = hash(user_password, DB_user_salt, Code_site_key)
+# That means an attacker needs access to both your site's code *and* its
+# database to mount an "offline dictionary attack.":http://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/web-authentication.html
+# 
+# It's probably of minor importance, but recommended by best practices: 'defense
+# in depth'.  Needless to say, if you upload this to github or the youtubes or
+# otherwise place it in public view you'll kinda defeat the point.  Your users'
+# passwords are still secure, and the world won't end, but defense_in_depth -= 1.
+# 
+# Please note: if you change this, all the passwords will be invalidated, so DO
+# keep it someplace secure.  Use the random value given or type in the lyrics to
+# your favorite Jay-Z song or something; any moderately long, unpredictable text.
+REST_AUTH_SITE_KEY         = '<%= $rest_auth_site_key_from_generator %>'
+
+# Repeated applications of the hash make brute force (even with a compromised
+# database and site key) harder, and scale with Moore's law.
+#
+#   bq. "To squeeze the most security out of a limited-entropy password or
+#   passphrase, we can use two techniques [salting and stretching]... that are
+#   so simple and obvious that they should be used in every password system.
+#   There is really no excuse not to use them." http://tinyurl.com/37lb73
+#   Practical Security (Ferguson & Scheier) p350
+# 
+# A modest 10 foldings (the default here) adds 3ms.  This makes brute forcing 10
+# times harder, while reducing an app that otherwise serves 100 reqs/s to 78 signin
+# reqs/s, an app that does 10reqs/s to 9.7 reqs/s
+# 
+# More:
+# * http://www.owasp.org/index.php/Hashing_Java
+# * "An Illustrated Guide to Cryptographic Hashes":http://www.unixwiz.net/techtips/iguide-crypto-hashes.html
+
+REST_AUTH_DIGEST_STRETCHES = <%= $rest_auth_digest_stretches_from_generator %>
diff --git a/generators/authenticated/templates/spec/controllers/users_controller_spec.rb b/generators/authenticated/templates/spec/controllers/users_controller_spec.rb
@@ -62,12 +62,12 @@
 
   <% if options[:include_activation] %>
   it 'activates user' do
-    <%= class_name %>.authenticate('aaron', 'test').should be_nil
+    <%= class_name %>.authenticate('aaron', 'monkey').should be_nil
     get :activate, :activation_code => <%= table_name %>(:aaron).activation_code
     response.should redirect_to('/<%= controller_file_path %>/new')
     flash[:notice].should_not be_nil
     flash[:error ].should     be_nil
-    <%= class_name %>.authenticate('aaron', 'test').should == <%= table_name %>(:aaron)
+    <%= class_name %>.authenticate('aaron', 'monkey').should == <%= table_name %>(:aaron)
   end
 
   it 'does not activate user without key' do
@@ -90,7 +90,7 @@
   
   def create_<%= file_name %>(options = {})
     post :create, :<%= file_name %> => { :login => 'quire', :email => 'quire@example.com',
-      :password => 'quire', :password_confirmation => 'quire' }.merge(options)
+      :password => 'quire69', :password_confirmation => 'quire69' }.merge(options)
   end
 end