9278: Ensure locator signatures expire no later than expires_at.

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index df969e3315150b040df8175eb41d6698a82e5236..4a612924b617f9b9b1dc5c4d7507f8113fdb5553 100644 (file)
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -39,7 +39,10 @@ class Collection < ArvadosModel
                 # expose signed_manifest_text as manifest_text in the
                 # API response, and never let clients select the
                 # manifest_text column.
-                'manifest_text' => ['manifest_text'],
+                #
+                # We need expires_at to determine the correct
+                # timestamp in signed_manifest_text.
+                'manifest_text' => ['manifest_text', 'expires_at'],
                 )
   end
 
@@ -213,14 +216,19 @@ class Collection < ArvadosModel
   def signed_manifest_text
     if has_attribute? :manifest_text
       token = current_api_client_authorization.andand.api_token
-      @signed_manifest_text = self.class.sign_manifest manifest_text, token
+      exp = [db_current_time.to_i + Rails.configuration.blob_signature_ttl,
+             expires_at].compact.map(&:to_i).min
+      @signed_manifest_text = self.class.sign_manifest manifest_text, token, exp
     end
   end
 
-  def self.sign_manifest manifest, token
+  def self.sign_manifest manifest, token, exp=nil
+    if exp.nil?
+      exp = db_current_time.to_i + Rails.configuration.blob_signature_ttl
+    end
     signing_opts = {
       api_token: token,
-      expire: db_current_time.to_i + Rails.configuration.blob_signature_ttl,
+      expire: exp,
     }
     m = munge_manifest_locators(manifest) do |match|
       Blob.sign_locator(match[0], signing_opts)

diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index 536d9ca47e8a378f40d15da3058815eb2b000862..91568927ae37654117da4dec7c811882818d0add 100644 (file)
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -307,6 +307,29 @@ class CollectionTest < ActiveSupport::TestCase
     end
   end
 
+  test 'signature expiry does not exceed expires_at' do
+    act_as_user users(:active) do
+      t0 = db_current_time
+      c = Collection.create!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:x\n", name: 'foo')
+      c.update_attributes! expires_at: (t0 + 1.hours)
+      c.reload
+      sig_exp = /\+A[0-9a-f]{40}\@([0-9]+)/.match(c.signed_manifest_text)[1].to_i
+      assert_operator sig_exp.to_i, :<=, (t0 + 1.hours).to_i
+    end
+  end
+
+  test 'far-future expiry date cannot be used to circumvent configured permission ttl' do
+    act_as_user users(:active) do
+      c = Collection.create!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:x\n",
+                             name: 'foo',
+                             expires_at: db_current_time + 1.years)
+      sig_exp = /\+A[0-9a-f]{40}\@([0-9]+)/.match(c.signed_manifest_text)[1].to_i
+      expect_max_sig_exp = db_current_time.to_i + Rails.configuration.blob_signature_ttl
+      assert_operator c.expires_at.to_i, :>, expect_max_sig_exp
+      assert_operator sig_exp.to_i, :<=, expect_max_sig_exp
+    end
+  end
+
   test "create collection with properties" do
     act_as_system_user do
       c = Collection.create(manifest_text: ". acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo\n",