2755: Verify permission signatures on create.

Phase 1 of #2755: when creating a new collection, verify any permission
signatures found in the manifest.  Unsigned locators in the manifest are
implicitly permitted (to be disabled in Phase 4)

* Collections.create checks permission signatures in a manifest.
* Collections.show signs locators in a manifest.
* Unit test test_create_collection_with_signed_manifest exercises the
  'create' and 'show' methods on a signed manifest.
* application.default.yml, application.yml.example: added configuration
  variable Rails.configuration.permission_key.

diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 8db93c36c2171fa310e6939ae00ddd830dd06ee7..4d0d0045084df882da519bd383d0f902909a91ae 100644 (file)
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -10,6 +10,35 @@ class Arvados::V1::CollectionsController < ApplicationController
       logger.warn "User #{current_user.andand.uuid} tried to set collection owner_uuid to #{owner_uuid}"
       raise ArvadosModel::PermissionDeniedError
     end
+
+    # Check permissions on the collection manifest.
+    # If any signature cannot be verified, return 403 Permission denied.
+    perms_ok = true
+    api_token = current_api_client_authorization.andand.api_token
+    signing_opts = { key: Rails.configuration.permission_key, api_token: api_token }
+    resource_attrs[:manifest_text].lines.each do |entry|
+      # TODO(twp): fail the request if this match fails.
+      # Add in Phase 4 (see #2755)
+      m = /([[:xdigit:]]{32}(\+[[:digit:]]+)?)(\+A\S*)?/.match(entry)
+      if m and m[3]
+        if !api_token
+          logger.warn "No API token present; cannot verify signature #{m[0]}"
+          perms_ok = false
+        elsif !Blob.verify_signature m[0], signing_opts
+          logger.warn "Invalid signature on locator #{m[0]}"
+          perms_ok = false
+        end
+      end
+    end
+    unless perms_ok
+      raise ArvadosModel::PermissionDeniedError
+    end
+
+    # Remove any permission signatures from the manifest.
+    resource_attrs[:manifest_text]
+      .gsub!(/^(\S+\s+)([[:xdigit:]]{32}(\+[[:digit:]]+)?)(\+A\S*)/, '\1\2')
+
+    # Save the collection with the stripped manifest.
     act_as_system_user do
       @object = model_class.new resource_attrs.reject { |k,v| k == :owner_uuid }
       begin
@@ -45,6 +74,16 @@ class Arvados::V1::CollectionsController < ApplicationController
   end
 
   def show
+    if current_api_client_authorization
+      signing_opts = {
+        key: Rails.configuration.permission_key,
+        api_token: current_api_client_authorization.api_token,
+      }
+      @object[:manifest_text]
+        .gsub!(/^(\S+\s+)([[:xdigit:]]{32}(\+[[:digit:]]+)?)/) { |m|
+        $1 + Blob.sign_locator($2, signing_opts)
+      }
+    end
     render json: @object.as_api_response(:with_data)
   end
 

diff --git a/services/api/config/application.default.yml b/services/api/config/application.default.yml
index 67aa401eca2433c3b71da4575b8010af4cc4553e..92c8995cf1396062d5e1dea7888564ae3dcd548c 100644 (file)
--- a/services/api/config/application.default.yml
+++ b/services/api/config/application.default.yml
@@ -43,6 +43,7 @@ test:
 
 common:
   secret_token: ~
+  permission_key: ~
   uuid_prefix: <%= Digest::MD5.hexdigest(`hostname`).to_i(16).to_s(36)[0..4] %>
 
   # Git repositories must be readable by api server, or you won't be

diff --git a/services/api/config/application.yml.example b/services/api/config/application.yml.example
index 9162fc444585d9fa1e59ade9f0df7e144f81cbda..89662f551f30a861e15ece5914314c87fb5b5afb 100644 (file)
--- a/services/api/config/application.yml.example
+++ b/services/api/config/application.yml.example
@@ -11,11 +11,20 @@
 # 5. Section in application.default.yml called "common"
 
 development:
+  # The permission_key needs to be identical to the
+  # permission key given to Keep.  If you run both
+  # apiserver and Keep in development, change this to
+  # a hardcoded string and make sure both systems use
+  # the same value.
+  permission_key: <%= rand(2**512).to_s(36) %>
 
 production:
   # At minimum, you need a nice long randomly generated secret_token here.
   secret_token: ~
 
+  # The permission key must be identical to the key provisioned to Keep.
+  permission_key: ~
+
   uuid_prefix: bogus
 
   # compute_node_domain: example.org
@@ -35,6 +44,7 @@ production:
 test:
   uuid_prefix: zzzzz
   secret_token: <%= rand(2**512).to_s(36) %>
+  permission_key: <%= rand(2**512).to_s(36) %>
 
 common:
   #git_repositories_dir: /var/cache/git

diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index 501c5a13531be673df921d495dfb3fe30905bd93..33c0155c616513bd36c8d95b48aba7f1b9033323 100644 (file)
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -220,4 +220,49 @@ EOS
     assert_equal true, !!found.index('1f4b0bc7583c2a7f9102c395f4ffc5e3+45')
   end
 
+  test "create collection with signed manifest" do
+    authorize_with :active
+    locators = %w(
+      d41d8cd98f00b204e9800998ecf8427e+0
+      acbd18db4cc2f85cedef654fccc4a4d8+3
+      ea10d51bcf88862dbcc36eb292017dfd+45)
+
+    unsigned_manifest = locators.map { |loc|
+      ". " + loc + " 0:0:foo.txt\n"
+    }.join()
+    manifest_uuid = Digest::MD5.hexdigest(unsigned_manifest) +
+      '+' +
+      unsigned_manifest.length.to_s
+
+    # build a manifest with both signed and unsigned locators.
+    # TODO(twp): in phase 4, all locators will need to be signed, so
+    # this test should break and will need to be rewritten. Issue #2755.
+    signing_opts = {
+      key: Rails.configuration.permission_key,
+      api_token: api_token(:active),
+    }
+    signed_manifest =
+      ". " + locators[0] + " 0:0:foo.txt\n" +
+      ". " + Blob.sign_locator(locators[1], signing_opts) + " 0:0:foo.txt\n" +
+      ". " + Blob.sign_locator(locators[2], signing_opts) + " 0:0:foo.txt\n"
+
+    post :create, {
+      collection: {
+        manifest_text: signed_manifest,
+        uuid: manifest_uuid,
+      }
+    }
+    assert_response :success
+    assert_not_nil assigns(:object)
+    resp = JSON.parse(@response.body)
+    assert_equal manifest_uuid, resp['uuid']
+    assert_equal 48, resp['data_size']
+    # All of the locators in the output must be signed.
+    resp['manifest_text'].lines.each do |entry|
+      m = /([[:xdigit:]]{32}\+\S+)/.match(entry)
+      if m
+        assert Blob.verify_signature m[0], signing_opts
+      end
+    end
+  end
 end