Skip to content

Refuse redirects when fetching a key, and answer 400 for a date of the wrong type - #4

Merged
jwrosewell merged 1 commit into
mainfrom
fix/refuse-redirects
Sep 6, 2026
Merged

Refuse redirects when fetching a key, and answer 400 for a date of the wrong type#4
jwrosewell merged 1 commit into
mainfrom
fix/refuse-redirects

Conversation

@jwrosewell

Copy link
Copy Markdown
Contributor

Found in review after the fetch was merged.

The fetch followed redirects, up to five. PHP's http wrapper reopens
whatever Location names, any host and plain http included, so a creator
whose domain answered 302 to some other place would have that other
place's key trusted as its own, and a network attacker able to bend the
creator's DNS, or a creator that was simply misconfigured, could put a
key there and have forgeries verify. The Java port refuses a change of
protocol, so this port was the weaker of the two. follow_location is now
off, so with ignore_errors the 3xx comes back as the response code and
the caller reads the key as unavailable, which is what it is. The stand
in end point learned to answer with a redirect to another host, and the
test proves the key is not fetched from there. The test fails when
redirects are followed again.

The domain check used $ at the end of its pattern, and $ also matches
before a final line feed, so a domain of "51d.es" followed by a line
feed passed and reached the URL. It is \z now, which is what the Python
port's per character check already refused.

publicKeyResponseAt typed its date as ?string, so a request of
?date[]=1, which reaches an application as an array, threw a TypeError
out of what should be a 400, and a bool from a caller without strict
types coerced to "1" and was served as minute one. The parameter is
mixed now, an int is accepted as its digits, and anything else that is
not a string of digits is 400.

The README's fetch example asked a real resolver for creator.invalid and
asserted the answer. RFC 6761 only says resolvers should refuse the
name, and under a TLS intercepting proxy the answer is a block page,
which reads as an unreadable key rather than an unavailable one and
fails the README test. The example now passes a transport that refuses,
so it shows the same shape and makes no request.

Added the two pass check that tells selecting by start from anything
else: the genuine identifier verifies under the published schedule and
reads as not matching when every start is moved a week later.

154 tests, 8699 assertions, up from 151. The plain runner's 142 checks
are unchanged.

Produced with AI assistance after an adversarial review of #3, and needs human review before merging. The redirect fix was proved by mutation: the test fails when follow_location is turned back on.

…e wrong type

Found in review after the fetch was merged.

The fetch followed redirects, up to five. PHP's http wrapper reopens
whatever Location names, any host and plain http included, so a creator
whose domain answered 302 to some other place would have that other
place's key trusted as its own, and a network attacker able to bend the
creator's DNS, or a creator that was simply misconfigured, could put a
key there and have forgeries verify. The Java port refuses a change of
protocol, so this port was the weaker of the two. follow_location is now
off, so with ignore_errors the 3xx comes back as the response code and
the caller reads the key as unavailable, which is what it is. The stand
in end point learned to answer with a redirect to another host, and the
test proves the key is not fetched from there. The test fails when
redirects are followed again.

The domain check used $ at the end of its pattern, and $ also matches
before a final line feed, so a domain of "51d.es" followed by a line
feed passed and reached the URL. It is \z now, which is what the Python
port's per character check already refused.

publicKeyResponseAt typed its date as ?string, so a request of
?date[]=1, which reaches an application as an array, threw a TypeError
out of what should be a 400, and a bool from a caller without strict
types coerced to "1" and was served as minute one. The parameter is
mixed now, an int is accepted as its digits, and anything else that is
not a string of digits is 400.

The README's fetch example asked a real resolver for creator.invalid and
asserted the answer. RFC 6761 only says resolvers should refuse the
name, and under a TLS intercepting proxy the answer is a block page,
which reads as an unreadable key rather than an unavailable one and
fails the README test. The example now passes a transport that refuses,
so it shows the same shape and makes no request.

Added the two pass check that tells selecting by start from anything
else: the genuine identifier verifies under the published schedule and
reads as not matching when every start is moved a week later.

154 tests, 8699 assertions, up from 151. The plain runner's 142 checks
are unchanged.
@jwrosewell
jwrosewell merged commit 2b0835b into main Sep 6, 2026
6 checks passed
@jwrosewell
jwrosewell deleted the fix/refuse-redirects branch September 6, 2026 15:44
jwrosewell added a commit to 51Degrees/pipeline-php-did that referenced this pull request Sep 6, 2026
…ng a key (#13)

The 51Degrees fork of owid-php now carries SWAN-community/owid-php#4,
which stops the fetch following a redirect from a creator domain, so a
creator whose domain answered 302 could no longer have some other host's
key trusted as its own. It also closes the domain check against a
trailing line feed and answers 400 rather than a type error for a date
of the wrong type on the creator side. The pin moves from 247178f4 to
2b0835b6, the fork's main after the sync.

Tests: 125 passed, 2 skipped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant