From 2e8a2d2edf00c86d332ce28791d0b17dac86c7a7 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sun, 6 Sep 2026 16:34:44 +0100 Subject: [PATCH] Refuse redirects when fetching a creator's key Found in the review that followed the Python and PHP fetches, which were modelled on this one and inherited it. The key fetch followed redirects. HttpURLConnection follows one to any other host by default, refusing only a change of protocol, so a creator whose domain answered 302 to some other https host would have that other host'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. There is no legitimate reason for the well known key end point to send a caller off its own host, and this is the one request in the specification whose answer decides whether an identifier is genuine. setInstanceFollowRedirects(false) leaves the 3xx as the response code, which read() already treats as the key being unavailable with the code carried, which is what it is. The stand in end point learned a REDIRECT answer naming where to send the caller. The new test redirects to a second stand in serving the genuine schedule, so following would read as valid, and asserts the answer is unavailable, the creator was asked once, and the second stand in never was. It fails when redirects are followed again. DatedKeyFetchTest: 15 tests, up from 14. --- .../swancommunity/owid/PublicKeyFetch.java | 10 +++++++ .../swancommunity/owid/DatedKeyFetchTest.java | 28 +++++++++++++++++++ .../com/swancommunity/owid/KeyEndPoint.java | 23 ++++++++++++++- 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/swancommunity/owid/PublicKeyFetch.java b/src/main/java/com/swancommunity/owid/PublicKeyFetch.java index 4b7c4b0..47aa86b 100644 --- a/src/main/java/com/swancommunity/owid/PublicKeyFetch.java +++ b/src/main/java/com/swancommunity/owid/PublicKeyFetch.java @@ -237,6 +237,16 @@ private static String read(String url, String domain) null); } connection = (HttpURLConnection) opened; + // Never follow a redirect. HttpURLConnection follows one to + // any other host by default, 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. Left alone, the 3xx is the response code, and the + // check below reads it as the key being unavailable, which it + // is. + connection.setInstanceFollowRedirects(false); connection.setRequestMethod("GET"); connection.setConnectTimeout(CONNECT_TIMEOUT_MILLISECONDS); connection.setReadTimeout(READ_TIMEOUT_MILLISECONDS); diff --git a/src/test/java/com/swancommunity/owid/DatedKeyFetchTest.java b/src/test/java/com/swancommunity/owid/DatedKeyFetchTest.java index 65d6681..65dd0cc 100644 --- a/src/test/java/com/swancommunity/owid/DatedKeyFetchTest.java +++ b/src/test/java/com/swancommunity/owid/DatedKeyFetchTest.java @@ -234,6 +234,34 @@ void anEndPointThatCannotBeReachedIsKeyUnavailable() * end points served PEM a strict parser refused and every offline check * against them failed while the keys and the identifiers were both fine. */ + /** + * A creator whose domain answers with a redirect does not get the key + * at the other end trusted as its own. The other end here serves the + * genuine schedule, so following the redirect would read as valid, + * and refusing it must read as the key being unavailable with the + * request to the other host never made. Without this a network + * attacker able to bend a creator's DNS, or a misconfigured creator, + * could substitute the key and forgeries would verify. + */ + @Test + void aRedirectIsNotFollowed() throws IOException, OwidException { + Owid owid = KeyFixtures.identifier(); + KeyEndPoint elsewhere = endPoint(KeyEndPoint.Answer.SCHEDULE); + KeyEndPoint creator = KeyEndPoint.start( + KeyEndPoint.Answer.REDIRECT, elsewhere.urlFor(owid)); + started.add(creator); + assertEquals(OwidSignatureStatus.KEY_UNAVAILABLE, + PublicKeyFetch.verifyAtUrl(owid, creator.urlFor(owid), ALONE) + .getStatus(), + "a redirect is the key being unavailable, never a key from " + + "wherever it points"); + assertEquals(1, creator.dates().size(), + "the creator was asked once"); + assertTrue(elsewhere.dates().isEmpty(), + "the request that would have gone to the other host was " + + "never made"); + } + @Test void aKeyThatCannotBeReadIsInvalidKey() throws IOException, OwidException { diff --git a/src/test/java/com/swancommunity/owid/KeyEndPoint.java b/src/test/java/com/swancommunity/owid/KeyEndPoint.java index 151f0c3..bab46e5 100644 --- a/src/test/java/com/swancommunity/owid/KeyEndPoint.java +++ b/src/test/java/com/swancommunity/owid/KeyEndPoint.java @@ -66,7 +66,13 @@ enum Answer { SCHEDULE, /** Text shaped like a PEM that no key can be read out of. */ - BROKEN_KEY + BROKEN_KEY, + + /** + * A redirect to somewhere else, which a client must not follow. + * Started with {@link #start(Answer, String)} naming where. + */ + REDIRECT } private final HttpServer server; @@ -87,6 +93,15 @@ static KeyEndPoint start() throws IOException, OwidException { /** Starts an end point serving what the answer says. */ static KeyEndPoint start(final Answer answer) throws IOException, OwidException { + return start(answer, null); + } + + /** + * Starts an end point serving what the answer says, redirecting to + * the URL given where the answer is {@link Answer#REDIRECT}. + */ + static KeyEndPoint start(final Answer answer, final String redirectTo) + throws IOException, OwidException { final PublicKeySchedule schedule = KeyFixtures.schedule(); HttpServer server = HttpServer.create(new InetSocketAddress( InetAddress.getByName("127.0.0.1"), 0), 0); @@ -98,6 +113,12 @@ public void handle(HttpExchange exchange) throws IOException { String date = parameter( exchange.getRequestURI().getRawQuery(), "date"); endPoint.dates.add(date); + if (answer == Answer.REDIRECT) { + exchange.getResponseHeaders().set("Location", redirectTo); + exchange.sendResponseHeaders(302, -1); + exchange.close(); + return; + } String body; try { body = body(schedule, answer, date);