Skip to content

Commit

Permalink
Fix content corruption in a chunked response on TLS
Browse files Browse the repository at this point in the history
Motivation:

There's a chance of HTTP content corruption when:

- The current connection is TLS;
- The current response is using chunked encoding; and
- The content length is greater than 16378 (16384 - 6).

.. due to a bug in Netty: netty/netty#11792

Modifications:

- Fixed the math mistake in `Http1ObjectEncoder.MAX_TLS_DATA_LENGTH`,
  to prevent `HttpObjectEncoder` from putting its static final `ByteBuf`
  in the first place of `SslHandler`'s internal queue.

Result:

- No more HTTP content conrruption.
  • Loading branch information
trustin committed Oct 25, 2021
1 parent 25f810c commit c7aee88
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 2 deletions.
Expand Up @@ -52,12 +52,13 @@ public abstract class Http1ObjectEncoder implements HttpObjectEncoder {
* <ul>
* <li>16384 - The maximum length of a cleartext TLS record.</li>
* <li>6 - The maximum header length of an HTTP chunk. i.e. "4000\r\n".length()</li>
* <li>2 - The trailing "\r\n".</li>
* </ul>
*
* <p>To be precise, we have a chance of wasting 6 bytes because we may not use chunked encoding,
* <p>To be precise, we have a chance of wasting 8 bytes because we may not use chunked encoding,
* but it is not worth adding complexity to be that precise.
*/
private static final int MAX_TLS_DATA_LENGTH = 16384 - 6;
private static final int MAX_TLS_DATA_LENGTH = 16384 - 8;

/**
* A non-last empty {@link HttpContent}.
Expand Down
@@ -0,0 +1,120 @@
/*
* Copyright 2021 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package com.linecorp.armeria.server;

import static org.assertj.core.api.Assertions.assertThat;

import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingDeque;
import java.util.concurrent.Semaphore;
import java.util.concurrent.ThreadLocalRandom;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.linecorp.armeria.client.ClientFactory;
import com.linecorp.armeria.client.WebClient;
import com.linecorp.armeria.common.Flags;
import com.linecorp.armeria.common.HttpData;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.HttpResponseWriter;
import com.linecorp.armeria.common.HttpStatus;
import com.linecorp.armeria.common.ResponseHeaders;
import com.linecorp.armeria.common.SessionProtocol;
import com.linecorp.armeria.testing.junit5.server.ServerExtension;

class HttpServerTlsCorruptionTest {

private static final Logger logger = LoggerFactory.getLogger(HttpServerTlsCorruptionTest.class);

private static final HttpData content;

static {
final byte[] data = new byte[1048576];
ThreadLocalRandom.current().nextBytes(data);
content = HttpData.wrap(data);
}

@RegisterExtension
static final ServerExtension server = new ServerExtension() {
@Override
protected void configure(ServerBuilder sb) throws Exception {
sb.https(0);
sb.tlsSelfSigned();
sb.service("/", (ctx, req) -> {
// Use a streaming response to generate HTTP chunks.
final HttpResponseWriter res = HttpResponse.streaming();
res.write(ResponseHeaders.of(200));
for (int i = 0; i < content.length();) {
final int chunkSize = Math.min(32768, content.length() - i);
res.write(HttpData.wrap(content.array(), i, chunkSize));
i += chunkSize;
}
res.close();
return res;
});
}
};

/**
* Makes sure Armeria is unaffected by https://github.com/netty/netty/issues/11792
*/
@Test
void test() throws Throwable {
final int numEventLoops = Flags.numCommonWorkers();
final ClientFactory clientFactory = ClientFactory.builder()
.tlsNoVerify()
.maxNumEventLoopsPerEndpoint(numEventLoops)
.maxNumEventLoopsPerHttp1Endpoint(numEventLoops)
.build();
final WebClient client = WebClient.builder(server.uri(SessionProtocol.H1))
.factory(clientFactory)
.build();
final Semaphore semaphore = new Semaphore(numEventLoops);
final BlockingQueue<Throwable> caughtExceptions = new LinkedBlockingDeque<>();
int i = 0;
try {
for (; i < 1000; i++) {
semaphore.acquire();
client.get("/")
.aggregate()
.handle((res, cause) -> {
semaphore.release();
if (cause != null) {
caughtExceptions.add(cause);
}
try {
assertThat(res.status()).isSameAs(HttpStatus.OK);
assertThat(res.content()).isEqualTo(content);
} catch (Throwable t) {
caughtExceptions.add(t);
}
return null;
});

final Throwable cause = caughtExceptions.poll();
if (cause != null) {
throw cause;
}
}
} catch (Throwable cause) {
logger.warn("Received a corrupt response after {} request(s)", i);
throw cause;
}
}
}

0 comments on commit c7aee88

Please sign in to comment.