Merge pull request #10 from chef/jdm/v1.3-rfc

Match changes requested in the RFC process for 1.3 signing

2 files changed, 25 insertions, 159 deletions

diff --git a/lib/mixlib/authentication/signedheaderauth.rb b/lib/mixlib/authentication/signedheaderauth.rb
index 8faf313..0b5f923 100644
--- a/lib/mixlib/authentication/signedheaderauth.rb
+++ b/lib/mixlib/authentication/signedheaderauth.rb
@@ -30,14 +30,14 @@ module Mixlib
 
       NULL_ARG = Object.new
 
-      ALGORITHMS_FOR_VERSION = {
-        '1.0' => ['sha1'],
-        '1.1' => ['sha1'],
-        '1.3' => ['sha256', 'sha1'],
+      ALGORITHM_FOR_VERSION = {
+        '1.0' => 'sha1',
+        '1.1' => 'sha1',
+        '1.3' => 'sha256',
       }.freeze()
 
       # Use of SUPPORTED_ALGORITHMS and SUPPORTED_VERSIONS is deprecated. Use
-      # ALGORITHMS_FOR_VERSION instead
+      # ALGORITHM_FOR_VERSION instead
       SUPPORTED_ALGORITHMS = ['sha1'].freeze
       SUPPORTED_VERSIONS = ['1.0', '1.1'].freeze
 
@@ -81,13 +81,12 @@ module Mixlib
                           args[:user_id],
                           args[:file],
                           args[:proto_version],
-                          args[:signing_algorithm],
                           args[:headers]
                          )
       end
 
       def algorithm
-        DEFAULT_SIGN_ALGORITHM
+        ALGORITHM_FOR_VERSION[proto_version] || DEFAULT_SIGN_ALGORITHM
       end
 
       def proto_version
@@ -122,14 +121,14 @@ module Mixlib
       end
 
       def validate_sign_version_digest!(sign_algorithm, sign_version)
-        if ALGORITHMS_FOR_VERSION[sign_version].nil?
+        if ALGORITHM_FOR_VERSION[sign_version].nil?
           raise AuthenticationError,
             "Unsupported version '#{sign_version}'"
         end
 
-        if !ALGORITHMS_FOR_VERSION[sign_version].include?(sign_algorithm)
+        if ALGORITHM_FOR_VERSION[sign_version] != sign_algorithm
           raise AuthenticationError,
-            "Unsupported version '#{sign_version}'"
+            "Unsupported algorithm #{sign_algorithm} for version '#{sign_version}'"
         end
 
         case sign_algorithm
@@ -197,9 +196,9 @@ module Mixlib
         when "1.3"
           [
             "Method:#{http_method.to_s.upcase}",
-            "Hashed Path:#{digester.hash_string(digest, canonical_path)}",
+            "Path:#{canonical_path}",
             "X-Ops-Content-Hash:#{hashed_body(digest)}",
-            "X-Ops-Sign:algorithm=#{sign_algorithm};version=#{sign_version}",
+            "X-Ops-Sign:version=#{sign_version}",
             "X-Ops-Timestamp:#{canonical_time}",
             "X-Ops-UserId:#{canonical_x_ops_user_id}",
             "X-Ops-Server-API-Version:#{server_api_version}",
@@ -217,9 +216,11 @@ module Mixlib
 
       def canonicalize_user_id(user_id, proto_version, digest=OpenSSL::Digest::SHA1)
         case proto_version
-        when "1.1", "1.3"
+        when "1.1"
+          # and 1.2 if that ever gets implemented
           digester.hash_string(digest, user_id)
         else
+          # versions 1.0 and 1.3
           user_id
         end
       end
@@ -264,26 +265,13 @@ module Mixlib
     # provides a more convenient interface to the constructor.
     class SigningObject < Struct.new(:http_method, :path, :body, :host,
                                      :timestamp, :user_id, :file, :proto_version,
-                                     :signing_algorithm, :headers)
+                                     :headers)
       include SignedHeaderAuth
 
       def proto_version
         (self[:proto_version] or DEFAULT_PROTO_VERSION).to_s
       end
 
-      def algorithm
-        if self[:signing_algorithm]
-          self[:signing_algorithm]
-        else
-          case proto_version
-          when '1.3'
-            ALGORITHMS_FOR_VERSION[proto_version].first
-          else
-            DEFAULT_SIGN_ALGORITHM
-          end
-        end
-      end
-
       def server_api_version
         key = (self[:headers] || {}).keys.select do |k|
           k.downcase == 'x-ops-server-api-version'
diff --git a/spec/mixlib/authentication/mixlib_authentication_spec.rb b/spec/mixlib/authentication/mixlib_authentication_spec.rb
index 41cd7e9..69500a4 100644
--- a/spec/mixlib/authentication/mixlib_authentication_spec.rb
+++ b/spec/mixlib/authentication/mixlib_authentication_spec.rb
@@ -90,22 +90,6 @@ describe "Mixlib::Authentication::SignedHeaderAuth" do
     expect(V1_1_SIGNING_OBJECT.sign(PRIVATE_KEY)).to eq(EXPECTED_SIGN_RESULT_V1_1)
   end
 
-  it "should generate the correct string to sign and signature for version 1.3 with SHA1" do
-    expect(V1_3_SHA1_SIGNING_OBJECT.proto_version).to eq("1.3")
-    expect(V1_3_SHA1_SIGNING_OBJECT.canonicalize_request).to eq(V1_3_SHA1_CANONICAL_REQUEST)
-    expect(V1_3_SHA1_SIGNING_OBJECT.algorithm).to eq("sha1")
-    expect(V1_3_SHA1_SIGNING_OBJECT.server_api_version).to eq("1")
-
-    # If you need to regenerate the constants in this test spec, print out
-    # the results of res.inspect and copy them as appropriate into the
-    # the constants in this file.
-    expect(V1_3_SHA1_SIGNING_OBJECT.sign(PRIVATE_KEY)).to eq(EXPECTED_SIGN_RESULT_V1_3_SHA1)
-  end
-
-  it "should default to server api version 0 for version 1.3" do
-    expect(V1_3_SHA1_SIGNING_OBJECT_API0.server_api_version).to eq('0')
-  end
-
   it "should generate the correct string to sign and signature for version 1.3 with SHA256" do
     expect(V1_3_SHA256_SIGNING_OBJECT.proto_version).to eq("1.3")
     expect(V1_3_SHA256_SIGNING_OBJECT.algorithm).to eq("sha256")
@@ -170,19 +154,6 @@ describe "Mixlib::Authentication::SignatureVerification" do
     expect(res).not_to be_nil
   end
 
-  it "should authenticate a File-containing request V1.3 SHA1 - Merb" do
-    request_params = MERB_REQUEST_PARAMS.clone
-    request_params["file"] =
-      { "size"=>MockFile.length, "content_type"=>"application/octet-stream", "filename"=>"zsh.tar.gz", "tempfile"=>MockFile.new }
-
-    mock_request = MockRequest.new(PATH, request_params, MERB_HEADERS_V1_3_SHA1, "")
-    expect(Time).to receive(:now).at_least(:once).and_return(TIMESTAMP_OBJ)
-
-    service = Mixlib::Authentication::SignatureVerification.new
-    res = service.authenticate_user_request(mock_request, @user_private_key)
-    expect(res).not_to be_nil
-  end
-
   it "should authenticate a File-containing request V1.3 SHA256 - Merb" do
     request_params = MERB_REQUEST_PARAMS.clone
     request_params["file"] =
@@ -208,17 +179,8 @@ describe "Mixlib::Authentication::SignatureVerification" do
     expect(res).not_to be_nil
   end
 
-  it "should authenticate a normal (post body) request v1.3 SHA1 - Merb" do
-    mock_request = MockRequest.new(PATH, MERB_REQUEST_PARAMS, MERB_HEADERS_V1_3_SHA1, BODY)
-    expect(Time).to receive(:now).at_least(:once).and_return(TIMESTAMP_OBJ)
-
-    service = Mixlib::Authentication::SignatureVerification.new
-    res = service.authenticate_user_request(mock_request, @user_private_key)
-    expect(res).not_to be_nil
-  end
-
   it "should authenticate a normal (post body) request v1.3 SHA256 - Merb" do
-    mock_request = MockRequest.new(PATH, MERB_REQUEST_PARAMS, MERB_HEADERS_V1_3_SHA1, BODY)
+    mock_request = MockRequest.new(PATH, MERB_REQUEST_PARAMS, MERB_HEADERS_V1_3_SHA256, BODY)
     expect(Time).to receive(:now).at_least(:once).and_return(TIMESTAMP_OBJ)
 
     service = Mixlib::Authentication::SignatureVerification.new
@@ -307,21 +269,6 @@ describe "Mixlib::Authentication::SignatureVerification" do
     expect(auth_req).to be_a_valid_content_hash
   end
 
-  it "shouldn't authenticate if the signature is wrong for v1.3 SHA1" do
-    headers =  MERB_HEADERS_V1_3_SHA1.dup
-    headers["HTTP_X_OPS_AUTHORIZATION_1"] = "epicfail"
-    mock_request = MockRequest.new(PATH, MERB_REQUEST_PARAMS, headers, BODY)
-    expect(Time).to receive(:now).at_least(:once).and_return(TIMESTAMP_OBJ)
-
-    auth_req = Mixlib::Authentication::SignatureVerification.new
-    res = auth_req.authenticate_user_request(mock_request, @user_private_key)
-    expect(res).to be_nil
-    expect(auth_req).not_to be_a_valid_request
-    expect(auth_req).not_to be_a_valid_signature
-    expect(auth_req).to be_a_valid_timestamp
-    expect(auth_req).to be_a_valid_content_hash
-  end
-
   it "shouldn't authenticate if the signature is wrong for v1.3 SHA256" do
     headers =  MERB_HEADERS_V1_3_SHA256.dup
     headers["HTTP_X_OPS_AUTHORIZATION_1"] = "epicfail"
@@ -340,7 +287,6 @@ end
 
 USER_ID = "spec-user"
 DIGESTED_USER_ID = Base64.encode64(Digest::SHA1.new.digest(USER_ID)).chomp
-DIGESTED_USER_ID_SHA256 = Base64.encode64(Digest::SHA256.new.digest(USER_ID)).chomp
 BODY = "Spec Body"
 HASHED_BODY = "DFteJZPVv6WKdQmMqZUQUumUyRs=" # Base64.encode64(Digest::SHA1.digest("Spec Body")).chomp
 HASHED_BODY_SHA256 = "hDlKNZhIhgso3Fs0S0pZwJ0xyBWtR1RBaeHs1DrzOho="
@@ -348,7 +294,6 @@ TIMESTAMP_ISO8601 = "2009-01-01T12:00:00Z"
 TIMESTAMP_OBJ = Time.parse("Thu Jan 01 12:00:00 -0000 2009")
 PATH = "/organizations/clownco"
 HASHED_CANONICAL_PATH = "YtBWDn1blGGuFIuKksdwXzHU9oE=" # Base64.encode64(Digest::SHA1.digest("/organizations/clownco")).chomp
-HASHED_CANONICAL_PATH_SHA256 = "Z3EsTMw/UBNY9n+q+WBWTJmeVg8hQFbdFzVWRxW4dOA="
 
 V1_0_ARGS = {
   :body => BODY,
@@ -369,20 +314,6 @@ V1_1_ARGS = {
   :proto_version => 1.1
 }
 
-V1_3_ARGS_SHA1 = {
-  :body => BODY,
-  :user_id => USER_ID,
-  :http_method => :post,
-  :timestamp => TIMESTAMP_ISO8601,    # fixed timestamp so we get back the same answer each time.
-  :file => MockFile.new,
-  :path => PATH,
-  :proto_version => '1.3',
-  :signing_algorithm => 'sha1',
-  :headers => {
-    'X-OpS-SeRvEr-ApI-VerSiOn' => '1'
-  }
-}
-
 V1_3_ARGS_SHA256 = {
   :body => BODY,
   :user_id => USER_ID,
@@ -430,22 +361,13 @@ X_OPS_AUTHORIZATION_LINES = [
 "FDlbAG7H8Dmvo+wBxmtNkszhzbBnEYtuwQqT8nM/8A=="
 ]
 
-X_OPS_AUTHORIZATION_LINES_V1_3_SHA1 = [
-  "Dh7xqnM3HabvuPVTsJCvHSWGyipvv0xkF9u7XfomC0tDHBF8wG4kEToRI7/1",
-  "CSa97jlHLQ+VqNq76uy2mxg0PBxPLxPcz+VREJxnxEv+gEEr6MAeMpV97ip0",
-  "VICuUZ3hPIVNl9hIjmaeOnQSbtJZZOIik0g0O+bpd7AQKa/Y7r2jw42D/Kgg",
-  "L/ts6ntD2wKb92iPZ5bEXYIJFKVKb7j10PTcHLxkMWd64Cd7GZAdHHl4z8/t",
-  "VZ5XCe23960z08d2P2I+iYBBCxRCOPwafBvbt0ubls2vecraHQYYXMXovjmV",
-  "Rxh8xRaTfEhpWwZJa1ONVvsldZlvGiHO/jhmRJ9oCA=="
-]
-
 X_OPS_AUTHORIZATION_LINES_V1_3_SHA256 = [
-  "BjR+iTK2eOgwmT2yGqLvE7Fp+VlpRGyL1dVoF2DmhUPO7EVsnxx2s32AmlOw",
-  "EpaACpav8SoB7K4rpOo3gfBm0XAYLnLLWzcec2OQG2O0wxxHiKVn4qWEe7Cs",
-  "RZ903DGM54t4uK75vx6wwoEdZqZe21npsLK+F3oAqnkgp+YXmlYv9Se5tFKB",
-  "0GWM1ibGJMjUIFAm7vxzjcuEvkkKN49MnXeMAAykfymcs74RU6xEKYzzSAyC",
-  "ygkV6xQSapDMp/aY29cVA/1FgZeVMhnFSTjtqBehchZYwXswr0A72A86gID9",
-  "h2QsUpmQJwbOK3bb1GptAnd5IiLzIxtu+vFeY6h4eA=="
+  "FZOmXAyOBAZQV/uw188iBljBJXOm+m8xQ/8KTGLkgGwZNcRFxk1m953XjE3W",
+  "VGy1dFT76KeaNWmPCNtDmprfH2na5UZFtfLIKrPv7xm80V+lzEzTd9WBwsfP",
+  "42dZ9N+V9I5SVfcL/lWrrlpdybfceJC5jOcP5tzfJXWUITwb6Z3Erg3DU3Uh",
+  "H9h9E0qWlYGqmiNCVrBnpe6Si1gU/Jl+rXlRSNbLJ4GlArAPuL976iTYJTzE",
+  "MmbLUIm3JRYi00Yb01IUCCKdI90vUq1HHNtlTEu93YZfQaJwRxXlGkCNwIJe",
+  "fy49QzaCIEu1XiOx5Jn+4GmkrZch/RrK9VzQWXgs+w=="
 ]
 # We expect Mixlib::Authentication::SignedHeaderAuth#sign to return this
 # if passed the BODY above, based on version
@@ -476,19 +398,6 @@ EXPECTED_SIGN_RESULT_V1_1 = {
   "X-Ops-Timestamp"=>TIMESTAMP_ISO8601
 }
 
-EXPECTED_SIGN_RESULT_V1_3_SHA1 = {
-  "X-Ops-Content-Hash"=>X_OPS_CONTENT_HASH,
-  "X-Ops-Userid"=>USER_ID,
-  "X-Ops-Sign"=>"algorithm=sha1;version=1.3;",
-  "X-Ops-Authorization-1"=>X_OPS_AUTHORIZATION_LINES_V1_3_SHA1[0],
-  "X-Ops-Authorization-2"=>X_OPS_AUTHORIZATION_LINES_V1_3_SHA1[1],
-  "X-Ops-Authorization-3"=>X_OPS_AUTHORIZATION_LINES_V1_3_SHA1[2],
-  "X-Ops-Authorization-4"=>X_OPS_AUTHORIZATION_LINES_V1_3_SHA1[3],
-  "X-Ops-Authorization-5"=>X_OPS_AUTHORIZATION_LINES_V1_3_SHA1[4],
-  "X-Ops-Authorization-6"=>X_OPS_AUTHORIZATION_LINES_V1_3_SHA1[5],
-  "X-Ops-Timestamp"=>TIMESTAMP_ISO8601
-}
-
 EXPECTED_SIGN_RESULT_V1_3_SHA256 = {
   "X-Ops-Content-Hash"=>X_OPS_CONTENT_HASH_SHA256,
   "X-Ops-Userid"=>USER_ID,
@@ -518,23 +427,6 @@ MERB_REQUEST_PARAMS = {
   "organization_id"=>"local-test-org", "requesting_actor_id"=>REQUESTING_ACTOR_ID,
 }
 
-MERB_HEADERS_V1_3_SHA1 = {
-  # These are used by signatureverification.
-  "HTTP_HOST"=>"127.0.0.1",
-  "HTTP_X_OPS_SIGN"=>"algorithm=sha1;version=1.3;",
-  "HTTP_X_OPS_REQUESTID"=>"127.0.0.1 1258566194.85386",
-  "HTTP_X_OPS_TIMESTAMP"=>TIMESTAMP_ISO8601,
-  "HTTP_X_OPS_CONTENT_HASH"=>X_OPS_CONTENT_HASH,
-  "HTTP_X_OPS_USERID"=>USER_ID,
-  "HTTP_X_OPS_SERVER_API_VERSION"=>"1",
-  "HTTP_X_OPS_AUTHORIZATION_1"=>X_OPS_AUTHORIZATION_LINES_V1_3_SHA1[0],
-  "HTTP_X_OPS_AUTHORIZATION_2"=>X_OPS_AUTHORIZATION_LINES_V1_3_SHA1[1],
-  "HTTP_X_OPS_AUTHORIZATION_3"=>X_OPS_AUTHORIZATION_LINES_V1_3_SHA1[2],
-  "HTTP_X_OPS_AUTHORIZATION_4"=>X_OPS_AUTHORIZATION_LINES_V1_3_SHA1[3],
-  "HTTP_X_OPS_AUTHORIZATION_5"=>X_OPS_AUTHORIZATION_LINES_V1_3_SHA1[4],
-  "HTTP_X_OPS_AUTHORIZATION_6"=>X_OPS_AUTHORIZATION_LINES_V1_3_SHA1[5],
-}.merge(OTHER_HEADERS)
-
 MERB_HEADERS_V1_3_SHA256 = {
   # These are used by signatureverification.
   "HTTP_HOST"=>"127.0.0.1",
@@ -690,32 +582,18 @@ X-Ops-UserId:#{DIGESTED_USER_ID}
 EOS
 V1_1_CANONICAL_REQUEST = V1_1_CANONICAL_REQUEST_DATA.chomp
 
-V1_3_SHA1_CANONICAL_REQUEST_DATA = <<EOS
-Method:POST
-Hashed Path:#{HASHED_CANONICAL_PATH}
-X-Ops-Content-Hash:#{HASHED_BODY}
-X-Ops-Sign:algorithm=sha1;version=1.3
-X-Ops-Timestamp:#{TIMESTAMP_ISO8601}
-X-Ops-UserId:#{DIGESTED_USER_ID}
-X-Ops-Server-API-Version:1
-EOS
-V1_3_SHA1_CANONICAL_REQUEST = V1_3_SHA1_CANONICAL_REQUEST_DATA.chomp
-
 V1_3_SHA256_CANONICAL_REQUEST_DATA = <<EOS
 Method:POST
-Hashed Path:#{HASHED_CANONICAL_PATH_SHA256}
+Path:#{PATH}
 X-Ops-Content-Hash:#{HASHED_BODY_SHA256}
-X-Ops-Sign:algorithm=sha256;version=1.3
+X-Ops-Sign:version=1.3
 X-Ops-Timestamp:#{TIMESTAMP_ISO8601}
-X-Ops-UserId:#{DIGESTED_USER_ID_SHA256}
+X-Ops-UserId:#{USER_ID}
 X-Ops-Server-API-Version:1
 EOS
 V1_3_SHA256_CANONICAL_REQUEST = V1_3_SHA256_CANONICAL_REQUEST_DATA.chomp
 
 V1_3_SHA256_SIGNING_OBJECT = Mixlib::Authentication::SignedHeaderAuth.signing_object(V1_3_ARGS_SHA256)
-V1_3_SHA1_SIGNING_OBJECT = Mixlib::Authentication::SignedHeaderAuth.signing_object(V1_3_ARGS_SHA1)
-V1_3_SHA1_SIGNING_OBJECT_API0 = Mixlib::Authentication::SignedHeaderAuth.signing_object(
-  V1_3_ARGS_SHA1.dup.tap {|x| x.delete(:headers)})
 V1_1_SIGNING_OBJECT = Mixlib::Authentication::SignedHeaderAuth.signing_object(V1_1_ARGS)
 V1_0_SIGNING_OBJECT = Mixlib::Authentication::SignedHeaderAuth.signing_object(V1_0_ARGS)
 LONG_SIGNING_OBJECT = Mixlib::Authentication::SignedHeaderAuth.signing_object(LONG_PATH_LONG_USER_ARGS)