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);