From 125dbd3daee0522bc5ee142e41e6eeab33650816 Mon Sep 17 00:00:00 2001 From: Vicki Cheung Date: Mon, 21 Dec 2015 12:22:02 -0800 Subject: [PATCH 1/7] Set is a built-in ruby class and should not be overwritten Rename Gaps::DB::Set to Gaps::DB::GroupSet --- bin/gaps_server.rb | 10 +++++----- lib/gaps/db/set.rb | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bin/gaps_server.rb b/bin/gaps_server.rb index 6c7f509..2256327 100755 --- a/bin/gaps_server.rb +++ b/bin/gaps_server.rb @@ -267,14 +267,14 @@ def die(msg) ## Suggested Sets get '/sets', auth: true do - @sets = Gaps::DB::Set.find_each + @sets = Gaps::DB::GroupSet.find_each erb :sets end post '/sets', auth: true do set_id = params[:set] log.info('Adding user to subscription set', set: set_id, user: @user.email) - Gaps::DB::Set.find(set_id).groups_.each do |group| + Gaps::DB::GroupSet.find(set_id).groups_.each do |group| membership = @user.group_member?(group) through_list = @user.group_member_through_list(group) direct_membership = membership && !through_list @@ -295,7 +295,7 @@ def die(msg) get '/sets/:id', auth: true do if params[:id] != 'new' - @set = Gaps::DB::Set.find(params[:id]) + @set = Gaps::DB::GroupSet.find(params[:id]) return not_found unless @set end @groups = Gaps::DB::Group.categorized(@user) @@ -310,9 +310,9 @@ def die(msg) } if params[:id] == 'new' - Gaps::DB::Set.new(args).save + Gaps::DB::GroupSet.new(args).save else - set = Gaps::DB::Set.find(params[:id]) + set = Gaps::DB::GroupSet.find(params[:id]) return not_found unless set old_groups = set.groups diff --git a/lib/gaps/db/set.rb b/lib/gaps/db/set.rb index 928e7bd..a3c3df5 100644 --- a/lib/gaps/db/set.rb +++ b/lib/gaps/db/set.rb @@ -1,5 +1,5 @@ module Gaps::DB - class Set < Base + class GroupSet < Base set_collection_name 'set' key :name, String From 2b5560547c55627a2ea0567db031b61dd5c832cc Mon Sep 17 00:00:00 2001 From: Greg Brockman Date: Sat, 16 Jan 2016 15:14:22 -0800 Subject: [PATCH 2/7] Allow users to store OAuth keys encrypted Also add documentation to explicitly call out the fact that this application has privileged access within your organization. --- Gemfile | 1 + Gemfile.lock | 7 +++++ README.md | 14 ++++++++- config.yaml | 5 +++ lib/gaps/db.rb | 10 ++++++ lib/gaps/db/user.rb | 74 +++++++++++++++++++++++++++++++++++++++++++-- site.yaml.sample | 2 ++ 7 files changed, 110 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index c493c74..701596e 100644 --- a/Gemfile +++ b/Gemfile @@ -17,6 +17,7 @@ gem 'bson_ext' gem 'einhorn' gem 'chalk-log' gem 'chalk-config' +gem 'symmetric-encryption' gem 'rake' diff --git a/Gemfile.lock b/Gemfile.lock index 1f55b54..a5c950e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -34,7 +34,11 @@ GEM logging lspace coderay (1.1.0) + coercible (1.0.0) + descendants_tracker (~> 0.0.1) configatron (4.4.1) + descendants_tracker (0.0.4) + thread_safe (~> 0.3, >= 0.3.1) einhorn (0.6.3) erubis (2.7.0) extlib (0.9.16) @@ -110,6 +114,8 @@ GEM rack-protection (~> 1.4) tilt (~> 1.3, >= 1.3.4) slop (3.6.0) + symmetric-encryption (3.8.2) + coercible (~> 1.0) thread (0.1.4) thread_safe (0.3.4) tilt (1.4.1) @@ -139,4 +145,5 @@ DEPENDENCIES rake rest-client sinatra + symmetric-encryption thread diff --git a/README.md b/README.md index 25e2a33..86c339f 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,19 @@ configuration file. Clone this repository and execute `bin/docker-runner` to run the Docker image we've published with your `site.yaml` bind-mounted inside. -# Permissions +# Security + +The Gaps application runs with the minimal privileges it can. However, +it needs to perform powerful actions such as viewing all lists and +subscribing a user to any list. You should restrict access to Gaps +appropriately. + +Please be aware that Gaps stores its OAuth credentials in its backing +database. You can enable encryption of those credentials via the +`encrypt_oauth_credentials` configuration option (and also providing a +value for `db.encryption_key`). + +## Permissions Gaps uses your domain admin's credentials to perform most actions (listing all groups, joining a group). So permissions are entrusted to diff --git a/config.yaml b/config.yaml index f9cf85a..9f204ca 100644 --- a/config.yaml +++ b/config.yaml @@ -1,4 +1,9 @@ default: &default + # We disable this by default for now. People need to generate their + # own encryption key (under `db.encryption_key`) for it to be + # useful. + encrypt_oauth_credentials: false + cache: # How large of a threadpool to maintain for populating the cache # of what each list is subscribed to. (Note for implementation diff --git a/lib/gaps/db.rb b/lib/gaps/db.rb index 130a086..09577c9 100644 --- a/lib/gaps/db.rb +++ b/lib/gaps/db.rb @@ -6,6 +6,16 @@ class DBError < StandardError; end class UniqueKeyViolation < DBError; end def self.init + # Initialize even if we're not encrypting, since we may need to + # unencrypt some records. + if configatron.db.key?(:encryption_key) + SymmetricEncryption.cipher = SymmetricEncryption::Cipher.new( + cipher_name: 'aes-128-cbc', + key: configatron.db.encryption_key, + encoding: :base64strict + ) + end + MongoMapper.database = configatron.db.database MongoMapper.connection = Mongo::MongoClient.from_uri(configatron.db.mongodb_url, pool_size: 5) diff --git a/lib/gaps/db/user.rb b/lib/gaps/db/user.rb index 438ecca..c7b6e9f 100644 --- a/lib/gaps/db/user.rb +++ b/lib/gaps/db/user.rb @@ -1,4 +1,5 @@ require 'thread' +require 'symmetric-encryption' module Gaps::DB class User < Base @@ -13,8 +14,10 @@ class InvalidUser < StandardError; end key :admin, Boolean, default: false key :image_url, String - key :refresh_token, String - key :access_token, String + key :refresh_token, String # plaintext, thus deprecated + key :access_token, String # plaintext, thus deprecated + key :encrypted_refresh_token, String + key :encrypted_access_token, String key :expires_in, Integer key :issued_at, Integer @@ -26,6 +29,73 @@ class InvalidUser < StandardError; end key :sets, Array, :default => [] + ### Secrets + + def initialize_from_database(*args) + ret = super + + # Migrate back and forth between encrypted credentials at load time + if configatron.encrypt_oauth_credentials && (@access_token || @refresh_token) + self.encrypted_access_token = encrypt(@access_token) + self.encrypted_refresh_token = encrypt(@refresh_token) + @access_token = nil + @refresh_token = nil + save! + elsif !configatron.encrypt_oauth_credentials && (@encrypted_access_token || @encrypted_refresh_token) + self.access_token = decrypt(@encrypted_access_token) + self.refresh_token = decrypt(@encrypted_refresh_token) + @encrypted_access_token = nil + @encrypted_refresh_token = nil + save! + end + + ret + end + + def encrypt(str) + # Use random iv and compress + SymmetricEncryption.encrypt(str, true, true) + end + + def decrypt(str) + SymmetricEncryption.decrypt(str) + end + + def access_token + if configatron.encrypt_oauth_credentials + decrypt(self.encrypted_access_token) + else + super + end + end + + def access_token=(tok) + if configatron.encrypt_oauth_credentials + self.encrypted_access_token = encrypt(tok) + else + super + end + end + + def refresh_token + if configatron.encrypt_oauth_credentials + decrypt(self.encrypted_refresh_token) + else + super + end + end + + def refresh_token=(tok) + if configatron.encrypt_oauth_credentials + self.encrypted_refresh_token = encrypt(tok) + else + super + end + end + + + ### End secrets + def self.build_index self.ensure_index([[:google_id, 1]], unique: true, sparse: true) end diff --git a/site.yaml.sample b/site.yaml.sample index 7694ca5..f5358de 100644 --- a/site.yaml.sample +++ b/site.yaml.sample @@ -5,6 +5,8 @@ db: # persisted). Note that Gaps stores some interesting state, such as # people's filter configuration and group categorization. mongodb_url: mongodb://localhost:27017 + # Set this to a random string, such as `openssl rand -base64 129` + encryption_key: "FN25NUJ6pk1Ys53aVHJp7usl1Dnz18QHy7+7FQH3R/QXd5XC9Syh5Dx+tsvD2o4OWylHqw9sgPQUU472B5RZXkx27Wo2rw0nlhXsYFWAVRpRz8jZ7Egzl4K0t4c0m/Gzy8QuMRbDCEktJq41S78J5KJYopTJ1oNKPlxjAKj7mFCl" # Your favicon favicon: From 3b66f0a8b1388d1c2224313e7463c7b58d037002 Mon Sep 17 00:00:00 2001 From: Greg Brockman Date: Sat, 16 Jan 2016 16:04:57 -0800 Subject: [PATCH 3/7] Swap Docker base image to official Ruby build The phusion one changed the Ruby version, which broke the build --- Dockerfile | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 77ba7d9..3334f21 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,9 +1,11 @@ -FROM phusion/passenger-ruby21 +FROM ruby:2.1.5 +RUN useradd app && mkdir /home/app MAINTAINER Greg Brockman ADD . /gaps # If you're running a version of docker before .dockerignore RUN rm -f /gaps/site.yaml* -RUN chown -R app: /gaps +# Need app to write to /usr/local so the bundle command works +RUN chown -R app: /gaps /usr/local USER app ENV HOME /home/app RUN cd /gaps && bundle install --path vendor/bundle From 8dcc284927011863ab9fc38e501386a02302bb4c Mon Sep 17 00:00:00 2001 From: Vicki Cheung Date: Mon, 21 Dec 2015 12:26:09 -0800 Subject: [PATCH 4/7] Fix Mongo connection pool size to match thread pool size --- lib/gaps/db.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/gaps/db.rb b/lib/gaps/db.rb index 09577c9..d950599 100644 --- a/lib/gaps/db.rb +++ b/lib/gaps/db.rb @@ -17,7 +17,10 @@ def self.init end MongoMapper.database = configatron.db.database - MongoMapper.connection = Mongo::MongoClient.from_uri(configatron.db.mongodb_url, pool_size: 5) + MongoMapper.connection = Mongo::MongoClient.from_uri( + configatron.db.mongodb_url, + pool_size: [configatron.cache.pool_size, 5].max + ) Cache.build_index Group.build_index From 8840ec4eeb73cb385e2c8390426412def80b63e0 Mon Sep 17 00:00:00 2001 From: Greg Brockman Date: Sat, 27 Feb 2016 17:32:48 -0800 Subject: [PATCH 5/7] Add db encryption key to Heroku --- app.json | 7 ++++++- bin/hk-runner | 3 +++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app.json b/app.json index 1529fb3..3d1123a 100644 --- a/app.json +++ b/app.json @@ -23,6 +23,11 @@ "required": true, "description": "A secret key for sessions.", "generator": "secret" + }, + "DB_ENCRYPTION_KEY": { + "required": false, + "description": "Secret key to encrypt your OAuth credentials at rest", + "generator": "secret" } } -} \ No newline at end of file +} diff --git a/bin/hk-runner b/bin/hk-runner index 24cecb7..06f9a81 100755 --- a/bin/hk-runner +++ b/bin/hk-runner @@ -16,9 +16,12 @@ arr=(${MONGODB_URL//\// }) MONGODB_DATABASE=${arr[2]} cat < site.yaml +encrypt_oauth_credentials: true + db: database: $MONGODB_DATABASE mongodb_url: $MONGODB_URL + encryption_key: $DB_ENCRYPTION_KEY favicon: $FAVICON_URL From 12ea460064c7ca569d2ca6163bdc5c57769d9c71 Mon Sep 17 00:00:00 2001 From: Greg Brockman Date: Sat, 27 Feb 2016 19:01:04 -0800 Subject: [PATCH 6/7] Bump Mongo for compatibility with more modern servers --- Gemfile | 2 ++ Gemfile.lock | 14 +++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index 701596e..83c4770 100644 --- a/Gemfile +++ b/Gemfile @@ -13,6 +13,8 @@ gem 'rack_csrf' gem 'erubis' gem 'sinatra' gem 'mongo_mapper' +# Needed for MongoLab which requires SCRAM-SHA-1 +gem 'mongo', '~> 1.12' gem 'bson_ext' gem 'einhorn' gem 'chalk-log' diff --git a/Gemfile.lock b/Gemfile.lock index a5c950e..309fc68 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -23,9 +23,9 @@ GEM addressable (>= 2.3.1) extlib (>= 0.9.15) multi_json (>= 1.0.0) - bson (1.11.1) - bson_ext (1.11.1) - bson (~> 1.11.1) + bson (1.12.5) + bson_ext (1.12.5) + bson (~> 1.12.5) builder (3.2.2) chalk-config (0.2.1) configatron (~> 4.4) @@ -74,8 +74,8 @@ GEM minitest (5.4.3) mocha (1.1.0) metaclass (~> 0.0.1) - mongo (1.11.1) - bson (= 1.11.1) + mongo (1.12.5) + bson (= 1.12.5) mongo_mapper (0.13.1) activemodel (>= 3.0.0) activesupport (>= 3.0) @@ -137,6 +137,7 @@ DEPENDENCIES google-api-client mail mocha + mongo (~> 1.12) mongo_mapper pry puma @@ -147,3 +148,6 @@ DEPENDENCIES sinatra symmetric-encryption thread + +BUNDLED WITH + 1.11.2 From 3ba3ed6fe98350c835143a7cc2a8901cdf7e7367 Mon Sep 17 00:00:00 2001 From: Greg Brockman Date: Sat, 27 Feb 2016 19:19:44 -0800 Subject: [PATCH 7/7] Turn on api_scheme --- config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.yaml b/config.yaml index 9f204ca..7d8ccb1 100644 --- a/config.yaml +++ b/config.yaml @@ -46,7 +46,7 @@ default: &default gaps_scheme: true # Whether to use the Google Groups Settings API to decide if a # list is private. (Can be composed with the Gaps scheme.) - api_scheme: false + api_scheme: true # Whether to fetch the Google Groups settings upon group # refresh. Needed to make the api_scheme work. In the future will