use SSL-only flag on CSRF cookie

fixes CNVS-16884

test-plan:
 - in HTTP-session environment (`secure: false` or unset in
   config/session_store.yml), CSRF cookie should be sent without secure
   flag

 - in HTTPS-session environment (`secure: true` in
   config/session_store.yml), CSRF cookie should be sent with secure
   flag.

 - on a non-files host, javascript should be able to read the CSRF
   cookie: value of "document.cookie" in javascript console should
   include "_csrf_token"

 - on a files host, javascript should not be able to read the CSRF
   cookie: value of "document.cookie" in javascript console should not
   include "_csrf_token"

Change-Id: Ifd57c973478a6d07497a404dcf1a9b9caa9014af
Reviewed-on: https://gerrit.instructure.com/44451
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>

app/controllers/application_controller.rb

@@ -1115,7 +1115,7 @@ class ApplicationController < ActionController::Base
   end
 
   def form_authenticity_token
-    CanvasBreachMitigation::MaskingSecrets.masked_authenticity_token(cookies)
+    masked_authenticity_token
   end
 
   API_REQUEST_REGEX = %r{\A/api/v\d}

gems/canvas_breach_mitigation/lib/canvas_breach_mitigation/masking_secrets.rb

@@ -6,13 +6,16 @@ module CanvasBreachMitigation
       # Sets the token value for the current session and returns it in
       # a masked form that's safe to send to the client. See section
       # 3.4 of "BREACH: Reviving the CRIME attack".
-      def masked_authenticity_token(cookies)
-        one_time_pad = SecureRandom.random_bytes(AUTHENTICITY_TOKEN_LENGTH)
+      def masked_authenticity_token(cookies, options={})
+        # remask token
+        encoded_masked_token = masked_token(unmasked_token(cookies['_csrf_token']))
 
-        encrypted_csrf_token = xor_byte_strings(one_time_pad, unmasked_token(cookies['_csrf_token']))
-        masked_token = one_time_pad + encrypted_csrf_token
-        encoded_masked_token = Base64.strict_encode64(masked_token)
-        cookies['_csrf_token'] = encoded_masked_token
+        cookie = { value: encoded_masked_token }
+        [:domain, :httponly, :secure].each do |key|
+          next unless options.has_key?(key)
+          cookie[key] = options[key]
+        end
+        cookies['_csrf_token'] = cookie
 
         encoded_masked_token
       end
@@ -27,6 +30,13 @@ module CanvasBreachMitigation
             unmasked_token(cookies['_csrf_token']) == unmasked_token(encoded_masked_token)
       end
 
+      def masked_token(token)
+        one_time_pad = SecureRandom.random_bytes(AUTHENTICITY_TOKEN_LENGTH)
+        encrypted_csrf_token = xor_byte_strings(one_time_pad, token)
+        masked_token = one_time_pad + encrypted_csrf_token
+        Base64.strict_encode64(masked_token)
+      end
+
       private
 
       def unmasked_token(encoded_masked_token)
@@ -46,4 +56,4 @@ module CanvasBreachMitigation
       end
     end
   end
-end
+end

gems/canvas_breach_mitigation/spec/masking_secrets_spec.rb

@@ -2,7 +2,7 @@ require "spec_helper"
 
 describe CanvasBreachMitigation::MaskingSecrets do
   before do
-    Rails = double("Rails").as_null_object unless defined? Rails
+    Rails = mock("Rails") unless defined? Rails
   end
 
   let(:masking_secrets) { CanvasBreachMitigation::MaskingSecrets }
@@ -11,59 +11,85 @@ describe CanvasBreachMitigation::MaskingSecrets do
     it "puts :_csrf_token into the supplied object" do
       hash = {}
       masking_secrets.masked_authenticity_token(hash)
-      hash['_csrf_token'].should_not be_nil
+      expect(hash['_csrf_token']).not_to be nil
     end
 
     it "returns a byte string" do
-      masking_secrets.masked_authenticity_token({}).should_not be nil
+      expect(masking_secrets.masked_authenticity_token({})).not_to be nil
     end
   end
 
   describe ".valid_authenticity_token?" do
-    let(:cookies) do
-      # Seed a "cookie jar" with a :_csrf_token
-      Hash.new.tap do |cookies|
-        masking_secrets.masked_authenticity_token(cookies)
-      end
-    end
-
-    let(:session) do
-      Hash.new
-    end
-
-    it "returns true for a valid unmasked token" do
-      valid_unmasked = cookies['_csrf_token']
-      masking_secrets.valid_authenticity_token?(session, cookies, valid_unmasked).should == true
-    end
-
-    it "returns false for an invalid unmasked token" do
-      masking_secrets.valid_authenticity_token?(session, cookies, SecureRandom.base64(32)).should == false
-    end
+    let(:unmasked_token) { SecureRandom.base64(32) }
+    let(:cookies) { {'_csrf_token' => masking_secrets.masked_token(unmasked_token)} }
+    let(:session) { {} }
 
     it "returns true for a valid masked token" do
-      valid_masked = masking_secrets.masked_authenticity_token(cookies)
-      masking_secrets.valid_authenticity_token?(session, cookies, valid_masked).should == true
+      valid_masked = masking_secrets.masked_token(unmasked_token)
+      expect(masking_secrets.valid_authenticity_token?(session, cookies, valid_masked)).to be true
     end
 
     it "returns false for an invalid masked token" do
-      masking_secrets.valid_authenticity_token?(session, cookies, SecureRandom.base64(64)).should == false
+      expect(masking_secrets.valid_authenticity_token?(session, cookies, SecureRandom.base64(64))).to be false
     end
 
     it "returns false for a token of the wrong length" do
-      masking_secrets.valid_authenticity_token?(session, cookies, SecureRandom.base64(2)).should == false
+      expect(masking_secrets.valid_authenticity_token?(session, cookies, SecureRandom.base64(2))).to be false
     end
 
     it "returns true if the session is still set instead of the cookies" do
       cookies.delete('_csrf_token')
-
       session[:_csrf_token] = SecureRandom.base64(32)
-      one_time_pad = SecureRandom.random_bytes(32)
-
-      encrypted_csrf_token = masking_secrets.send(:xor_byte_strings, one_time_pad, Base64.strict_decode64(session[:_csrf_token]))
-      masked_token = one_time_pad + encrypted_csrf_token
-      encoded_masked_token = Base64.strict_encode64(masked_token)
-
-      masking_secrets.valid_authenticity_token?(session, cookies, encoded_masked_token).should == true
+      encoded_masked_token = masking_secrets.masked_token(Base64.strict_decode64(session[:_csrf_token]))
+      expect(masking_secrets.valid_authenticity_token?(session, cookies, encoded_masked_token)).to be true
     end
   end
-end
+
+  describe ".masked_authenticity_token" do
+    let(:cookies) { {} }
+
+    it "initializes the _csrf_token cookie" do
+      token = masking_secrets.masked_authenticity_token(cookies)
+      expect(cookies['_csrf_token'][:value]).to eq token
+    end
+
+    it "initializes the _csrf_token cookie without explicit domain by default" do
+      masking_secrets.masked_authenticity_token(cookies)
+      expect(cookies['_csrf_token']).not_to be_has_key(:domain)
+    end
+
+    it "initializes the _csrf_token cookie without explicit httponly by default" do
+      masking_secrets.masked_authenticity_token(cookies)
+      expect(cookies['_csrf_token']).not_to be_has_key(:httponly)
+    end
+
+    it "initializes the _csrf_token cookie without explicit secure by default" do
+      masking_secrets.masked_authenticity_token(cookies)
+      expect(cookies['_csrf_token']).not_to be_has_key(:secure)
+    end
+
+    it "copies domain option to _csrf_token cookie" do
+      masking_secrets.masked_authenticity_token(cookies, domain: "domain")
+      expect(cookies['_csrf_token'][:domain]).to eq "domain"
+    end
+
+    it "copies httponly option to _csrf_token cookie" do
+      masking_secrets.masked_authenticity_token(cookies, httponly: true)
+      expect(cookies['_csrf_token'][:httponly]).to be true
+    end
+
+    it "copies secure option to _csrf_token cookie" do
+      masking_secrets.masked_authenticity_token(cookies, secure: true)
+      expect(cookies['_csrf_token'][:secure]).to be true
+    end
+
+    it "remasks an existing _csrf_token cookie" do
+      original_token = masking_secrets.masked_token(SecureRandom.base64(32))
+      cookies['_csrf_token'] = original_token
+      token = masking_secrets.masked_authenticity_token(cookies)
+      expect(token).not_to eq original_token
+      expect(cookies['_csrf_token'][:value]).to eq token
+      expect(masking_secrets.valid_authenticity_token?({}, {'_csrf_token' => token}, original_token)).to be true
+    end
+  end
+end

lib/authentication_methods.rb

@@ -83,9 +83,18 @@ module AuthenticationMethods
     end
   end
 
+  def masked_authenticity_token
+    session_options = CanvasRails::Application.config.session_options
+    options = session_options.slice(:domain, :secure)
+    options[:httponly] = HostUrl.is_file_host?(request.host_with_port)
+    CanvasBreachMitigation::MaskingSecrets.masked_authenticity_token(cookies, options)
+  end
+  private :masked_authenticity_token
+
   def load_user
     @current_user = @current_pseudonym = nil
-    CanvasBreachMitigation::MaskingSecrets.masked_authenticity_token(cookies) # ensure that the cookie is set
+
+    masked_authenticity_token # ensure that the cookie is set
 
     load_pseudonym_from_access_token
 

spec/lib/authentication_methods_spec.rb

@@ -49,7 +49,8 @@ describe AuthenticationMethods do
         cas_ticket = CanvasUuid::Uuid.generate_securish_uuid
 
         request = stub(:env => {'encrypted_cookie_store.session_refreshed_at' => 5.minutes.ago},
-                      :format => stub(:json? => false))
+                      :format => stub(:json? => false),
+                      :host_with_port => "")
         controller = RSpec::MockController.new(domain_root_account, request)
         controller.expects(:destroy_session).once
         controller.expects(:redirect_to_login).once
@@ -137,7 +138,8 @@ describe AuthenticationMethods do
   describe "#load_user" do
     before do
       @request = stub(:env => {'encrypted_cookie_store.session_refreshed_at' => 5.minutes.ago},
-                      :format => stub(:json? => false))
+                      :format => stub(:json? => false),
+                      :host_with_port => "")
       @controller = RSpec::MockController.new(nil, @request)
       @controller.stubs(:load_pseudonym_from_access_token)
       @controller.stubs(:api_request?).returns(false)
@@ -174,6 +176,53 @@ describe AuthenticationMethods do
         expect(@controller.instance_variable_get(:@current_user)).to eq @user
         expect(@controller.instance_variable_get(:@current_pseudonym)).to eq @pseudonym
       end
+
+      it "should set the CSRF cookie" do
+        @controller.send(:load_user)
+        expect(@controller.cookies['_csrf_token']).not_to be nil
+      end
+    end
+  end
+
+  describe "#masked_authenticity_token" do
+    before do
+      @request = stub(host_with_port: "")
+      @controller = RSpec::MockController.new(nil, @request)
+      @session_options = {}
+      CanvasRails::Application.config.expects(:session_options).at_least_once.returns(@session_options)
+    end
+
+    it "should not set SSL-only explicitly if session_options doesn't specify" do
+      @controller.send(:masked_authenticity_token)
+      expect(@controller.cookies['_csrf_token']).not_to be_has_key(:secure)
+    end
+
+    it "should set SSL-only if session_options specifies" do
+      @session_options[:secure] = true
+      @controller.send(:masked_authenticity_token)
+      expect(@controller.cookies['_csrf_token'][:secure]).to be true
+    end
+
+    it "should set httponly explicitly false on a non-files host" do
+      @controller.send(:masked_authenticity_token)
+      expect(@controller.cookies['_csrf_token'][:httponly]).to be false
+    end
+
+    it "should set httponly explicitly true on a files host" do
+      HostUrl.expects(:is_file_host?).once.with(@request.host_with_port).returns(true)
+      @controller.send(:masked_authenticity_token)
+      expect(@controller.cookies['_csrf_token'][:httponly]).to be true
+    end
+
+    it "should not set a cookie domain explicitly if session_options doesn't specify" do
+      @controller.send(:masked_authenticity_token)
+      expect(@controller.cookies['_csrf_token']).not_to be_has_key(:domain)
+    end
+
+    it "should set a cookie domain explicitly if session_options specifies" do
+      @session_options[:domain] = "cookie domain"
+      @controller.send(:masked_authenticity_token)
+      expect(@controller.cookies['_csrf_token'][:domain]).to eq @session_options[:domain]
     end
   end
 end
@@ -205,6 +254,8 @@ class RSpec::MockController
     options[:target]
   end
 
-  def cookies; {}; end
+  def cookies
+    @cookies ||= {}
+  end
 end