andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Use fast HPACK comparisons when not checking sensitive headers (#9259)

Browse Source 
Motivation:
Constant time comparison functions are used to compare HTTP/2 header
values, even if they are not sensitive.

Modification:
After checking for sensitivity, use fast comparison.

Result: Faster HPACK table reads/writes
This commit is contained in:
Carl Mastrangelo 2019-10-23 23:37:57 -07:00 committed by Norman Maurer
parent 05480c190f
commit 03ad809a3b
5 changed files with 46 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
codec-http2/src/main/java/io/netty/handler/codec/http2/HpackEncoder.java
View File

@ -41,6 +41,7 @@ import java.util.Arrays;
import java.util.Map;
import static io.netty.handler.codec.http2.HpackUtil.equalsConstantTime;
import static io.netty.handler.codec.http2.HpackUtil.equalsVariableTime;
import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_HEADER_TABLE_SIZE;
import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_HEADER_LIST_SIZE;
import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_HEADER_TABLE_SIZE;
@ -53,6 +54,13 @@ import static io.netty.util.internal.MathUtil.findNextPositivePowerOfTwo;
import static java.lang.Math.max;
import static java.lang.Math.min;
/**
* An HPACK encoder.
*
* <p>Implementation note: This class is security sensitive, and depends on users correctly identifying their headers
* as security sensitive or not. If a header is considered not sensitive, methods names "insensitive" are used which
* are fast, but don't provide any security guarantees.
*/
final class HpackEncoder {
static final int HUFF_CODE_THRESHOLD = 512;
// a linked hash map of header fields
@ -153,7 +161,7 @@ final class HpackEncoder {
// If the peer will only use the static table
if (maxHeaderTableSize == 0) {
int staticTableIndex = HpackStaticTable.getIndex(name, value);
int staticTableIndex = HpackStaticTable.getIndexInsensitive(name, value);
if (staticTableIndex == -1) {
int nameIndex = HpackStaticTable.getIndex(name);
encodeLiteral(out, name, value, IndexType.NONE, nameIndex);
@ -170,13 +178,13 @@ final class HpackEncoder {
return;
}
HeaderEntry headerField = getEntry(name, value);
HeaderEntry headerField = getEntryInsensitive(name, value);
if (headerField != null) {
int index = getIndex(headerField.index) + HpackStaticTable.length;
// Section 6.1. Indexed Header Field Representation
encodeInteger(out, 0x80, 7, index);
} else {
int staticTableIndex = HpackStaticTable.getIndex(name, value);
int staticTableIndex = HpackStaticTable.getIndexInsensitive(name, value);
if (staticTableIndex != -1) {
// Section 6.1. Indexed Header Field Representation
encodeInteger(out, 0x80, 7, staticTableIndex);
@ -351,15 +359,16 @@ final class HpackEncoder {
* Returns the header entry with the lowest index value for the header field. Returns null if
* header field is not in the dynamic table.
*/
private HeaderEntry getEntry(CharSequence name, CharSequence value) {
private HeaderEntry getEntryInsensitive(CharSequence name, CharSequence value) {
if (length() == 0 || name == null || value == null) {
return null;
}
int h = AsciiString.hashCode(name);
int i = index(h);
for (HeaderEntry e = headerFields[i]; e != null; e = e.next) {
// To avoid short circuit behavior a bitwise operator is used instead of a boolean operator.
if (e.hash == h && (equalsConstantTime(name, e.name) & equalsConstantTime(value, e.value)) != 0) {
// Check the value before then name, as it is more likely the value will be different incase there is no
// match.
if (e.hash == h && equalsVariableTime(value, e.value) && equalsVariableTime(name, e.name)) {
return e;
}
}

20
codec-http2/src/main/java/io/netty/handler/codec/http2/HpackHeaderField.java
View File

@ -31,6 +31,7 @@
*/
package io.netty.handler.codec.http2;
import static io.netty.handler.codec.http2.HpackUtil.equalsVariableTime;
import static java.util.Objects.requireNonNull;
class HpackHeaderField {
@ -57,23 +58,8 @@ class HpackHeaderField {
return name.length() + value.length() + HEADER_ENTRY_OVERHEAD;
}
@Override
public final int hashCode() {
// TODO(nmittler): Netty's build rules require this. Probably need a better implementation.
return super.hashCode();
}
@Override
public final boolean equals(Object obj) {
if (obj == this) {
return true;
}
if (!(obj instanceof HpackHeaderField)) {
return false;
}
HpackHeaderField other = (HpackHeaderField) obj;
// To avoid short circuit behavior a bitwise operator is used instead of a boolean operator.
return (HpackUtil.equalsConstantTime(name, other.name) & HpackUtil.equalsConstantTime(value, other.value)) != 0;
public final boolean equalsForTest(HpackHeaderField other) {
return equalsVariableTime(name, other.name) && equalsVariableTime(value, other.value);
}
@Override

8
codec-http2/src/main/java/io/netty/handler/codec/http2/HpackStaticTable.java
View File

@ -38,6 +38,7 @@ import java.util.Arrays;
import java.util.List;
import static io.netty.handler.codec.http2.HpackUtil.equalsConstantTime;
import static io.netty.handler.codec.http2.HpackUtil.equalsVariableTime;
final class HpackStaticTable {
@ -145,7 +146,7 @@ final class HpackStaticTable {
* Returns the index value for the given header field in the static table. Returns -1 if the
* header field is not in the static table.
*/
static int getIndex(CharSequence name, CharSequence value) {
static int getIndexInsensitive(CharSequence name, CharSequence value) {
int index = getIndex(name);
if (index == -1) {
return -1;
@ -154,10 +155,7 @@ final class HpackStaticTable {
// Note this assumes all entries for a given header field are sequential.
while (index <= length) {
HpackHeaderField entry = getEntry(index);
if (equalsConstantTime(name, entry.name) == 0) {
break;
}
if (equalsConstantTime(value, entry.value) != 0) {
if (equalsVariableTime(name, entry.name) && equalsVariableTime(value, entry.value)) {
return index;
}
index++;

10
codec-http2/src/main/java/io/netty/handler/codec/http2/HpackUtil.java
View File

@ -65,6 +65,16 @@ final class HpackUtil {
return ConstantTimeUtils.equalsConstantTime(s1, s2);
}
/**
* Compare two {@link CharSequence}s.
* @param s1 the first value.
* @param s2 the second value.
* @return {@code false} if not equal. {@code true} if equal.
*/
static boolean equalsVariableTime(CharSequence s1, CharSequence s2) {
return AsciiString.contentEquals(s1, s2);
}
// Section 6.2. Literal Header Field Representation
enum IndexType {
INCREMENTAL, // Section 6.2.1. Literal Header Field with Incremental Indexing

18
codec-http2/src/test/java/io/netty/handler/codec/http2/HpackTestCase.java
View File

@ -102,7 +102,7 @@ final class HpackTestCase {
List<HpackHeaderField> expectedDynamicTable = headerBlock.getDynamicTable();
if (!expectedDynamicTable.equals(actualDynamicTable)) {
if (!headersEqual(expectedDynamicTable, actualDynamicTable)) {
throw new AssertionError(
"\nEXPECTED DYNAMIC TABLE:\n" + expectedDynamicTable +
"\nACTUAL DYNAMIC TABLE:\n" + actualDynamicTable);
@ -128,7 +128,7 @@ final class HpackTestCase {
expectedHeaders.add(new HpackHeaderField(h.name, h.value));
}
if (!expectedHeaders.equals(actualHeaders)) {
if (!headersEqual(expectedHeaders, actualHeaders)) {
throw new AssertionError(
"\nEXPECTED:\n" + expectedHeaders +
"\nACTUAL:\n" + actualHeaders);
@ -141,7 +141,7 @@ final class HpackTestCase {
List<HpackHeaderField> expectedDynamicTable = headerBlock.getDynamicTable();
if (!expectedDynamicTable.equals(actualDynamicTable)) {
if (!headersEqual(expectedDynamicTable, actualDynamicTable)) {
throw new AssertionError(
"\nEXPECTED DYNAMIC TABLE:\n" + expectedDynamicTable +
"\nACTUAL DYNAMIC TABLE:\n" + actualDynamicTable);
@ -224,6 +224,18 @@ final class HpackTestCase {
return ret.toString();
}
private static boolean headersEqual(List<HpackHeaderField> expected, List<HpackHeaderField> actual) {
if (expected.size() != actual.size()) {
return false;
}
for (int i = 0; i < expected.size(); i++) {
if (!expected.get(i).equalsForTest(actual.get(i))) {
return false;
}
}
return true;
}
static class HeaderBlock {
private int maxHeaderTableSize = -1;
private byte[] encodedBytes;