Validate trailers when assigning headers with Headers#[]=.

context/headers.md

@@ -74,12 +74,13 @@ headers.trailer!
 headers.add("etag", '"12345"')
 headers.add("date", Time.now.httpdate)
 
-# These will be silently ignored for security
-headers.add("authorization", "Bearer token") # Ignored - credential leakage risk
-headers.add("connection", "close") # Ignored - hop-by-hop header
+# Each of these will raise InvalidTrailerError for security:
+headers.add("authorization", "Bearer token") # => raises InvalidTrailerError (credential leakage risk)
+headers.add("connection", "close") # => raises InvalidTrailerError (hop-by-hop header)
+headers["content-length"] = "0" # => raises InvalidTrailerError (framing header)
 ```
 
-The trailer security system prevents HTTP request smuggling by restricting which headers can appear in trailers:
+The trailer security system prevents HTTP request smuggling by restricting which headers can appear in trailers. Once `trailer!` has been called, every method which appends fields (`add`, `set`, `[]=` and `merge!`) validates the header against the policy before modifying the headers, and raises {ruby Protocol::HTTP::InvalidTrailerError} if it is not permitted:
 
 **Allowed headers** (return `true` for `trailer?`):
 - `date` - Response generation timestamps.

guides/headers/readme.md

@@ -74,12 +74,13 @@ headers.trailer!
 headers.add("etag", '"12345"')
 headers.add("date", Time.now.httpdate)
 
-# These will be silently ignored for security
-headers.add("authorization", "Bearer token") # Ignored - credential leakage risk
-headers.add("connection", "close") # Ignored - hop-by-hop header
+# Each of these will raise InvalidTrailerError for security:
+headers.add("authorization", "Bearer token") # => raises InvalidTrailerError (credential leakage risk)
+headers.add("connection", "close") # => raises InvalidTrailerError (hop-by-hop header)
+headers["content-length"] = "0" # => raises InvalidTrailerError (framing header)
 ```
 
-The trailer security system prevents HTTP request smuggling by restricting which headers can appear in trailers:
+The trailer security system prevents HTTP request smuggling by restricting which headers can appear in trailers. Once `trailer!` has been called, every method which appends fields (`add`, `set`, `[]=` and `merge!`) validates the header against the policy before modifying the headers, and raises {ruby Protocol::HTTP::InvalidTrailerError} if it is not permitted:
 
 **Allowed headers** (return `true` for `trailer?`):
 - `date` - Response generation timestamps.

lib/protocol/http/error.rb

@@ -68,6 +68,8 @@ class InvalidTrailerError < InvalidHeaderError
 			# @parameter key [String] The trailer key that is invalid.
 			def initialize(key)
 				super("Invalid trailer key: #{key.inspect}")
+
+				@key = key
 			end
 
 			# @attribute [String] key The trailer key that is invalid.

lib/protocol/http/headers.rb

@@ -251,15 +251,12 @@ def extract(keys)
 			# @parameter key [String] the header key.
 			# @parameter value [String] the header value to assign.
 			# @parameter trailer [Boolean] whether this header is being added as a trailer.
+			# @raises [InvalidTrailerError] If the header is not permitted in the trailer section.
 			def add(key, value, trailer: self.trailer?)
 				value = value.to_s
 
 				if trailer
-					policy = @policy[key.downcase]
-
-					if !policy or !policy.trailer?
-						raise InvalidTrailerError, key
-					end
+					validate_trailer!(key, @policy[key.downcase])
 				end
 
 				if @indexed
@@ -273,7 +270,13 @@ def add(key, value, trailer: self.trailer?)
 			#
 			# @parameter key [String] the header key to replace.
 			# @parameter value [String] the header value to assign.
+			# @raises [InvalidTrailerError] If the header is not permitted in the trailer section.
 			def set(key, value)
+				# Validate before deleting, so that a rejected assignment leaves the headers unchanged:
+				if self.trailer?
+					validate_trailer!(key, @policy[key.downcase])
+				end
+
 				self.delete(key)
 				self.add(key, value)
 			end
@@ -284,13 +287,21 @@ def set(key, value)
 			#
 			# @parameter key [String] the header key.
 			# @parameter value [String | Array] the header value to assign.
+			# @raises [InvalidTrailerError] If the header is not permitted in the trailer section.
 			def []=(key, value)
+				policy = @policy[key.downcase]
+
+				# Validate before deleting, so that a rejected assignment leaves the headers unchanged:
+				if self.trailer?
+					validate_trailer!(key, policy)
+				end
+
 				key = key.downcase
 
 				# Delete existing value if any:
 				self.delete(key)
 
-				if policy = @policy[key]
+				if policy
 					unless value.is_a?(policy)
 						value = policy.coerce(value)
 					end
@@ -448,6 +459,19 @@ def delete(key)
 				end
 			end
 
+			# Ensure the given header is permitted in the trailer section.
+			#
+			# Headers which affect framing, routing, connection handling or authorization must not appear in trailers, as they could otherwise be used for request smuggling or to bypass processing that occurred while the header section was being handled.
+			#
+			# @parameter key [String] The header key.
+			# @parameter policy [Class | Boolean | Nil] The policy for the header key, `false` if the header is a simple singleton string, or `nil` if the policy has no entry for the key.
+			# @raises [InvalidTrailerError] If the header is not permitted in the trailer section.
+			protected def validate_trailer!(key, policy)
+				if !policy or !policy.trailer?
+					raise InvalidTrailerError, key
+				end
+			end
+
 			# Merge the value into the hash according to the policy for the given key.
 			# 
 			# @parameter hash [Hash] The hash to merge into.

releases.md

@@ -1,5 +1,11 @@
 # Releases
 
+## Unreleased
+
+  - `Protocol::HTTP::Headers#[]=` now validates trailers in the same way as `#add`, raising `InvalidTrailerError` when a forbidden header (e.g. `content-length`, `transfer-encoding`, `authorization`) is assigned after `#trailer!` has been called.
+  - `Protocol::HTTP::Headers#set` now validates the trailer before deleting the existing header, so a rejected assignment no longer removes the existing value.
+  - `Protocol::HTTP::InvalidTrailerError#key` now returns the rejected header key instead of `nil`.
+
 ## v0.72.0
 
   - Clarified body stream lifecycle and ownership, including the directional semantics of `Protocol::HTTP::Body::Stream#close_read`, `#close_write`, and `#close`, how premature input closure affects the associated HTTP exchange, and ownership of streams passed to `Streamable#call`.

test/protocol/http/headers.rb

@@ -274,6 +274,59 @@
 			headers["Content-Length"] = "1"
 			expect(headers["content-length"]).to be == "1"
 		end
+
+		with "trailer" do
+			it "can assign a permitted trailer" do
+				trailer = headers.trailer!
+
+				headers["ETag"] = "abcd"
+
+				expect(trailer.to_a).to be == [["etag", "abcd"]]
+				expect(headers["etag"]).to be == "abcd"
+			end
+
+			%w[content-length transfer-encoding authorization].each do |key|
+				with "forbidden trailer #{key.inspect}", unique: key do
+					it "can't assign a #{key.inspect} header in the trailer" do
+						headers.trailer!
+
+						expect do
+							headers[key] = "example"
+						end.to raise_exception(Protocol::HTTP::InvalidTrailerError).and(have_attributes(key: be == key))
+
+						expect(headers.trailer.to_a).to be(:empty?)
+						expect(headers).not.to be(:include?, key)
+					end
+				end
+			end
+
+			it "reports the key as given in the error" do
+				headers.trailer!
+
+				expect do
+					headers["Content-Length"] = "1"
+				end.to raise_exception(Protocol::HTTP::InvalidTrailerError, message: be == 'Invalid trailer key: "Content-Length"').and(have_attributes(key: be == "Content-Length"))
+			end
+
+			it "does not delete an existing header when the trailer is rejected" do
+				headers["content-length"] = "1"
+				headers.trailer!
+
+				expect do
+					headers["content-length"] = "999"
+				end.to raise_exception(Protocol::HTTP::InvalidTrailerError)
+
+				expect(headers["content-length"]).to be == "1"
+				expect(headers.trailer.to_a).to be(:empty?)
+			end
+
+			it "can assign a forbidden trailer before the trailer section starts" do
+				headers["content-length"] = "1"
+
+				expect(headers).not.to be(:trailer?)
+				expect(headers["content-length"]).to be == "1"
+			end
+		end
 	end
 
 	with "#add" do
@@ -293,6 +346,26 @@
 
 			expect(headers["accept-encoding"]).to be == ["gzip"]
 		end
+
+		it "can't set a forbidden header in the trailer" do
+			headers.trailer!
+
+			expect do
+				headers.set("content-length", "999")
+			end.to raise_exception(Protocol::HTTP::InvalidTrailerError)
+		end
+
+		it "does not delete an existing header when the trailer is rejected" do
+			headers.set("content-length", "1")
+			headers.trailer!
+
+			expect do
+				headers.set("content-length", "999")
+			end.to raise_exception(Protocol::HTTP::InvalidTrailerError)
+
+			expect(headers["content-length"]).to be == "1"
+			expect(headers.trailer.to_a).to be(:empty?)
+		end
 	end
 
 	with "#extract" do
@@ -339,6 +412,16 @@
 		end
 	end
 
+	with "#merge!" do
+		it "can't merge a forbidden header into the trailer" do
+			headers.trailer!
+
+			expect do
+				headers.merge!("content-length" => "999")
+			end.to raise_exception(Protocol::HTTP::InvalidTrailerError)
+		end
+	end
+
 	with "#trailer!" do
 		it "can add trailer" do
 			headers.add("trailer", "etag")