Refuse redirects when fetching a key, and answer 400 for a date of the wrong type - #4
Merged
Merged
Conversation
…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
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.