zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-05-13 16:07:15 +02:00
Code Issues Projects Releases Packages Wiki Activity

Fix EPP quota handling bug

Browse source 
We limit the maximum number of concurrent connections that a client can make the proxy. The quota is implemented as a (thread-safe) map of client certificate hash to available number of connections. When a new connection is made, we decrement the availability counter by one. When the counter hits zero, no more connections can be made and any new connection from the same client is terminated by the proxy.

Currently, the counter is incremented when a connection is terminated, including connections that are terminated *because* the quota is reached (i. e. the connections for which the counter is not decremented because the counter is already zero). This means that the first time the quota is reached, the next connection is dropped, the counter is incremented to 1 and new connections can be made again, bypassing the quota. This process can be repeated to achieve, theoretically, infinite quota.

This CL fixes this bug by only incrementing the counter, upon connection termination, for connections that have decremented the counter in the first place.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=217231593
This commit is contained in:
jianglai 2018-10-15 16:51:43 -07:00 • committed by Ben McIlwain
parent ecdbdbca63
commit 3cfde5d4a1
2 changed files with 7 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

4
java/google/registry/proxy/handler/QuotaHandler.java
View file

@ -156,7 +156,9 @@ public abstract class QuotaHandler extends ChannelInboundHandlerAdapter {
public void channelInactive(ChannelHandlerContext ctx) { public void channelInactive(ChannelHandlerContext ctx) {
// If no reads occurred before the connection is inactive (for example when the handshake // If no reads occurred before the connection is inactive (for example when the handshake
// is not successful), no quota is leased and therefore no return is needed. // is not successful), no quota is leased and therefore no return is needed.
if (quotaResponse != null) { // Note that the quota response can be a failure, in which case no token was leased to us from
// the token store. Consequently no return is necessary.
if (quotaResponse != null && quotaResponse.success()) {
Future<?> unusedFuture = quotaManager.releaseQuota(QuotaRebate.create(quotaResponse)); Future<?> unusedFuture = quotaManager.releaseQuota(QuotaRebate.create(quotaResponse));
} }
ctx.fireChannelInactive(); ctx.fireChannelInactive();

4
javatests/google/registry/proxy/handler/EppQuotaHandlerTest.java
View file

@ -109,7 +109,11 @@ public class EppQuotaHandlerTest {
.thenReturn(QuotaResponse.create(false, clientCertHash, now)); .thenReturn(QuotaResponse.create(false, clientCertHash, now));
OverQuotaException e = OverQuotaException e =
assertThrows(OverQuotaException.class, () -> channel.writeInbound(message)); assertThrows(OverQuotaException.class, () -> channel.writeInbound(message));
ChannelFuture unusedFuture = channel.close();
assertThat(e).hasMessageThat().contains(clientCertHash); assertThat(e).hasMessageThat().contains(clientCertHash);
verify(quotaManager).acquireQuota(QuotaRequest.create(clientCertHash));
// Make sure that quotaManager.releaseQuota() is not called when the channel closes.
verifyNoMoreInteractions(quotaManager);
verify(metrics).registerQuotaRejection("epp", clientCertHash); verify(metrics).registerQuotaRejection("epp", clientCertHash);
verifyNoMoreInteractions(metrics); verifyNoMoreInteractions(metrics);
} }