From 171298abe176b52a9b95503b3a2469349648e998 Mon Sep 17 00:00:00 2001 From: Bob Hageman Date: Thu, 31 Aug 2023 08:42:41 +0200 Subject: [PATCH 1/4] feat: implement ratelimiting --- .../email/CleanupBackgroundJob.java | 43 +++++ .../privacybydesign/email/EmailRestApi.java | 46 ++++- .../email/ratelimit/MemoryRateLimit.java | 165 ++++++++++++++++++ .../email/ratelimit/RateLimit.java | 76 ++++++++ .../privacybydesign/email/RateLimitTest.java | 46 +++++ 5 files changed, 373 insertions(+), 3 deletions(-) create mode 100644 src/main/java/foundation/privacybydesign/email/CleanupBackgroundJob.java create mode 100644 src/main/java/foundation/privacybydesign/email/ratelimit/MemoryRateLimit.java create mode 100644 src/main/java/foundation/privacybydesign/email/ratelimit/RateLimit.java create mode 100644 src/test/java/foundation/privacybydesign/email/RateLimitTest.java diff --git a/src/main/java/foundation/privacybydesign/email/CleanupBackgroundJob.java b/src/main/java/foundation/privacybydesign/email/CleanupBackgroundJob.java new file mode 100644 index 0000000..d9db4eb --- /dev/null +++ b/src/main/java/foundation/privacybydesign/email/CleanupBackgroundJob.java @@ -0,0 +1,43 @@ +package foundation.privacybydesign.email; + +import foundation.privacybydesign.email.ratelimit.MemoryRateLimit; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import jakarta.servlet.ServletContextEvent; +import jakarta.servlet.ServletContextListener; +import jakarta.servlet.annotation.WebListener; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +/** + * Clean up in-memory data structures once in a while. + */ +@WebListener +public class CleanupBackgroundJob implements ServletContextListener { + private static Logger logger = LoggerFactory.getLogger(CleanupBackgroundJob.class); + private ScheduledExecutorService scheduler; + + @Override + public void contextInitialized(ServletContextEvent event) { + logger.info("Setting up background cleanup task"); + scheduler = Executors.newSingleThreadScheduledExecutor(); + + scheduler.scheduleAtFixedRate(new Runnable() { + @Override public void run() { + try { + MemoryRateLimit.getInstance().periodicCleanup(); + } catch (Exception e) { + logger.error("Failed to run periodic cleanup:"); + e.printStackTrace(); + } + } + }, 5, 5, TimeUnit.MINUTES); + } + + @Override + public void contextDestroyed(ServletContextEvent event) { + scheduler.shutdownNow(); + } +} diff --git a/src/main/java/foundation/privacybydesign/email/EmailRestApi.java b/src/main/java/foundation/privacybydesign/email/EmailRestApi.java index eb0ee2c..e935cc2 100644 --- a/src/main/java/foundation/privacybydesign/email/EmailRestApi.java +++ b/src/main/java/foundation/privacybydesign/email/EmailRestApi.java @@ -8,8 +8,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import foundation.privacybydesign.email.ratelimit.MemoryRateLimit; +import foundation.privacybydesign.email.ratelimit.RateLimit; import jakarta.mail.internet.AddressException; +import jakarta.servlet.http.HttpServletRequest; import jakarta.ws.rs.*; +import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import java.io.UnsupportedEncodingException; @@ -26,11 +30,14 @@ @Path("") public class EmailRestApi { private static Logger logger = LoggerFactory.getLogger(EmailRestApi.class); + private static RateLimit rateLimiter = MemoryRateLimit.getInstance(); private static final String ERR_ADDRESS_MALFORMED = "error:email-address-malformed"; private static final String ERR_INVALID_TOKEN = "error:invalid-token"; private static final String ERR_INVALID_LANG = "error:invalid-language"; private static final String OK_RESPONSE = "OK"; // value doesn't really matter + private static final String PROXY_IP_HEADER = "X-Real-IP"; + private static final String ERR_RATE_LIMITED = "error:ratelimit"; private EmailTokens signer; @@ -42,7 +49,8 @@ public EmailRestApi() { @POST @Path("/send-email") @Produces(MediaType.TEXT_PLAIN) - public Response sendEmail(@FormParam("email") String email, + public Response sendEmail(@Context HttpServletRequest req, + @FormParam("email") String email, @FormParam("language") String lang, @HeaderParam("Authorization") String auth) { EmailConfiguration conf = EmailConfiguration.getInstance(); @@ -58,8 +66,24 @@ public Response sendEmail(@FormParam("email") String email, return Response.status(Response.Status.BAD_REQUEST).entity(ERR_ADDRESS_MALFORMED).build(); } - String token = signer.createToken(email); try { + String ip = req.getHeader(PROXY_IP_HEADER); + if (ip == null) { + ip = req.getRemoteAddr(); + } + + long retryAfter = rateLimiter.rateLimited(ip, email); + if (retryAfter > 0) { + // 429 Too Many Requests + // https://tools.ietf.org/html/rfc6585#section-4 + return Response.status(429) + .entity(ERR_RATE_LIMITED) + .header("Retry-After", (int) Math.ceil(retryAfter / 1000.0)) + .build(); + } + + String token = signer.createToken(email); + String url = conf.getServerURL(lang) + "#verify-email/" + token + "/" + URLEncoder.encode(client.getReturnURL(), StandardCharsets.UTF_8.toString()); EmailSender.send( @@ -93,7 +117,8 @@ public Response sendEmail(@FormParam("email") String email, @POST @Path("/send-email-token") @Produces(MediaType.TEXT_PLAIN) - public Response sendEmailToken(@FormParam("email") String emailAddress, + public Response sendEmailToken(@Context HttpServletRequest req, + @FormParam("email") String emailAddress, @FormParam("language") String language) { EmailConfiguration conf = EmailConfiguration.getInstance(); @@ -103,6 +128,21 @@ public Response sendEmailToken(@FormParam("email") String emailAddress, return Response.status(Response.Status.BAD_REQUEST).entity(ERR_ADDRESS_MALFORMED).build(); } + String ip = req.getHeader(PROXY_IP_HEADER); + if (ip == null) { + ip = req.getRemoteAddr(); + } + + long retryAfter = rateLimiter.rateLimited(ip, emailAddress); + if (retryAfter > 0) { + // 429 Too Many Requests + // https://tools.ietf.org/html/rfc6585#section-4 + return Response.status(429) + .entity(ERR_RATE_LIMITED) + .header("Retry-After", (int) Math.ceil(retryAfter / 1000.0)) + .build(); + } + // Test email with signature String token = signer.createToken(emailAddress); diff --git a/src/main/java/foundation/privacybydesign/email/ratelimit/MemoryRateLimit.java b/src/main/java/foundation/privacybydesign/email/ratelimit/MemoryRateLimit.java new file mode 100644 index 0000000..9115ffc --- /dev/null +++ b/src/main/java/foundation/privacybydesign/email/ratelimit/MemoryRateLimit.java @@ -0,0 +1,165 @@ +package foundation.privacybydesign.email.ratelimit; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Store rate limits in memory. Useful for debugging and rate limits that + * aren't very long. + * + * How it works: + * How much budget a user has, is expressed in a timestamp. The timestamp is + * initially some period in the past, but with every usage (countIP and + * countPhone) this timestamp is incremented. For IP address counting, this + * is a fixed amount (currently 10 seconds), but for phone numbers this + * amount is exponential. + * + * An algorithm with a similar goal is the Token Bucket algorithm. This + * algorithm probably works well, but seemed harder to implement. + * https://en.wikipedia.org/wiki/Token_bucket + */ +public class MemoryRateLimit extends RateLimit { + private static final long SECOND = 1000; // 1000ms = 1s + private static final long MINUTE = SECOND * 60; + private static final long HOUR = MINUTE * 60; + private static final long DAY = HOUR * 24; + private static final int IP_TIMEOUT = 10 * 1000; // timeout in seconds + private static final int IP_TRIES = 3; // number of tries on first visit + + private static MemoryRateLimit instance; + + private final Map ipLimits; + private final Map emailLimits; + + public MemoryRateLimit() { + ipLimits = new ConcurrentHashMap<>(); + emailLimits = new ConcurrentHashMap<>(); + } + + public static MemoryRateLimit getInstance() { + if (instance == null) { + instance = new MemoryRateLimit(); + } + return instance; + } + + private long startLimitIP(long now) { + return now - IP_TIMEOUT*IP_TRIES; + } + + @Override + protected synchronized long nextTryIP(String ip, long now) { + // Allow at most 1 try in each period (TIMEOUT), but kick in only + // after 3 tries. Thus while the user can do only 1 try per period + // over longer periods, the initial budget is 3 periods. + long limit = 0; // First try - last try was "long in the past". + if (ipLimits.containsKey(ip)) { + // Ah, there was a request before. + limit = ipLimits.get(ip); + } + + long startLimit = startLimitIP(now); + if (limit < startLimit) { + // First visit or previous visit was long ago. + // Act like the last try was 3 periods ago. + limit = startLimit; + } + + // Add a period to the current limit. + limit += IP_TIMEOUT; + return limit; + } + + @Override + protected synchronized void countIP(String ip, long now) { + long nextTry = nextTryIP(ip, now); + if (nextTry > now) { + throw new IllegalStateException("counting rate limit while over the limit"); + } + ipLimits.put(ip, nextTry); + } + + // Is the user over the rate limit per e-mail address? + @Override + protected synchronized long nextTryEmail(String email, long now) { + // Rate limiter durations (sort-of logarithmic): + // 1 10 second + // 2 5 minute + // 3 1 hour + // 4 24 hour + // 5+ 1 per day + // Keep log 5 days for proper limiting. + + Limit limit = emailLimits.get(email); + if (limit == null) { + limit = new Limit(now); + emailLimits.put(email, limit); + } + long nextTry; // timestamp when the next request is allowed + switch (limit.tries) { + case 0: // try 1: always succeeds + nextTry = limit.timestamp; + break; + case 1: // try 2: allowed after 10 seconds + nextTry = limit.timestamp + 10 * SECOND; + break; + case 2: // try 3: allowed after 5 minutes + nextTry = limit.timestamp + 5 * MINUTE; + break; + case 3: // try 4: allowed after 3 hours + nextTry = limit.timestamp + 3 * HOUR; + break; + case 4: // try 5: allowed after 24 hours + nextTry = limit.timestamp + 24 * HOUR; + break; + default: + throw new IllegalStateException("invalid tries count"); + } + return nextTry; + } + + // Count the usage of this rate limit - adding to the budget for this + // e-mail address. + @Override + protected synchronized void countEmail(String email, long now) { + long nextTry = nextTryEmail(email, now); + Limit limit = emailLimits.get(email); + if (nextTry > now) { + throw new IllegalStateException("counting rate limit while over the limit"); + } + limit.tries = Math.min(limit.tries+1, 5); // add 1, max at 5 + // If the last usage was e.g. ≥2 days ago, we should allow them 2 tries + // extra tries this day. + long lastTryDaysAgo = (now-limit.timestamp)/DAY; + long bonusTries = limit.tries - lastTryDaysAgo; + if (bonusTries >= 1) { + limit.tries = (int)bonusTries; + } + limit.timestamp = now; + } + + public void periodicCleanup() { + long now = System.currentTimeMillis(); + // Use enhanced for loop, because an iterator makes sure concurrency issues cannot occur. + for (Map.Entry entry : ipLimits.entrySet()) { + if (entry.getValue() < startLimitIP(now)) { + ipLimits.remove(entry.getKey()); + } + } + for (Map.Entry entry : emailLimits.entrySet()) { + if (entry.getValue().timestamp < now - 5*DAY) { + emailLimits.remove(entry.getKey()); + } + } + } +} + +class Limit { + long timestamp; + int tries; + + Limit(long now) { + tries = 0; + timestamp = now; + } +} diff --git a/src/main/java/foundation/privacybydesign/email/ratelimit/RateLimit.java b/src/main/java/foundation/privacybydesign/email/ratelimit/RateLimit.java new file mode 100644 index 0000000..6703b9d --- /dev/null +++ b/src/main/java/foundation/privacybydesign/email/ratelimit/RateLimit.java @@ -0,0 +1,76 @@ +package foundation.privacybydesign.email.ratelimit; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.net.InetAddress; +import java.net.UnknownHostException; + +/** + * Base class for rate limiting. Subclasses provide storage methods (memory + * for easier debugging and database for production). + */ +public abstract class RateLimit { + private static Logger logger = LoggerFactory.getLogger(RateLimit.class); + + /** Take an IP address and an e-mail address and rate limit them. + * @param remoteAddr IP address (IPv4 or IPv6 in any format) + * @param email e-mail address + * @return the number of milliseconds that the client should wait - 0 if + * it shouldn't wait. + */ + public long rateLimited(String remoteAddr, String email) { + String addr = getAddressPrefix(remoteAddr); + long now = System.currentTimeMillis(); + long ipRetryAfter = nextTryIP(addr, now); + long phoneRetryAfter = nextTryEmail(email, now); + long retryAfter = Math.max(ipRetryAfter, phoneRetryAfter); + if (retryAfter > now) { + logger.warn("Denying request from {}: rate limit (ip and/or email) exceeded", addr); + // Don't count this request if it has been denied. + return retryAfter - now; + } + countIP(addr, now); + countEmail(email, now); + return 0; + } + + /** Insert an IP address (IPv4 or IPv6) and get a canonicalized version. + * For IPv6, also truncate to /56 (recommended residential block). + * + * This is a public method to ease testing. + */ + public static String getAddressPrefix(String remoteAddr) { + byte[] rawAddr; + try { + InetAddress addr = InetAddress.getByName(remoteAddr); + rawAddr = addr.getAddress(); + } catch (UnknownHostException e) { + // Shouldn't be possible - we're using IP addresses here, not + // host names. + throw new RuntimeException("host name lookup on IP address?"); + } + if (rawAddr.length == 4) { // IPv4 + // take the whole IP address + } else if (rawAddr.length == 16) { // IPv6 + // Use only the first /56 bytes, set the rest to 0. + for (int i=7; i<16; i++) { + rawAddr[i] = 0; + } + } else { + // I hope this will never happen. + throw new RuntimeException("no IPv4 or IPv6?"); + } + try { + return InetAddress.getByAddress(rawAddr).getHostAddress(); + } catch (UnknownHostException e) { + // Should Not Happen™ + throw new RuntimeException("host name lookup on IP address?"); + } + } + + protected abstract long nextTryIP(String ip, long now); + protected abstract long nextTryEmail(String email, long now); + protected abstract void countIP(String ip, long now); + protected abstract void countEmail(String email, long now); +} diff --git a/src/test/java/foundation/privacybydesign/email/RateLimitTest.java b/src/test/java/foundation/privacybydesign/email/RateLimitTest.java new file mode 100644 index 0000000..95380b5 --- /dev/null +++ b/src/test/java/foundation/privacybydesign/email/RateLimitTest.java @@ -0,0 +1,46 @@ +package foundation.privacybydesign.email; + +import org.junit.Test; + +import foundation.privacybydesign.email.ratelimit.RateLimit; + +import static junit.framework.TestCase.assertFalse; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * Test whether the canonicalization algorithm for IP addresses works well. + */ +public class RateLimitTest { + @Test + public void testIPv4Address() { + assertEquals("1.2.3.4", + RateLimit.getAddressPrefix("1.2.3.4")); + } + + @Test + public void testIPv6AddressLocal() { + assertEquals("0:0:0:0:0:0:0:0", + RateLimit.getAddressPrefix("0:0:0:0:0:0:0:1")); + } + + @Test + public void testIPv6AddressRegular() { + assertEquals("2a00:123:4567:8900:0:0:0:0", + RateLimit.getAddressPrefix("2a00:0123:4567:89ab:cdef:fedc:ba98:7654")); + } + + @Test + public void testIPv6Equals() { + String addr1 = RateLimit.getAddressPrefix("2a00:0123:4567:89ab:cdef:fedc:ba98:7654"); + String addr2 = RateLimit.getAddressPrefix("2a00:0123:4567:89ac:cdef:fedc:ba98:7654"); + assertTrue("IPv6 addresses should match", addr1.equals(addr2)); + } + + @Test + public void testIPv6Unequals() { + String addr1 = RateLimit.getAddressPrefix("2a00:0123:4567:89ab:cdef:fedc:ba98:7654"); + String addr2 = RateLimit.getAddressPrefix("2a00:0123:4567:88ab:cdef:fedc:ba98:7654"); + assertFalse("IPv6 addresses should not match", addr1.equals(addr2)); + } +} From 1161e6f1a7f5f6980f06b4a512f5bee23ac03671 Mon Sep 17 00:00:00 2001 From: Bob Hageman Date: Thu, 31 Aug 2023 09:22:31 +0200 Subject: [PATCH 2/4] feat: show ratelimiting errors in ui --- webapp/common.js | 40 ++++++++++++++++++++++++++++++++++++---- webapp/en/messages.js | 9 ++++++++- webapp/nl/messages.js | 9 ++++++++- 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/webapp/common.js b/webapp/common.js index 97bb0c0..42c2ce0 100644 --- a/webapp/common.js +++ b/webapp/common.js @@ -80,14 +80,46 @@ function addEmail(e) { $('#email-form input').prop('disabled', false).val(''); }) .fail(function(e) { - // Address format problem? - setStatus('danger', MESSAGES[e.responseText] || MESSAGES['unknown-problem']); + var errormsg = e.responseText; + console.error('failed to submit email address:', errormsg); + + if (!errormsg || !MESSAGES[errormsg]) { + errormsg = 'error:internal'; + } + + if (errormsg == 'error:ratelimit') { + var retryAfter = e.getResponseHeader('Retry-After'); + + // In JavaScript, we can mostly ignore the fact we're dealing + // with a string here and treat it as an integer... + if (retryAfter < 60) { + var timemsg = MESSAGES['seconds']; + if (retryAfter == 1) { + var timemsg = MESSAGES['second']; + } + } else if (retryAfter < 60*60) { + retryAfter = Math.round(retryAfter / 60); + var timemsg = MESSAGES['minutes']; + if (retryAfter == 1) { + var timemsg = MESSAGES['minute']; + } + } else { + retryAfter = Math.round(retryAfter / 60 / 60); + var timemsg = MESSAGES['hours']; + if (retryAfter == 1) { + var timemsg = MESSAGES['hour']; + } + } + setStatus('danger', MESSAGES[errormsg].replace('%time%', + timemsg.replace('%n%', retryAfter))); + } else { + setStatus('danger', MESSAGES[errormsg]); + } + setWindow('email-add'); // Make editable again $('#email-form input').prop('disabled', false); - - console.error('fail', e.responseText); }); } diff --git a/webapp/en/messages.js b/webapp/en/messages.js index 8714cb7..89b7c7f 100644 --- a/webapp/en/messages.js +++ b/webapp/en/messages.js @@ -6,7 +6,6 @@ var MESSAGES = { 'sending-verification-email': 'An e-mail is being sent to:
', 'sent-verification-email': 'An e-mail has been sent to:
', 'verifying-email-token': 'E-mail address is being verified ...', - 'unknown-problem': 'Unknown problem', 'error:invalid-token': 'Link in the e-mail is out-of-date or invalid', 'email-failed-to-verify': 'Unknown problem during verification of the e-mail address', 'email-add-verified': 'E-mail address verified', @@ -17,4 +16,12 @@ var MESSAGES = { 'submit-email-add': 'Add address', 'submit-email-confirm': 'Confirm', 'issuers-overview-page': 'https://privacybydesign.foundation/issuance/', + 'error:internal': 'Internal error. Please contact Yivi if this happens more often.', + 'error:ratelimit': 'Please try again in %time%.', + 'second': '%n% seconds', + 'seconds': '%n% seconds', + 'minute': '%n% minute', + 'minutes': '%n% minutes', + 'hour': '%n% hour', + 'hours': '%n% hour', }; diff --git a/webapp/nl/messages.js b/webapp/nl/messages.js index 514a21e..84401b3 100644 --- a/webapp/nl/messages.js +++ b/webapp/nl/messages.js @@ -6,7 +6,6 @@ var MESSAGES = { 'sending-verification-email': 'Een e-mail wordt gestuurd naar:
', 'sent-verification-email': 'Er is een e-mail gestuurd naar:
', 'verifying-email-token': 'Het e-mailadres wordt geverifieerd...', - 'unknown-problem': 'Onbekend probleem', 'error:invalid-token': 'De link in de e-mail is verouderd of ongeldig', 'email-failed-to-verify': 'Onbekend probleem tijdens het verifiëren van het e-mail adres', 'email-add-verified': 'Het e-mailadres geverifieerd', @@ -17,4 +16,12 @@ var MESSAGES = { 'submit-email-add': 'Voeg adres toe', 'submit-email-confirm': 'Bevestig', 'issuers-overview-page': 'https://privacybydesign.foundation/uitgifte/', + 'error:internal': 'Interne fout. Neem contact op met Yivi als dit vaker voorkomt.', + 'error:ratelimit': 'Probeer het opnieuw over %time%.', + 'second': '%n% seconde', + 'seconds': '%n% seconden', + 'minute': '%n% minuut', + 'minutes': '%n% minuten', + 'hour': '%n% uur', + 'hours': '%n% uur', }; From bf65c51a51c120e8a37be41ebf1e7d6cecc2a8e0 Mon Sep 17 00:00:00 2001 From: Bob Hageman Date: Wed, 6 Sep 2023 09:27:37 +0200 Subject: [PATCH 3/4] improve: remove ip ratelimiting, set limits more tolerable --- .../privacybydesign/email/EmailRestApi.java | 14 +--- .../email/ratelimit/MemoryRateLimit.java | 83 +++++-------------- .../email/ratelimit/RateLimit.java | 54 ++---------- .../privacybydesign/email/RateLimitTest.java | 39 ++------- 4 files changed, 37 insertions(+), 153 deletions(-) diff --git a/src/main/java/foundation/privacybydesign/email/EmailRestApi.java b/src/main/java/foundation/privacybydesign/email/EmailRestApi.java index e935cc2..c065f97 100644 --- a/src/main/java/foundation/privacybydesign/email/EmailRestApi.java +++ b/src/main/java/foundation/privacybydesign/email/EmailRestApi.java @@ -36,7 +36,6 @@ public class EmailRestApi { private static final String ERR_INVALID_TOKEN = "error:invalid-token"; private static final String ERR_INVALID_LANG = "error:invalid-language"; private static final String OK_RESPONSE = "OK"; // value doesn't really matter - private static final String PROXY_IP_HEADER = "X-Real-IP"; private static final String ERR_RATE_LIMITED = "error:ratelimit"; private EmailTokens signer; @@ -67,12 +66,8 @@ public Response sendEmail(@Context HttpServletRequest req, } try { - String ip = req.getHeader(PROXY_IP_HEADER); - if (ip == null) { - ip = req.getRemoteAddr(); - } - long retryAfter = rateLimiter.rateLimited(ip, email); + long retryAfter = rateLimiter.rateLimited(email); if (retryAfter > 0) { // 429 Too Many Requests // https://tools.ietf.org/html/rfc6585#section-4 @@ -128,12 +123,7 @@ public Response sendEmailToken(@Context HttpServletRequest req, return Response.status(Response.Status.BAD_REQUEST).entity(ERR_ADDRESS_MALFORMED).build(); } - String ip = req.getHeader(PROXY_IP_HEADER); - if (ip == null) { - ip = req.getRemoteAddr(); - } - - long retryAfter = rateLimiter.rateLimited(ip, emailAddress); + long retryAfter = rateLimiter.rateLimited(emailAddress); if (retryAfter > 0) { // 429 Too Many Requests // https://tools.ietf.org/html/rfc6585#section-4 diff --git a/src/main/java/foundation/privacybydesign/email/ratelimit/MemoryRateLimit.java b/src/main/java/foundation/privacybydesign/email/ratelimit/MemoryRateLimit.java index 9115ffc..cd463ec 100644 --- a/src/main/java/foundation/privacybydesign/email/ratelimit/MemoryRateLimit.java +++ b/src/main/java/foundation/privacybydesign/email/ratelimit/MemoryRateLimit.java @@ -9,9 +9,8 @@ * * How it works: * How much budget a user has, is expressed in a timestamp. The timestamp is - * initially some period in the past, but with every usage (countIP and - * countPhone) this timestamp is incremented. For IP address counting, this - * is a fixed amount (currently 10 seconds), but for phone numbers this + * initially some period in the past, but with every usage (countEmail) + * this timestamp is incremented. For e-mail addresses this * amount is exponential. * * An algorithm with a similar goal is the Token Bucket algorithm. This @@ -23,16 +22,12 @@ public class MemoryRateLimit extends RateLimit { private static final long MINUTE = SECOND * 60; private static final long HOUR = MINUTE * 60; private static final long DAY = HOUR * 24; - private static final int IP_TIMEOUT = 10 * 1000; // timeout in seconds - private static final int IP_TRIES = 3; // number of tries on first visit private static MemoryRateLimit instance; - private final Map ipLimits; private final Map emailLimits; public MemoryRateLimit() { - ipLimits = new ConcurrentHashMap<>(); emailLimits = new ConcurrentHashMap<>(); } @@ -43,53 +38,17 @@ public static MemoryRateLimit getInstance() { return instance; } - private long startLimitIP(long now) { - return now - IP_TIMEOUT*IP_TRIES; - } - - @Override - protected synchronized long nextTryIP(String ip, long now) { - // Allow at most 1 try in each period (TIMEOUT), but kick in only - // after 3 tries. Thus while the user can do only 1 try per period - // over longer periods, the initial budget is 3 periods. - long limit = 0; // First try - last try was "long in the past". - if (ipLimits.containsKey(ip)) { - // Ah, there was a request before. - limit = ipLimits.get(ip); - } - - long startLimit = startLimitIP(now); - if (limit < startLimit) { - // First visit or previous visit was long ago. - // Act like the last try was 3 periods ago. - limit = startLimit; - } - - // Add a period to the current limit. - limit += IP_TIMEOUT; - return limit; - } - - @Override - protected synchronized void countIP(String ip, long now) { - long nextTry = nextTryIP(ip, now); - if (nextTry > now) { - throw new IllegalStateException("counting rate limit while over the limit"); - } - ipLimits.put(ip, nextTry); - } - // Is the user over the rate limit per e-mail address? @Override protected synchronized long nextTryEmail(String email, long now) { // Rate limiter durations (sort-of logarithmic): - // 1 10 second - // 2 5 minute - // 3 1 hour - // 4 24 hour - // 5+ 1 per day - // Keep log 5 days for proper limiting. - + // 1 10 seconds + // 2 1 minute + // 3 5 minutes + // 4 1 hour + // 5 12 hours + // 6+ 2 per day + // Keep log 2 days for proper limiting. Limit limit = emailLimits.get(email); if (limit == null) { limit = new Limit(now); @@ -103,14 +62,17 @@ protected synchronized long nextTryEmail(String email, long now) { case 1: // try 2: allowed after 10 seconds nextTry = limit.timestamp + 10 * SECOND; break; - case 2: // try 3: allowed after 5 minutes + case 2: // try 3: allowed after 1 minute + nextTry = limit.timestamp + MINUTE; + break; + case 3: // try 4: allowed after 5 minutes nextTry = limit.timestamp + 5 * MINUTE; break; - case 3: // try 4: allowed after 3 hours - nextTry = limit.timestamp + 3 * HOUR; + case 4: // try 5: allowed after 1 hour + nextTry = limit.timestamp + HOUR; break; - case 4: // try 5: allowed after 24 hours - nextTry = limit.timestamp + 24 * HOUR; + case 5: // try 6: allowed after 12 hours + nextTry = limit.timestamp + 12 * HOUR; break; default: throw new IllegalStateException("invalid tries count"); @@ -127,8 +89,8 @@ protected synchronized void countEmail(String email, long now) { if (nextTry > now) { throw new IllegalStateException("counting rate limit while over the limit"); } - limit.tries = Math.min(limit.tries+1, 5); // add 1, max at 5 - // If the last usage was e.g. ≥2 days ago, we should allow them 2 tries + limit.tries = Math.min(limit.tries+1, 6); // add 1, max at 6 + // If the last usage was e.g. ≥2 days ago, we should allow them 2 // extra tries this day. long lastTryDaysAgo = (now-limit.timestamp)/DAY; long bonusTries = limit.tries - lastTryDaysAgo; @@ -141,13 +103,8 @@ protected synchronized void countEmail(String email, long now) { public void periodicCleanup() { long now = System.currentTimeMillis(); // Use enhanced for loop, because an iterator makes sure concurrency issues cannot occur. - for (Map.Entry entry : ipLimits.entrySet()) { - if (entry.getValue() < startLimitIP(now)) { - ipLimits.remove(entry.getKey()); - } - } for (Map.Entry entry : emailLimits.entrySet()) { - if (entry.getValue().timestamp < now - 5*DAY) { + if (entry.getValue().timestamp < now - 2*DAY) { emailLimits.remove(entry.getKey()); } } diff --git a/src/main/java/foundation/privacybydesign/email/ratelimit/RateLimit.java b/src/main/java/foundation/privacybydesign/email/ratelimit/RateLimit.java index 6703b9d..9e31dfa 100644 --- a/src/main/java/foundation/privacybydesign/email/ratelimit/RateLimit.java +++ b/src/main/java/foundation/privacybydesign/email/ratelimit/RateLimit.java @@ -3,9 +3,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.net.InetAddress; -import java.net.UnknownHostException; - /** * Base class for rate limiting. Subclasses provide storage methods (memory * for easier debugging and database for production). @@ -13,64 +10,25 @@ public abstract class RateLimit { private static Logger logger = LoggerFactory.getLogger(RateLimit.class); - /** Take an IP address and an e-mail address and rate limit them. - * @param remoteAddr IP address (IPv4 or IPv6 in any format) + /** Take an e-mail address and rate limit it. * @param email e-mail address * @return the number of milliseconds that the client should wait - 0 if * it shouldn't wait. */ - public long rateLimited(String remoteAddr, String email) { - String addr = getAddressPrefix(remoteAddr); + public long rateLimited(String email) { long now = System.currentTimeMillis(); - long ipRetryAfter = nextTryIP(addr, now); - long phoneRetryAfter = nextTryEmail(email, now); - long retryAfter = Math.max(ipRetryAfter, phoneRetryAfter); + long retryAfter = nextTryEmail(email, now); + if (retryAfter > now) { - logger.warn("Denying request from {}: rate limit (ip and/or email) exceeded", addr); + logger.warn("Denying request: email rate limit email exceeded"); // Don't count this request if it has been denied. return retryAfter - now; } - countIP(addr, now); + countEmail(email, now); return 0; } - /** Insert an IP address (IPv4 or IPv6) and get a canonicalized version. - * For IPv6, also truncate to /56 (recommended residential block). - * - * This is a public method to ease testing. - */ - public static String getAddressPrefix(String remoteAddr) { - byte[] rawAddr; - try { - InetAddress addr = InetAddress.getByName(remoteAddr); - rawAddr = addr.getAddress(); - } catch (UnknownHostException e) { - // Shouldn't be possible - we're using IP addresses here, not - // host names. - throw new RuntimeException("host name lookup on IP address?"); - } - if (rawAddr.length == 4) { // IPv4 - // take the whole IP address - } else if (rawAddr.length == 16) { // IPv6 - // Use only the first /56 bytes, set the rest to 0. - for (int i=7; i<16; i++) { - rawAddr[i] = 0; - } - } else { - // I hope this will never happen. - throw new RuntimeException("no IPv4 or IPv6?"); - } - try { - return InetAddress.getByAddress(rawAddr).getHostAddress(); - } catch (UnknownHostException e) { - // Should Not Happen™ - throw new RuntimeException("host name lookup on IP address?"); - } - } - - protected abstract long nextTryIP(String ip, long now); protected abstract long nextTryEmail(String email, long now); - protected abstract void countIP(String ip, long now); protected abstract void countEmail(String email, long now); } diff --git a/src/test/java/foundation/privacybydesign/email/RateLimitTest.java b/src/test/java/foundation/privacybydesign/email/RateLimitTest.java index 95380b5..c8aceb8 100644 --- a/src/test/java/foundation/privacybydesign/email/RateLimitTest.java +++ b/src/test/java/foundation/privacybydesign/email/RateLimitTest.java @@ -2,45 +2,24 @@ import org.junit.Test; -import foundation.privacybydesign.email.ratelimit.RateLimit; +import foundation.privacybydesign.email.ratelimit.MemoryRateLimit; -import static junit.framework.TestCase.assertFalse; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNotEquals; /** - * Test whether the canonicalization algorithm for IP addresses works well. + * Test whether the rate limiter works as expected. */ public class RateLimitTest { - @Test - public void testIPv4Address() { - assertEquals("1.2.3.4", - RateLimit.getAddressPrefix("1.2.3.4")); - } @Test - public void testIPv6AddressLocal() { - assertEquals("0:0:0:0:0:0:0:0", - RateLimit.getAddressPrefix("0:0:0:0:0:0:0:1")); - } + public void testRateLimit() { - @Test - public void testIPv6AddressRegular() { - assertEquals("2a00:123:4567:8900:0:0:0:0", - RateLimit.getAddressPrefix("2a00:0123:4567:89ab:cdef:fedc:ba98:7654")); - } + var rateLimit = MemoryRateLimit.getInstance(); + var email = "test@test.nl"; - @Test - public void testIPv6Equals() { - String addr1 = RateLimit.getAddressPrefix("2a00:0123:4567:89ab:cdef:fedc:ba98:7654"); - String addr2 = RateLimit.getAddressPrefix("2a00:0123:4567:89ac:cdef:fedc:ba98:7654"); - assertTrue("IPv6 addresses should match", addr1.equals(addr2)); - } - - @Test - public void testIPv6Unequals() { - String addr1 = RateLimit.getAddressPrefix("2a00:0123:4567:89ab:cdef:fedc:ba98:7654"); - String addr2 = RateLimit.getAddressPrefix("2a00:0123:4567:88ab:cdef:fedc:ba98:7654"); - assertFalse("IPv6 addresses should not match", addr1.equals(addr2)); + assertEquals("not rate limited", 0, rateLimit.rateLimited(email)); + + assertNotEquals("rate limited", 0, rateLimit.rateLimited(email)); } } From 9b30f65de985850e8706137a749a15813d22d799 Mon Sep 17 00:00:00 2001 From: Bob Hageman Date: Wed, 6 Sep 2023 09:38:21 +0200 Subject: [PATCH 4/4] chore: remove unused parameter --- .../foundation/privacybydesign/email/EmailRestApi.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/main/java/foundation/privacybydesign/email/EmailRestApi.java b/src/main/java/foundation/privacybydesign/email/EmailRestApi.java index c065f97..3bacbe8 100644 --- a/src/main/java/foundation/privacybydesign/email/EmailRestApi.java +++ b/src/main/java/foundation/privacybydesign/email/EmailRestApi.java @@ -11,9 +11,7 @@ import foundation.privacybydesign.email.ratelimit.MemoryRateLimit; import foundation.privacybydesign.email.ratelimit.RateLimit; import jakarta.mail.internet.AddressException; -import jakarta.servlet.http.HttpServletRequest; import jakarta.ws.rs.*; -import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import java.io.UnsupportedEncodingException; @@ -48,8 +46,7 @@ public EmailRestApi() { @POST @Path("/send-email") @Produces(MediaType.TEXT_PLAIN) - public Response sendEmail(@Context HttpServletRequest req, - @FormParam("email") String email, + public Response sendEmail(@FormParam("email") String email, @FormParam("language") String lang, @HeaderParam("Authorization") String auth) { EmailConfiguration conf = EmailConfiguration.getInstance(); @@ -112,8 +109,7 @@ public Response sendEmail(@Context HttpServletRequest req, @POST @Path("/send-email-token") @Produces(MediaType.TEXT_PLAIN) - public Response sendEmailToken(@Context HttpServletRequest req, - @FormParam("email") String emailAddress, + public Response sendEmailToken(@FormParam("email") String emailAddress, @FormParam("language") String language) { EmailConfiguration conf = EmailConfiguration.getInstance();