Merge pull request #58 from elijh/bugfix/security

fix misc security related bugs

9 files changed, 67 insertions, 26 deletions

diff --git a/README.md b/README.md
index 7817c0e..8e59c76 100644
--- a/README.md
+++ b/README.md
@@ -24,10 +24,14 @@ For more information, see these files in the ``doc`` directory:
 Known problems
 ---------------------------
 
-* Client certificates are generated without a CSR. The problem is that this makes the web 
-application extremely vulnerable to denial of service attacks. This was not an issue until we 
-started to allow the possibility of anonymously fetching a client certificate without 
-authenticating first.
+* Client certificates are generated without a CSR. The problem is that this makes the web
+  application extremely vulnerable to denial of service attacks. This was not an issue until we
+  started to allow the possibility of anonymously fetching a client certificate without
+  authenticating first.
+
+* By its very nature, the user database is vulnerable to enumeration attacks. These are
+  very hard to prevent, because our protocol is designed to allow query of a user database via
+  proxy in order to provide network perspective.
 
 Installation
 ---------------------------
@@ -57,12 +61,27 @@ Typically, you run ``bundle`` as a normal user and it will ask you for a sudo pa
 Configuration
 ----------------------------
 
-The webapp can hand out certs for the EIP client. These certs are either picked from a pool in CouchDB or from a file. For now you can either run [Leap CA](http://github.com/leapcode/leap_ca) to fill the pool or you can put your certs file in config/cert.
-
-We also ship provider information through the webapp. For now please add your eip-service.json to the public/config directory.
-
-Copy the example configuration file and customize as appropriate:
-     cp config/config.yml.example config/config.yml
+The configuration file `config/defaults.yml` providers good defaults for most
+values. You can override these defaults by creating a file `config/config.yml`.
+
+There are a few values you should make sure to modify:
+
+    production:
+      admins: ["myusername","otherusername"]
+      domain: example.net
+      force_ssl: true
+      secret_token: "4be2f60fafaf615bd4a13b96bfccf2c2c905898dad34..."
+      client_ca_key: "/etc/ssl/ca.key"
+      client_ca_cert: "/etc/ssl/ca.crt"
+      ca_key_password: nil
+
+* `admins` is an array of usernames that are granted special admin privilege.
+* `domain` is your fully qualified domain name.
+* `force_ssl`, if set to true, will require secure cookies and turn on HSTS. Don't do this if you are using a self-signed server certificate.
+* `secret_token`, used for cookie security, you can create one with `rake secret`. Should be at least 30 characters.
+* `client_ca_key`, the private key of the CA used to generate client certificates.
+* `client_ca_cert`, the public certificate the CA used to generate client certificates.
+* `ca_key_password`, used to unlock the client_ca_key, if needed.
 
 Running
 -----------------------------
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 62d9df2..9734a33 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -1,5 +1,7 @@
 class ApplicationController < ActionController::Base
   protect_from_forgery
+  before_filter :no_cache_header
+  before_filter :no_frame_header
 
   ActiveSupport.run_load_hooks(:application_controller, self)
 
@@ -15,4 +17,20 @@ class ApplicationController < ActionController::Base
   end
   helper_method :bold
 
+  #
+  # we want to prevent the browser from caching anything, just to be safe.
+  #
+  def no_cache_header
+    response.headers["Cache-Control"] = "no-cache, no-store, must-revalidate"
+    response.headers["Pragma"] = "no-cache"
+    response.headers["Expires"] = "0"
+  end
+
+  #
+  # prevent app from being embedded in an iframe, for browsers that support x-frame-options.
+  #
+  def no_frame_header
+    response.headers["X-Frame-Options"] = "DENY"
+  end
+
 end
diff --git a/app/views/layouts/_navigation.html.haml b/app/views/layouts/_navigation.html.haml
index b42c1fe..2f79a22 100644
--- a/app/views/layouts/_navigation.html.haml
+++ b/app/views/layouts/_navigation.html.haml
@@ -2,5 +2,5 @@
   = link_to_navigation t(:overview), user_overview_path(@user), :active => controller?(:overviews)
   = link_to_navigation t(:account_settings), edit_user_path(@user), :active => controller?(:users)
   = link_to_navigation t(:email_settings), edit_user_email_settings_path(@user), :active => controller?(:email_settings)
-  = link_to_navigation t(:support_tickets), auto_tickets_path(@user), :active => controller?(:tickets)
+  = link_to_navigation t(:support_tickets), auto_tickets_path, :active => controller?(:tickets)
   = link_to_navigation t(:logout), logout_path, :method => :delete
diff --git a/config/application.rb b/config/application.rb
index 5e52c7b..ec25da5 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -15,6 +15,11 @@ if defined?(Bundler)
   # Bundler.require(:default, :assets, Rails.env)
 end
 
+APP_CONFIG = ["defaults.yml", "config.yml"].inject({}) {|config, file|
+  filepath = File.expand_path(file, File.dirname(__FILE__))
+  config.merge(File.exists?(filepath) ? YAML.load_file(filepath)[Rails.env] : {})
+}.with_indifferent_access
+
 module LeapWeb
   class Application < Rails::Application
     # Settings in config/environments/* take precedence over those specified here.
diff --git a/config/defaults.yml b/config/defaults.yml
index 54c5a23..343c3ac 100644
--- a/config/defaults.yml
+++ b/config/defaults.yml
@@ -13,24 +13,28 @@ cert_options: &cert_options
   limited_cert_prefix: "LIMITED"
   unlimited_cert_prefix: "UNLIMITED"
 
+common: &common
+  force_ssl: false
+  pagination_size: 30
+
 development:
   <<: *dev_ca
   <<: *cert_options
+  <<: *common
   admins: [blue, admin, admin2]
   domain: example.org
-  secret_token: '550df064dbc5052d9e192b324c1c5a1095c85a2195f88bd6f6829c63b74d8dffa4556494a2e8cc44345a1926be8b6cb17aa4b3f3102d826f5679c3fb57bb7100'
-  pagination_size: 30
+  secret_token: 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
 
 test:
   <<: *dev_ca
   <<: *cert_options
+  <<: *common
   admins: [admin, admin2]
   domain: test.me
-  secret_token: '550df064dbc5052d9e192b324c1c5a1095c85a2195f88bd6f6829c63b74d8dffa4556494a2e8cc44345a1926be8b6cb17aa4b3f3102d826f5679c3fb57bb7100'
-  pagination_size: 30
+  secret_token: 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
 
 production:
   <<: *cert_options
+  <<: *common
   admins: []
   domain: example.net
-  pagination_size: 30
\ No newline at end of file
diff --git a/config/environments/production.rb b/config/environments/production.rb
index d9d37bd..32b4558 100644
--- a/config/environments/production.rb
+++ b/config/environments/production.rb
@@ -28,7 +28,7 @@ LeapWeb::Application.configure do
   # config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for nginx
 
   # Force all access to the app over SSL, use Strict-Transport-Security, and use secure cookies.
-  # config.force_ssl = true
+  config.force_ssl = APP_CONFIG[:force_ssl]
 
   # See everything in the log (default is :info)
   # config.log_level = :debug
diff --git a/config/initializers/secret_token.rb b/config/initializers/secret_token.rb
index c4e1e6d..bdd9b1c 100644
--- a/config/initializers/secret_token.rb
+++ b/config/initializers/secret_token.rb
@@ -5,8 +5,8 @@
 # Make sure the secret is at least 30 characters and all random,
 # no regular words or you'll be exposed to dictionary attacks.
 
-if token = APP_CONFIG['secret_token']
+if token = APP_CONFIG[:secret_token]
   LeapWeb::Application.config.secret_token = token
 else
-  raise StandartError.new("No secret_token defined in config/config.yml - please provide one.")
+  raise StandardError.new("No secret_token defined in config/config.yml - please provide one.")
 end
diff --git a/core/config/initializers/load_config.rb b/core/config/initializers/load_config.rb
deleted file mode 100644
index b2b0318..0000000
--- a/core/config/initializers/load_config.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-def load_config_file(file)
-  File.exists?(file) ? YAML.load_file(file)[Rails.env] : {}
-end
-
-defaults = load_config_file("#{Rails.root}/config/defaults.yml") || {}
-config = load_config_file("#{Rails.root}/config/config.yml") || {}
-APP_CONFIG = defaults.merge(config).with_indifferent_access
diff --git a/help/app/helpers/auto_tickets_path_helper.rb b/help/app/helpers/auto_tickets_path_helper.rb
index bb71260..93f3cb9 100644
--- a/help/app/helpers/auto_tickets_path_helper.rb
+++ b/help/app/helpers/auto_tickets_path_helper.rb
@@ -13,6 +13,7 @@ module AutoTicketsPathHelper
   protected
 
   def auto_tickets_path(options={})
+    return unless options.class == Hash
     options = ticket_view_options.merge options
     if @user
       user_tickets_path(@user, options)
@@ -31,6 +32,7 @@ module AutoTicketsPathHelper
   end
 
   def auto_new_ticket_path(options={})
+    return unless options.class == Hash
     options = ticket_view_options.merge options
     if @user
       new_user_ticket_path(@user, options)