Merge branch '2755-api-collection-permissions'
authorTim Pierce <twp@curoverse.com>
Wed, 21 May 2014 18:15:01 +0000 (14:15 -0400)
committerTim Pierce <twp@curoverse.com>
Wed, 21 May 2014 19:54:56 +0000 (15:54 -0400)
closes #2784, closes #2787

services/api/app/controllers/arvados/v1/collections_controller.rb
services/api/app/models/locator.rb [new file with mode: 0644]
services/api/config/application.default.yml
services/api/config/application.yml.example
services/api/test/functional/arvados/v1/collections_controller_test.rb

index 8db93c36c2171fa310e6939ae00ddd830dd06ee7..1982a525f7c03366a5174e10d39e9e0a53bd96d0 100644 (file)
@@ -10,6 +10,48 @@ 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.blob_signing_key,
+      api_token: api_token,
+      ttl: Rails.configuration.blob_signing_ttl,
+    }
+    resource_attrs[:manifest_text].lines.each do |entry|
+      entry.split[1..-1].each do |tok|
+        # TODO(twp): in Phase 4, fail the request if the locator
+        # lacks a permission signature. (see #2755)
+        loc = Locator.parse(tok)
+        if loc and loc.signature
+          if !api_token
+            logger.warn "No API token present; cannot verify signature on #{loc}"
+            perms_ok = false
+          elsif !Blob.verify_signature tok, signing_opts
+            logger.warn "Invalid signature on locator #{loc}"
+            perms_ok = false
+          end
+        end
+      end
+    end
+    unless perms_ok
+      raise ArvadosModel::PermissionDeniedError
+    end
+
+    # Remove any permission signatures from the manifest.
+    resource_attrs[:manifest_text]
+      .gsub!(/ [[:xdigit:]]{32}(\+[[:digit:]]+)?(\+\S+)/) { |word|
+      loc = Locator.parse(word)
+      if loc
+        " " + loc.without_signature.to_s
+      else
+        word
+      end
+    }
+
+    # 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
@@ -25,7 +67,6 @@ class Arvados::V1::CollectionsController < ApplicationController
           @object = @existing_object || @object
         end
       end
-
       if @object
         link_attrs = {
           owner_uuid: owner_uuid,
@@ -45,6 +86,22 @@ class Arvados::V1::CollectionsController < ApplicationController
   end
 
   def show
+    if current_api_client_authorization
+      signing_opts = {
+        key: Rails.configuration.blob_signing_key,
+        api_token: current_api_client_authorization.api_token,
+        ttl: Rails.configuration.blob_signing_ttl,
+      }
+      @object[:manifest_text]
+        .gsub!(/ [[:xdigit:]]{32}(\+[[:digit:]]+)?(\+\S+)/) { |word|
+        loc = Locator.parse(word)
+        if loc
+          " " + Blob.sign_locator(word, signing_opts)
+        else
+          word
+        end
+      }
+    end
     render json: @object.as_api_response(:with_data)
   end
 
@@ -214,5 +271,4 @@ class Arvados::V1::CollectionsController < ApplicationController
       end
     end
   end
-
 end
diff --git a/services/api/app/models/locator.rb b/services/api/app/models/locator.rb
new file mode 100644 (file)
index 0000000..39d7da9
--- /dev/null
@@ -0,0 +1,84 @@
+# A Locator is used to parse and manipulate Keep locator strings.
+#
+# Locators obey the following syntax:
+#
+#   locator      ::= address hint*
+#   address      ::= digest size-hint
+#   digest       ::= <32 hexadecimal digits>
+#   size-hint    ::= "+" [0-9]+
+#   hint         ::= "+" hint-type hint-content
+#   hint-type    ::= [A-Z]
+#   hint-content ::= [A-Za-z0-9@_-]+
+#
+# Individual hints may have their own required format:
+# 
+#   sign-hint      ::= "+A" <40 lowercase hex digits> "@" sign-timestamp
+#   sign-timestamp ::= <8 lowercase hex digits>
+
+class Locator
+  def initialize(hasharg, sizearg, hintarg)
+    @hash = hasharg
+    @size = sizearg
+    @hints = hintarg
+  end
+
+  # Locator.parse returns a Locator object parsed from the string tok.
+  # Returns nil if tok could not be parsed as a valid locator.
+  def self.parse(tok)
+    begin
+      Locator.parse!(tok)
+    rescue ArgumentError => e
+      nil
+    end
+  end
+
+  # Locator.parse! returns a Locator object parsed from the string tok,
+  # raising an ArgumentError if tok cannot be parsed.
+  def self.parse!(tok)
+    m = /^([[:xdigit:]]{32})(\+([[:digit:]]+))?(\+([[:upper:]][[:alnum:]+@_-]*))?$/.match(tok.strip)
+    unless m
+      raise ArgumentError.new "could not parse #{tok}"
+    end
+
+    tokhash, _, toksize, _, trailer = m[1..5]
+    tokhints = []
+    if trailer
+      trailer.split('+').each do |hint|
+        if hint =~ /^[[:upper:]][[:alnum:]@_-]+$/
+          tokhints.push(hint)
+        else
+          raise ArgumentError.new "unknown hint #{hint}"
+        end
+      end
+    end
+
+    Locator.new(tokhash, toksize, tokhints)
+  end
+
+  # Returns the signature hint supplied with this locator,
+  # or nil if the locator was not signed.
+  def signature
+    @hints.grep(/^A/).first
+  end
+
+  # Returns an unsigned Locator.
+  def without_signature
+    Locator.new(@hash, @size, @hints.reject { |o| o.start_with?("A") })
+  end
+
+  def hash
+    @hash
+  end
+
+  def size
+    @size
+  end
+
+  def hints
+    @hints
+  end
+
+  def to_s
+    [ @hash, @size, *@hints ].join('+')
+  end
+end
index 67aa401eca2433c3b71da4575b8010af4cc4553e..a3ff6800be23bf336f8741a147c642000d1a69b9 100644 (file)
@@ -43,6 +43,7 @@ test:
 
 common:
   secret_token: ~
+  blob_signing_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
@@ -122,3 +123,7 @@ common:
   # configuration variable so that the primary server can give out the correct
   # address of the dedicated websocket server:
   #websocket_address: wss://127.0.0.1:3333/websocket
+
+  # Amount of time (in seconds) for which a blob permission signature
+  # remains valid.  Default: 2 weeks (1209600 seconds)
+  blob_signing_ttl: 1209600
index 9162fc444585d9fa1e59ade9f0df7e144f81cbda..030e23894f00a5436354d36cf946272bdea616a2 100644 (file)
 # 5. Section in application.default.yml called "common"
 
 development:
+  # The blob_signing_key is a string of alphanumeric characters used
+  # to sign permission hints for Keep locators. It must 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.
+  blob_signing_key: ~
 
 production:
   # At minimum, you need a nice long randomly generated secret_token here.
+  # Use a long string of alphanumeric characters (at least 36).
   secret_token: ~
 
+  # blob_signing_key is required and must be identical to the
+  # permission secret provisioned to Keep.
+  # Use a long string of alphanumeric characters (at least 36).
+  blob_signing_key: ~
+
   uuid_prefix: bogus
 
   # compute_node_domain: example.org
@@ -39,3 +51,4 @@ test:
 common:
   #git_repositories_dir: /var/cache/git
   #git_internal_dir: /var/cache/arvados/internal.git
+
index 501c5a13531be673df921d495dfb3fe30905bd93..afda91cc026d98e0d1f7d4f704b6028c70d009ab 100644 (file)
@@ -220,4 +220,220 @@ 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.blob_signing_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
+
+  test "create collection with signed manifest and explicit TTL" 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.blob_signing_key,
+      api_token: api_token(:active),
+      ttl: 3600   # 1 hour
+    }
+    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
+
+  test "create fails with invalid signature" do
+    authorize_with :active
+    signing_opts = {
+      key: Rails.configuration.blob_signing_key,
+      api_token: api_token(:active),
+    }
+
+    # Generate a locator with a bad signature.
+    unsigned_locator = "d41d8cd98f00b204e9800998ecf8427e+0"
+    bad_locator = unsigned_locator + "+Affffffff@ffffffff"
+    assert !Blob.verify_signature(bad_locator, signing_opts)
+
+    # Creating a collection with this locator should
+    # produce 403 Permission denied.
+    unsigned_manifest = ". #{unsigned_locator} 0:0:foo.txt\n"
+    manifest_uuid = Digest::MD5.hexdigest(unsigned_manifest) +
+      '+' +
+      unsigned_manifest.length.to_s
+
+    bad_manifest = ". #{bad_locator} 0:0:foo.txt\n"
+    post :create, {
+      collection: {
+        manifest_text: bad_manifest,
+        uuid: manifest_uuid
+      }
+    }
+
+    assert_response 403
+  end
+
+  test "create fails with uuid of signed manifest" do
+    authorize_with :active
+    signing_opts = {
+      key: Rails.configuration.blob_signing_key,
+      api_token: api_token(:active),
+    }
+
+    unsigned_locator = "d41d8cd98f00b204e9800998ecf8427e+0"
+    signed_locator = Blob.sign_locator(unsigned_locator, signing_opts)
+    signed_manifest = ". #{signed_locator} 0:0:foo.txt\n"
+    manifest_uuid = Digest::MD5.hexdigest(signed_manifest) +
+      '+' +
+      signed_manifest.length.to_s
+
+    post :create, {
+      collection: {
+        manifest_text: signed_manifest,
+        uuid: manifest_uuid
+      }
+    }
+
+    assert_response 422
+  end
+
+  test "multiple locators per line" do
+    authorize_with :active
+    locators = %w(
+      d41d8cd98f00b204e9800998ecf8427e+0
+      acbd18db4cc2f85cedef654fccc4a4d8+3
+      ea10d51bcf88862dbcc36eb292017dfd+45)
+
+    manifest_text = [".", *locators, "0:0:foo.txt\n"].join(" ")
+    manifest_uuid = Digest::MD5.hexdigest(manifest_text) +
+      '+' +
+      manifest_text.length.to_s
+
+    post :create, {
+      collection: {
+        manifest_text: manifest_text,
+        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']
+    assert_equal resp['manifest_text'], manifest_text
+  end
+
+  test "multiple signed locators per line" do
+    authorize_with :active
+    locators = %w(
+      d41d8cd98f00b204e9800998ecf8427e+0
+      acbd18db4cc2f85cedef654fccc4a4d8+3
+      ea10d51bcf88862dbcc36eb292017dfd+45)
+
+    signing_opts = {
+      key: Rails.configuration.blob_signing_key,
+      api_token: api_token(:active),
+    }
+
+    unsigned_manifest = [".", *locators, "0:0:foo.txt\n"].join(" ")
+    manifest_uuid = Digest::MD5.hexdigest(unsigned_manifest) +
+      '+' +
+      unsigned_manifest.length.to_s
+
+    signed_locators = locators.map { |loc| Blob.sign_locator loc, signing_opts }
+    signed_manifest = [".", *signed_locators, "0:0:foo.txt\n"].join(" ")
+
+    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.
+    # Each line is of the form "path locator locator ... 0:0:file.txt"
+    # entry.split[1..-2] will yield just the tokens in the middle of the line
+    returned_locator_count = 0
+    resp['manifest_text'].lines.each do |entry|
+      entry.split[1..-2].each do |tok|
+        returned_locator_count += 1
+        assert Blob.verify_signature tok, signing_opts
+      end
+    end
+    assert_equal locators.count, returned_locator_count
+  end
 end