Remove in-place TLS record assembly for now.
Decrypting is very easy to do in-place, but encrypting in-place is a hassle.
The rules actually were wrong due to record-splitting. The aliasing prefix and
the alignment prefix actually differ by 1. Take it out for now in preparation
for tightening the aliasing rules.
If we decide to do in-place encrypt later, probably it'd be more useful to
return header + in-place ciphertext + trailer. (That, in turn, needs a
scatter/gather thing on the AEAD thanks to TLS 1.3's padding and record type
construction.) We may also wish to rethink how record-splitting works here.
Change-Id: I0187d39c541e76ef933b7c2c193323164fd8a156
Reviewed-on: https://boringssl-review.googlesource.com/8230
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/internal.h b/crypto/internal.h
index 433072c..54e78d5 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -152,6 +152,19 @@
#endif
+/* buffers_alias returns one if |a| and |b| alias and zero otherwise. */
+static inline int buffers_alias(const uint8_t *a, size_t a_len,
+ const uint8_t *b, size_t b_len) {
+ /* Cast |a| and |b| to integers. In C, pointer comparisons between unrelated
+ * objects are undefined whereas pointer to integer conversions are merely
+ * implementation-defined. We assume the implementation defined it in a sane
+ * way. */
+ uintptr_t a_u = (uintptr_t)a;
+ uintptr_t b_u = (uintptr_t)b;
+ return a_u + a_len > b_u && b_u + b_len > a_u;
+}
+
+
/* Constant-time utility functions.
*
* The following methods return a bitmask of all ones (0xff...f) for true and 0
diff --git a/ssl/internal.h b/ssl/internal.h
index e9cf918..6d96c6c 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -402,17 +402,21 @@
uint8_t *out_alert, uint8_t *in,
size_t in_len);
-/* ssl_seal_prefix_len returns the length of the prefix before the ciphertext
- * when sealing a record with |ssl|. Note that this value may differ from
- * |ssl_record_prefix_len| when TLS 1.0 CBC record-splitting is enabled. Sealing
- * a small record may also result in a smaller output than this value.
+/* ssl_seal_align_prefix_len returns the length of the prefix before the start
+ * of the bulk of the ciphertext when sealing a record with |ssl|. Callers may
+ * use this to align buffers.
+ *
+ * Note when TLS 1.0 CBC record-splitting is enabled, this includes the one byte
+ * record and is the offset into second record's ciphertext. Thus this value may
+ * differ from |ssl_record_prefix_len| and sealing a small record may result in
+ * a smaller output than this value.
*
* TODO(davidben): Expose this as part of public API once the high-level
* buffer-free APIs are available. */
-size_t ssl_seal_prefix_len(const SSL *ssl);
+size_t ssl_seal_align_prefix_len(const SSL *ssl);
/* ssl_max_seal_overhead returns the maximum overhead of sealing a record with
- * |ssl|. This includes |ssl_seal_prefix_len|.
+ * |ssl|.
*
* TODO(davidben): Expose this as part of public API once the high-level
* buffer-free APIs are available. */
@@ -423,11 +427,12 @@
* and zero on error. If enabled, |tls_seal_record| implements TLS 1.0 CBC 1/n-1
* record splitting and may write two records concatenated.
*
- * For a large record, the ciphertext will begin |ssl_seal_prefix_len| bytes
- * into out. Aligning |out| appropriately may improve performance. It writes at
- * most |in_len| + |ssl_max_seal_overhead| bytes to |out|.
+ * For a large record, the bulk of the ciphertext will begin
+ * |ssl_seal_align_prefix_len| bytes into out. Aligning |out| appropriately may
+ * improve performance. It writes at most |in_len| + |ssl_max_seal_overhead|
+ * bytes to |out|.
*
- * If |in| and |out| alias, |out| + |ssl_seal_prefix_len| must be <= |in|. */
+ * |in| and |out| may not alias. */
int tls_seal_record(SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out,
uint8_t type, const uint8_t *in, size_t in_len);
diff --git a/ssl/ssl_buffer.c b/ssl/ssl_buffer.c
index df814fa..a3cf360 100644
--- a/ssl/ssl_buffer.c
+++ b/ssl/ssl_buffer.c
@@ -224,7 +224,7 @@
return 0;
}
- size_t header_len = ssl_seal_prefix_len(ssl);
+ size_t header_len = ssl_seal_align_prefix_len(ssl);
/* TODO(davidben): This matches the original behavior in keeping the malloc
* size consistent. Does this matter? |cap| could just be |max_len|. */
diff --git a/ssl/tls_record.c b/ssl/tls_record.c
index 24dfb21..e1553e3 100644
--- a/ssl/tls_record.c
+++ b/ssl/tls_record.c
@@ -116,6 +116,7 @@
#include <openssl/mem.h>
#include "internal.h"
+#include "../crypto/internal.h"
/* kMaxEmptyRecords is the number of consecutive, empty records that will be
@@ -159,7 +160,7 @@
}
}
-size_t ssl_seal_prefix_len(const SSL *ssl) {
+size_t ssl_seal_align_prefix_len(const SSL *ssl) {
if (SSL_IS_DTLS(ssl)) {
return DTLS1_RT_HEADER_LENGTH +
SSL_AEAD_CTX_explicit_nonce_len(ssl->s3->aead_write_ctx);
@@ -301,12 +302,14 @@
OPENSSL_PUT_ERROR(SSL, SSL_R_BUFFER_TOO_SMALL);
return 0;
}
- /* Check the record header does not alias any part of the input.
- * |SSL_AEAD_CTX_seal| will internally enforce other aliasing requirements. */
- if (in < out + SSL3_RT_HEADER_LENGTH && out < in + in_len) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_OUTPUT_ALIASES_INPUT);
- return 0;
- }
+
+ /* Either |in| and |out| don't alias or |in| aligns with the
+ * ciphertext. |tls_seal_record| forbids aliasing, but TLS 1.3 aliases them
+ * internally. */
+ assert(!buffers_alias(in, in_len, out, max_out) ||
+ in ==
+ out + SSL3_RT_HEADER_LENGTH +
+ SSL_AEAD_CTX_explicit_nonce_len(ssl->s3->aead_write_ctx));
out[0] = type;
@@ -347,6 +350,11 @@
int tls_seal_record(SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out,
uint8_t type, const uint8_t *in, size_t in_len) {
+ if (buffers_alias(in, in_len, out, max_out)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_OUTPUT_ALIASES_INPUT);
+ return 0;
+ }
+
size_t frag_len = 0;
/* TLS 1.3 hides the actual record type inside the encrypted data. */
@@ -369,18 +377,7 @@
if (type == SSL3_RT_APPLICATION_DATA && in_len > 1 &&
ssl_needs_record_splitting(ssl)) {
- /* |do_seal_record| will notice if it clobbers |in[0]|, but not if it
- * aliases the rest of |in|. */
- if (in + 1 <= out && out < in + in_len) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_OUTPUT_ALIASES_INPUT);
- return 0;
- }
- /* Ensure |do_seal_record| does not write beyond |in[0]|. */
- size_t frag_max_out = max_out;
- if (out <= in + 1 && in + 1 < out + frag_max_out) {
- frag_max_out = (size_t)(in + 1 - out);
- }
- if (!do_seal_record(ssl, out, &frag_len, frag_max_out, type, in, 1)) {
+ if (!do_seal_record(ssl, out, &frag_len, max_out, type, in, 1)) {
return 0;
}
in++;
