From b12f667daa270a4e3c656d16f30620ca763f9578 Mon Sep 17 00:00:00 2001 From: Tim Pierce Date: Tue, 13 May 2014 11:06:00 -0400 Subject: [PATCH] 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. --- .../arvados/v1/collections_controller.rb | 39 ++++++++++++++++ services/api/config/application.default.yml | 1 + services/api/config/application.yml.example | 10 +++++ .../arvados/v1/collections_controller_test.rb | 45 +++++++++++++++++++ 4 files changed, 95 insertions(+) diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb index 8db93c36c2..4d0d004508 100644 --- 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 67aa401eca..92c8995cf1 100644 --- 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 9162fc4445..89662f551f 100644 --- 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 501c5a1353..33c0155c61 100644 --- 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 -- 2.30.2