Remove dummy PQ padding extension.
Results written up at https://www.imperialviolet.org/2018/04/11/pqconftls.html
Change-Id: I4614fbda555323c67a7ee4683441b59b995f97fb
Reviewed-on: https://boringssl-review.googlesource.com/31064
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 85b244b..6b0d9bb 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3004,26 +3004,6 @@
OPENSSL_EXPORT const char *SSL_get_psk_identity(const SSL *ssl);
-// Dummy post-quantum padding.
-//
-// Dummy post-quantum padding invovles the client (and later server) sending
-// useless, random-looking bytes in an extension in their ClientHello or
-// ServerHello. These extensions are sized to simulate a post-quantum
-// key-exchange and so enable measurement of the latency impact of the
-// additional bandwidth.
-
-// SSL_set_dummy_pq_padding_size enables the sending of a dummy PQ padding
-// extension and configures its size. This is only effective for a client: a
-// server will echo an extension with one of equal length when we get to that
-// phase of the experiment. It returns one for success and zero otherwise.
-OPENSSL_EXPORT int SSL_set_dummy_pq_padding_size(SSL *ssl, size_t num_bytes);
-
-// SSL_dummy_pq_padding_used returns one if the server echoed a dummy PQ padding
-// extension and zero otherwise. It may only be called on a client connection
-// once the ServerHello has been processed, otherwise it'll return zero.
-OPENSSL_EXPORT int SSL_dummy_pq_padding_used(SSL *ssl);
-
-
// QUIC Transport Parameters.
//
// draft-ietf-quic-tls defines a new TLS extension quic_transport_parameters
diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h
index e395852..0a3e9e4 100644
--- a/include/openssl/tls1.h
+++ b/include/openssl/tls1.h
@@ -240,9 +240,6 @@
// This is not an IANA defined extension number
#define TLSEXT_TYPE_channel_id 30032
-// This is not an IANA defined extension number
-#define TLSEXT_TYPE_dummy_pq_padding 54537
-
// status request value from RFC 3546
#define TLSEXT_STATUSTYPE_nothing (-1)
#define TLSEXT_STATUSTYPE_ocsp 1
diff --git a/ssl/internal.h b/ssl/internal.h
index 3c47a20..f886070 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1588,11 +1588,6 @@
// grease_seed is the entropy for GREASE values. It is valid if
// |grease_seeded| is true.
uint8_t grease_seed[ssl_grease_last_index + 1] = {0};
-
- // dummy_pq_padding_len, in a server, is the length of the extension that
- // should be echoed in a ServerHello, or zero if no extension should be
- // echoed.
- uint16_t dummy_pq_padding_len = 0;
};
UniquePtr<SSL_HANDSHAKE> ssl_handshake_new(SSL *ssl);
@@ -2413,7 +2408,6 @@
// |client_CA|.
STACK_OF(X509_NAME) *cached_x509_client_CA = nullptr;
- uint16_t dummy_pq_padding_len = 0;
Array<uint16_t> supported_group_list; // our list
// The client's Channel ID private key.
@@ -3160,11 +3154,6 @@
// shutdown.
bool quiet_shutdown : 1;
- // did_dummy_pq_padding is only valid for a client. In that context, it is
- // true iff the client observed the server echoing a dummy PQ padding
- // extension.
- bool did_dummy_pq_padding : 1;
-
// If enable_early_data is true, early data can be sent and accepted.
bool enable_early_data : 1;
};
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index d6181f3..8d2f134 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -626,7 +626,6 @@
max_cert_list(ctx->max_cert_list),
server(false),
quiet_shutdown(ctx->quiet_shutdown),
- did_dummy_pq_padding(false),
enable_early_data(ctx->enable_early_data) {
CRYPTO_new_ex_data(&ex_data);
}
@@ -2442,26 +2441,6 @@
ctx->psk_server_callback = cb;
}
-int SSL_set_dummy_pq_padding_size(SSL *ssl, size_t num_bytes) {
- if (!ssl->config) {
- return 0;
- }
- if (num_bytes > 0xffff) {
- return 0;
- }
-
- ssl->config->dummy_pq_padding_len = num_bytes;
- return 1;
-}
-
-int SSL_dummy_pq_padding_used(SSL *ssl) {
- if (ssl->server) {
- return 0;
- }
-
- return ssl->did_dummy_pq_padding;
-}
-
void SSL_CTX_set_msg_callback(SSL_CTX *ctx,
void (*cb)(int write_p, int version,
int content_type, const void *buf,
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 4de563d..e129ab9 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -2368,79 +2368,6 @@
}
-// Dummy PQ Padding extension
-//
-// Dummy post-quantum padding invovles the client (and later server) sending
-// useless, random-looking bytes in an extension in their ClientHello or
-// ServerHello. These extensions are sized to simulate a post-quantum
-// key-exchange and so enable measurement of the latency impact of the
-// additional bandwidth.
-
-static bool ext_dummy_pq_padding_add(CBB *out, size_t len) {
- CBB contents;
- uint8_t *buffer;
- if (!CBB_add_u16(out, TLSEXT_TYPE_dummy_pq_padding) ||
- !CBB_add_u16_length_prefixed(out, &contents) ||
- !CBB_add_space(&contents, &buffer, len)) {
- return false;
- }
-
- // The length is used as the nonce so that different length extensions have
- // different contents. There's no reason this has to be the case, it just
- // makes things a little more obvious in a packet dump.
- uint8_t nonce[12] = {0};
- memcpy(nonce, &len, sizeof(len));
-
- memset(buffer, 0, len);
- static const uint8_t kZeroKey[32] = {0};
- CRYPTO_chacha_20(buffer, buffer, len, kZeroKey, nonce, 0);
-
- return CBB_flush(out);
-}
-
-static bool ext_dummy_pq_padding_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
- const size_t len = hs->config->dummy_pq_padding_len;
- if (len == 0) {
- return true;
- }
-
- return ext_dummy_pq_padding_add(out, len);
-}
-
-static bool ext_dummy_pq_padding_parse_serverhello(SSL_HANDSHAKE *hs,
- uint8_t *out_alert,
- CBS *contents) {
- if (contents == nullptr) {
- return true;
- }
-
- if (CBS_len(contents) != hs->config->dummy_pq_padding_len) {
- return false;
- }
-
- hs->ssl->did_dummy_pq_padding = true;
- return true;
-}
-
-static bool ext_dummy_pq_padding_parse_clienthello(SSL_HANDSHAKE *hs,
- uint8_t *out_alert,
- CBS *contents) {
- if (contents != nullptr &&
- 0 < CBS_len(contents) && CBS_len(contents) < (1 << 12)) {
- hs->dummy_pq_padding_len = CBS_len(contents);
- }
-
- return true;
-}
-
-static bool ext_dummy_pq_padding_add_serverhello(SSL_HANDSHAKE *hs, CBB *out) {
- if (!hs->dummy_pq_padding_len) {
- return true;
- }
-
- return ext_dummy_pq_padding_add(out, hs->dummy_pq_padding_len);
-}
-
// Negotiated Groups
//
// https://tools.ietf.org/html/rfc4492#section-5.1.2
@@ -2994,14 +2921,6 @@
dont_add_serverhello,
},
{
- TLSEXT_TYPE_dummy_pq_padding,
- NULL,
- ext_dummy_pq_padding_add_clienthello,
- ext_dummy_pq_padding_parse_serverhello,
- ext_dummy_pq_padding_parse_clienthello,
- ext_dummy_pq_padding_add_serverhello,
- },
- {
TLSEXT_TYPE_quic_transport_parameters,
NULL,
ext_quic_transport_params_add_clienthello,
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 009bdeb..26173d3 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -626,14 +626,6 @@
return false;
}
- const bool did_dummy_pq_padding = !!SSL_dummy_pq_padding_used(ssl);
- if (config->expect_dummy_pq_padding != did_dummy_pq_padding) {
- fprintf(stderr,
- "Dummy PQ padding %s observed, but expected the opposite.\n",
- did_dummy_pq_padding ? "was" : "was not");
- return false;
- }
-
return true;
}
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index aeb7ad0..f2bb9dc 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -140,7 +140,6 @@
extensionRenegotiationInfo uint16 = 0xff01
extensionQUICTransportParams uint16 = 0xffa5 // draft-ietf-quic-tls-13
extensionChannelID uint16 = 30032 // not IANA assigned
- extensionDummyPQPadding uint16 = 54537 // not IANA assigned
)
// TLS signaling cipher suite values
@@ -1596,15 +1595,6 @@
// require the server send the draft TLS 1.3 anti-downgrade signal.
ExpectDraftTLS13DowngradeRandom bool
- // ExpectDummyPQPaddingLength, if not zero, causes the server to
- // require that the client sent a dummy PQ padding extension of this
- // length.
- ExpectDummyPQPaddingLength int
-
- // SendDummyPQPaddingLength causes a client to send a dummy PQ padding
- // extension of the given length in the ClientHello.
- SendDummyPQPaddingLength int
-
// SendCompressedCoordinates, if true, causes ECDH key shares over NIST
// curves to use compressed coordinates.
SendCompressedCoordinates bool
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 3e20d87..614671d 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -100,7 +100,6 @@
pskBinderFirst: c.config.Bugs.PSKBinderFirst,
omitExtensions: c.config.Bugs.OmitExtensions,
emptyExtensions: c.config.Bugs.EmptyExtensions,
- dummyPQPaddingLen: c.config.Bugs.SendDummyPQPaddingLength,
}
if maxVersion >= VersionTLS13 {
@@ -1559,10 +1558,6 @@
c.quicTransportParams = serverExtensions.quicTransportParams
}
- if l := c.config.Bugs.ExpectDummyPQPaddingLength; l != 0 && serverExtensions.dummyPQPaddingLen != l {
- return fmt.Errorf("tls: expected %d-byte dummy PQ padding extension, but got %d bytes", l, serverExtensions.dummyPQPaddingLen)
- }
-
return nil
}
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go
index 5324d74..43eb6fe 100644
--- a/ssl/test/runner/handshake_messages.go
+++ b/ssl/test/runner/handshake_messages.go
@@ -295,7 +295,6 @@
omitExtensions bool
emptyExtensions bool
pad int
- dummyPQPaddingLen int
compressedCertAlgs []uint16
}
@@ -350,7 +349,6 @@
m.omitExtensions == m1.omitExtensions &&
m.emptyExtensions == m1.emptyExtensions &&
m.pad == m1.pad &&
- m.dummyPQPaddingLen == m1.dummyPQPaddingLen &&
eqUint16s(m.compressedCertAlgs, m1.compressedCertAlgs)
}
@@ -583,11 +581,6 @@
customExt := extensions.addU16LengthPrefixed()
customExt.addBytes([]byte(m.customExtension))
}
- if l := m.dummyPQPaddingLen; l != 0 {
- extensions.addU16(extensionDummyPQPadding)
- body := extensions.addU16LengthPrefixed()
- body.addBytes(make([]byte, l))
- }
if len(m.compressedCertAlgs) > 0 {
extensions.addU16(extensionCompressedCertAlgs)
body := extensions.addU16LengthPrefixed()
@@ -908,11 +901,6 @@
m.sctListSupported = true
case extensionCustom:
m.customExtension = string(body)
- case extensionDummyPQPadding:
- if len(body) == 0 {
- return false
- }
- m.dummyPQPaddingLen = len(body)
case extensionCompressedCertAlgs:
var algIDs byteReader
if !body.readU8LengthPrefixed(&algIDs) {
@@ -1198,7 +1186,6 @@
supportedCurves []CurveID
quicTransportParams []byte
serverNameAck bool
- dummyPQPaddingLen int
}
func (m *serverExtensions) marshal(extensions *byteBuilder) {
@@ -1333,11 +1320,6 @@
extensions.addU16(extensionServerName)
extensions.addU16(0) // zero length
}
- if l := m.dummyPQPaddingLen; l != 0 {
- extensions.addU16(extensionDummyPQPadding)
- body := extensions.addU16LengthPrefixed()
- body.addBytes(make([]byte, l))
- }
}
func (m *serverExtensions) unmarshal(data byteReader, version uint16) bool {
@@ -1442,8 +1424,6 @@
return false
}
m.hasEarlyData = true
- case extensionDummyPQPadding:
- m.dummyPQPaddingLen = len(body)
default:
// Unknown extensions are illegal from the server.
return false
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index c0653b1..2ba438a 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -337,10 +337,6 @@
}
}
- if expected := hs.clientHello.dummyPQPaddingLen; expected != config.Bugs.ExpectDummyPQPaddingLength {
- return fmt.Errorf("tls: expected dummy PQ padding extension of length %d, but got one of length %d", expected, config.Bugs.ExpectDummyPQPaddingLength)
- }
-
if err := checkRSAPSSSupport(config.Bugs.ExpectRSAPSSSupport, hs.clientHello.signatureAlgorithms, hs.clientHello.signatureAlgorithmsCert); err != nil {
return err
}
@@ -1430,10 +1426,6 @@
return errors.New("tls: no GREASE extension found")
}
- if l := hs.clientHello.dummyPQPaddingLen; l != 0 {
- serverExtensions.dummyPQPaddingLen = l
- }
-
serverExtensions.serverNameAck = c.config.Bugs.SendServerNameAck
return nil
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index d95cc28..5ac6ec4 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -7500,49 +7500,6 @@
shouldFail: true,
expectedError: ":INVALID_SCT_LIST:",
})
-
- for _, version := range allVersions(tls) {
- if version.version < VersionTLS12 {
- continue
- }
-
- for _, paddingLen := range []int{400, 1100} {
- testCases = append(testCases, testCase{
- name: fmt.Sprintf("DummyPQPadding-%d-%s", paddingLen, version.name),
- testType: clientTest,
- tls13Variant: version.tls13Variant,
- resumeSession: true,
- config: Config{
- MaxVersion: version.version,
- Bugs: ProtocolBugs{
- ExpectDummyPQPaddingLength: paddingLen,
- },
- },
- flags: []string{
- "-max-version", version.shimFlag(tls),
- "-dummy-pq-padding-len", strconv.Itoa(paddingLen),
- "-expect-dummy-pq-padding",
- },
- })
-
- testCases = append(testCases, testCase{
- name: fmt.Sprintf("DummyPQPadding-Server-%d-%s", paddingLen, version.name),
- testType: serverTest,
- tls13Variant: version.tls13Variant,
- resumeSession: true,
- config: Config{
- MaxVersion: version.version,
- Bugs: ProtocolBugs{
- SendDummyPQPaddingLength: paddingLen,
- ExpectDummyPQPaddingLength: paddingLen,
- },
- },
- flags: []string{
- "-max-version", version.shimFlag(tls),
- },
- })
- }
- }
}
func addResumptionVersionTests() {
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index fee4258..30cb98c 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -135,7 +135,6 @@
&TestConfig::allow_false_start_without_alpn },
{ "-expect-draft-downgrade", &TestConfig::expect_draft_downgrade },
{ "-handoff", &TestConfig::handoff },
- { "-expect-dummy-pq-padding", &TestConfig::expect_dummy_pq_padding },
{ "-no-rsa-pss-rsae-certs", &TestConfig::no_rsa_pss_rsae_certs },
{ "-use-ocsp-callback", &TestConfig::use_ocsp_callback },
{ "-set-ocsp-in-callback", &TestConfig::set_ocsp_in_callback },
@@ -216,7 +215,6 @@
{ "-read-size", &TestConfig::read_size },
{ "-expect-ticket-age-skew", &TestConfig::expect_ticket_age_skew },
{ "-tls13-variant", &TestConfig::tls13_variant },
- { "-dummy-pq-padding-len", &TestConfig::dummy_pq_padding_len },
};
const Flag<std::vector<int>> kIntVectorFlags[] = {
@@ -1613,10 +1611,6 @@
if (max_send_fragment > 0) {
SSL_set_max_send_fragment(ssl.get(), max_send_fragment);
}
- if (dummy_pq_padding_len > 0 &&
- !SSL_set_dummy_pq_padding_size(ssl.get(), dummy_pq_padding_len)) {
- return nullptr;
- }
if (!quic_transport_params.empty()) {
if (!SSL_set_quic_transport_params(
ssl.get(),
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 7f9f442..835d29d 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -157,9 +157,7 @@
std::string expect_msg_callback;
bool allow_false_start_without_alpn = false;
bool expect_draft_downgrade = false;
- int dummy_pq_padding_len = 0;
bool handoff = false;
- bool expect_dummy_pq_padding = false;
bool no_rsa_pss_rsae_certs = false;
bool use_ocsp_callback = false;
bool set_ocsp_in_callback = false;
