Drop C++ from certificate compression API.
It's 2018, but passing STL objects across the API boundary turns out to
still be more bother than it's worth. Since we're dropping UniquePtr in
the API anyway, go the whole way and make it a plain-C API.
Change-Id: Ic0202012e5d81afe62d71b3fb57e6a27a8f63c65
Update-note: this will need corresponding changes to the internal use of SSL_CTX_add_cert_compression_alg.
Reviewed-on: https://boringssl-review.googlesource.com/29564
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index f04e0ec..f693030 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2759,6 +2759,54 @@
int enabled);
+// Certificate compression.
+//
+// Certificates in TLS 1.3 can be compressed[1]. BoringSSL supports this as both
+// a client and a server, but does not link against any specific compression
+// libraries in order to keep dependencies to a minimum. Instead, hooks for
+// compression and decompression can be installed in an |SSL_CTX| to enable
+// support.
+//
+// [1] https://tools.ietf.org/html/draft-ietf-tls-certificate-compression-03.
+
+// ssl_cert_compression_func_t is a pointer to a function that performs
+// compression. It must write the compressed representation of |in| to |out|,
+// returning one on success and zero on error. The results of compressing
+// certificates are not cached internally. Implementations may wish to implement
+// their own cache if they expect it to be useful given the certificates that
+// they serve.
+typedef int (*ssl_cert_compression_func_t)(SSL *ssl, CBB *out,
+ const uint8_t *in, size_t in_len);
+
+// ssl_cert_decompression_func_t is a pointer to a function that performs
+// decompression. The compressed data from the peer is passed as |in| and the
+// decompressed result must be exactly |uncompressed_len| bytes long. It returns
+// one on success, in which case |*out| must be set to the result of
+// decompressing |in|, or zero on error. Setting |*out| transfers ownership,
+// i.e. |CRYPTO_BUFFER_free| will be called on |*out| at some point in the
+// future. The results of decompressions are not cached internally.
+// Implementations may wish to implement their own cache if they expect it to be
+// useful.
+typedef int (*ssl_cert_decompression_func_t)(SSL *ssl, CRYPTO_BUFFER **out,
+ size_t uncompressed_len,
+ const uint8_t *in, size_t in_len);
+
+// SSL_CTX_add_cert_compression_alg registers a certificate compression
+// algorithm on |ctx| with ID |alg_id|. (The value of |alg_id| should be an IANA
+// assigned value and each can only be registered once.)
+//
+// One of the function pointers may be NULL to avoid having to implement both
+// sides of a compression algorithm if you're only going to use it in one
+// direction. In this case, the unimplemented direction acts like it was never
+// configured.
+//
+// For a server, algorithms are registered in preference order with the most
+// preferable first. It returns one on success or zero on error.
+OPENSSL_EXPORT int SSL_CTX_add_cert_compression_alg(
+ SSL_CTX *ctx, uint16_t alg_id, ssl_cert_compression_func_t compress,
+ ssl_cert_decompression_func_t decompress);
+
+
// Next protocol negotiation.
//
// The NPN extension (draft-agl-tls-nextprotoneg-03) is the predecessor to ALPN
@@ -4448,50 +4496,6 @@
BORINGSSL_MAKE_DELETER(SSL_SESSION, SSL_SESSION_free)
BORINGSSL_MAKE_UP_REF(SSL_SESSION, SSL_SESSION_up_ref)
-// Certificate compression.
-//
-// Certificates in TLS 1.3 can be compressed[1]. BoringSSL supports this as both
-// a client and a server, but does not link against any specific compression
-// libraries in order to keep dependencies to a minimum. Instead, hooks for
-// compression and decompression can be installed in an |SSL_CTX| to enable
-// support.
-//
-// [1] https://tools.ietf.org/html/draft-ietf-tls-certificate-compression-03.
-
-// CertCompressFunc is a pointer to a function that performs compression. It
-// must write the compressed representation of |in| to |out|, returning one on
-// success and zero on error. The results of compressing certificates are not
-// cached internally. Implementations may wish to implement their own cache if
-// they expect it to be useful given the certificates that they serve.
-typedef bool (*CertCompressFunc)(SSL *ssl, CBB *out, Span<const uint8_t> in);
-
-// CertDecompressFunc is a pointer to a function that performs decompression.
-// The compressed data from the peer is passed as |in| and the decompressed
-// result must be exactly |uncompressed_len| bytes long. It returns one on
-// success, in which case |*out| must be set to the results of decompressing
-// |in|, or zero on error. The results of decompression are not cached
-// internally. Implementations may wish to implement their own cache if they
-// expect it to be useful.
-typedef bool (*CertDecompressFunc)(SSL *ssl,
- bssl::UniquePtr<CRYPTO_BUFFER> *out,
- size_t uncompressed_len,
- Span<const uint8_t> in);
-
-// SSL_CTX_add_cert_compression_alg registers a certificate compression
-// algorithm on |ctx| with ID |alg_id|. (The value of |alg_id| should be an IANA
-// assigned value and each can only be registered once.)
-//
-// One of the function pointers may be nullptr to avoid having to implement both
-// sides of a compression algorithm if you're only going to use it in one
-// direction. In this case, the unimplemented direction acts like it was never
-// configured.
-//
-// For a server, algorithms are registered in preference order with the most
-// preferable first. It returns one on success or zero on error.
-OPENSSL_EXPORT int SSL_CTX_add_cert_compression_alg(
- SSL_CTX *ctx, uint16_t alg_id, CertCompressFunc compress,
- CertDecompressFunc decompress);
-
enum class OpenRecordResult {
kOK,
kDiscard,
diff --git a/ssl/internal.h b/ssl/internal.h
index 52320f2..e3f0984 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -2050,8 +2050,8 @@
DECLARE_LHASH_OF(SSL_SESSION)
struct CertCompressionAlg {
- bssl::CertCompressFunc compress;
- bssl::CertDecompressFunc decompress;
+ ssl_cert_compression_func_t compress;
+ ssl_cert_decompression_func_t decompress;
uint16_t alg_id;
};
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 0363e32..c68968a 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -507,51 +507,6 @@
ssl->config->handoff = on;
}
-int SSL_CTX_add_cert_compression_alg(SSL_CTX *ctx, uint16_t alg_id,
- bssl::CertCompressFunc compress,
- bssl::CertDecompressFunc decompress) {
- assert(compress != nullptr || decompress != nullptr);
-
- for (CertCompressionAlg *alg : ctx->cert_compression_algs) {
- if (alg->alg_id == alg_id) {
- return 0;
- }
- }
-
- CertCompressionAlg *alg = reinterpret_cast<CertCompressionAlg *>(
- OPENSSL_malloc(sizeof(CertCompressionAlg)));
- if (alg == nullptr) {
- goto err;
- }
-
- OPENSSL_memset(alg, 0, sizeof(CertCompressionAlg));
- alg->alg_id = alg_id;
- alg->compress = compress;
- alg->decompress = decompress;
-
- if (ctx->cert_compression_algs == nullptr) {
- ctx->cert_compression_algs = sk_CertCompressionAlg_new_null();
- if (ctx->cert_compression_algs == nullptr) {
- goto err;
- }
- }
-
- if (!sk_CertCompressionAlg_push(ctx->cert_compression_algs, alg)) {
- goto err;
- }
-
- return 1;
-
-err:
- OPENSSL_free(alg);
- if (ctx->cert_compression_algs != nullptr &&
- sk_CertCompressionAlg_num(ctx->cert_compression_algs) == 0) {
- sk_CertCompressionAlg_free(ctx->cert_compression_algs);
- ctx->cert_compression_algs = nullptr;
- }
- return 0;
-}
-
} // namespace bssl
using namespace bssl;
@@ -2203,6 +2158,51 @@
ctx->allow_unknown_alpn_protos = !!enabled;
}
+int SSL_CTX_add_cert_compression_alg(SSL_CTX *ctx, uint16_t alg_id,
+ ssl_cert_compression_func_t compress,
+ ssl_cert_decompression_func_t decompress) {
+ assert(compress != nullptr || decompress != nullptr);
+
+ for (CertCompressionAlg *alg : ctx->cert_compression_algs) {
+ if (alg->alg_id == alg_id) {
+ return 0;
+ }
+ }
+
+ CertCompressionAlg *alg = reinterpret_cast<CertCompressionAlg *>(
+ OPENSSL_malloc(sizeof(CertCompressionAlg)));
+ if (alg == nullptr) {
+ goto err;
+ }
+
+ OPENSSL_memset(alg, 0, sizeof(CertCompressionAlg));
+ alg->alg_id = alg_id;
+ alg->compress = compress;
+ alg->decompress = decompress;
+
+ if (ctx->cert_compression_algs == nullptr) {
+ ctx->cert_compression_algs = sk_CertCompressionAlg_new_null();
+ if (ctx->cert_compression_algs == nullptr) {
+ goto err;
+ }
+ }
+
+ if (!sk_CertCompressionAlg_push(ctx->cert_compression_algs, alg)) {
+ goto err;
+ }
+
+ return 1;
+
+err:
+ OPENSSL_free(alg);
+ if (ctx->cert_compression_algs != nullptr &&
+ sk_CertCompressionAlg_num(ctx->cert_compression_algs) == 0) {
+ sk_CertCompressionAlg_free(ctx->cert_compression_algs);
+ ctx->cert_compression_algs = nullptr;
+ }
+ return 0;
+}
+
void SSL_CTX_set_tls_channel_id_enabled(SSL_CTX *ctx, int enabled) {
ctx->tlsext_channel_id_enabled = !!enabled;
}
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 6fd5f36..6b4732c 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -3913,37 +3913,39 @@
EXPECT_TRUE(SSL_is_signature_algorithm_rsa_pss(SSL_SIGN_RSA_PSS_RSAE_SHA384));
}
-static bool XORCompressFunc(SSL *ssl, CBB *out, Span<const uint8_t> in) {
- for (size_t i = 0; i < in.size(); i++) {
+static int XORCompressFunc(SSL *ssl, CBB *out, const uint8_t *in,
+ size_t in_len) {
+ for (size_t i = 0; i < in_len; i++) {
if (!CBB_add_u8(out, in[i] ^ 0x55)) {
- return false;
+ return 0;
}
}
SSL_set_app_data(ssl, XORCompressFunc);
- return true;
+ return 1;
}
-static bool XORDecompressFunc(SSL *ssl, bssl::UniquePtr<CRYPTO_BUFFER> *out,
- size_t uncompressed_len, Span<const uint8_t> in) {
- if (in.size() != uncompressed_len) {
- return false;
+static int XORDecompressFunc(SSL *ssl, CRYPTO_BUFFER **out,
+ size_t uncompressed_len, const uint8_t *in,
+ size_t in_len) {
+ if (in_len != uncompressed_len) {
+ return 0;
}
uint8_t *data;
- out->reset(CRYPTO_BUFFER_alloc(&data, uncompressed_len));
- if (out->get() == nullptr) {
- return false;
+ *out = CRYPTO_BUFFER_alloc(&data, uncompressed_len);
+ if (*out == nullptr) {
+ return 0;
}
- for (size_t i = 0; i < in.size(); i++) {
+ for (size_t i = 0; i < in_len; i++) {
data[i] = in[i] ^ 0x55;
}
SSL_set_app_data(ssl, XORDecompressFunc);
- return true;
+ return 1;
}
TEST(SSLTest, CertCompression) {
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index f7804d6..c6500ab 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -1346,44 +1346,44 @@
if (install_cert_compression_algs &&
(!SSL_CTX_add_cert_compression_alg(
ssl_ctx.get(), 0xff02,
- [](SSL *ssl, CBB *out, bssl::Span<const uint8_t> in) -> bool {
+ [](SSL *ssl, CBB *out, const uint8_t *in, size_t in_len) -> int {
if (!CBB_add_u8(out, 1) || !CBB_add_u8(out, 2) ||
!CBB_add_u8(out, 3) || !CBB_add_u8(out, 4) ||
- !CBB_add_bytes(out, in.data(), in.size())) {
- return false;
+ !CBB_add_bytes(out, in, in_len)) {
+ return 0;
}
- return true;
+ return 1;
},
- [](SSL *ssl, bssl::UniquePtr<CRYPTO_BUFFER> *out,
- size_t uncompressed_len, bssl::Span<const uint8_t> in) -> bool {
- if (in.size() < 4 || in[0] != 1 || in[1] != 2 || in[2] != 3 ||
- in[3] != 4 || uncompressed_len != in.size() - 4) {
- return false;
+ [](SSL *ssl, CRYPTO_BUFFER **out, size_t uncompressed_len,
+ const uint8_t *in, size_t in_len) -> int {
+ if (in_len < 4 || in[0] != 1 || in[1] != 2 || in[2] != 3 ||
+ in[3] != 4 || uncompressed_len != in_len - 4) {
+ return 0;
}
- const bssl::Span<const uint8_t> uncompressed(in.subspan(4));
- out->reset(CRYPTO_BUFFER_new(uncompressed.data(),
- uncompressed.size(), nullptr));
- return true;
+ const bssl::Span<const uint8_t> uncompressed(in + 4, in_len - 4);
+ *out = CRYPTO_BUFFER_new(uncompressed.data(), uncompressed.size(),
+ nullptr);
+ return 1;
}) ||
!SSL_CTX_add_cert_compression_alg(
ssl_ctx.get(), 0xff01,
- [](SSL *ssl, CBB *out, bssl::Span<const uint8_t> in) -> bool {
- if (in.size() < 2 || in[0] != 0 || in[1] != 0) {
- return false;
+ [](SSL *ssl, CBB *out, const uint8_t *in, size_t in_len) -> int {
+ if (in_len < 2 || in[0] != 0 || in[1] != 0) {
+ return 0;
}
- return CBB_add_bytes(out, in.data() + 2, in.size() - 2);
+ return CBB_add_bytes(out, in + 2, in_len - 2);
},
- [](SSL *ssl, bssl::UniquePtr<CRYPTO_BUFFER> *out,
- size_t uncompressed_len, bssl::Span<const uint8_t> in) -> bool {
- if (uncompressed_len != 2 + in.size()) {
- return false;
+ [](SSL *ssl, CRYPTO_BUFFER **out, size_t uncompressed_len,
+ const uint8_t *in, size_t in_len) -> int {
+ if (uncompressed_len != 2 + in_len) {
+ return 0;
}
- std::unique_ptr<uint8_t[]> buf(new uint8_t[2 + in.size()]);
+ std::unique_ptr<uint8_t[]> buf(new uint8_t[2 + in_len]);
buf[0] = 0;
buf[1] = 0;
- OPENSSL_memcpy(&buf[2], in.data(), in.size());
- out->reset(CRYPTO_BUFFER_new(buf.get(), 2 + in.size(), nullptr));
- return true;
+ OPENSSL_memcpy(&buf[2], in, in_len);
+ *out = CRYPTO_BUFFER_new(buf.get(), 2 + in_len, nullptr);
+ return 1;
}))) {
fprintf(stderr, "SSL_CTX_add_cert_compression_alg failed.\n");
abort();
diff --git a/ssl/tls13_both.cc b/ssl/tls13_both.cc
index 9149e76..696909a 100644
--- a/ssl/tls13_both.cc
+++ b/ssl/tls13_both.cc
@@ -130,7 +130,7 @@
return 0;
}
- bssl::CertDecompressFunc decompress = nullptr;
+ ssl_cert_decompression_func_t decompress = nullptr;
for (const auto& alg : ssl->ctx->cert_compression_algs) {
if (alg->alg_id == alg_id) {
decompress = alg->decompress;
@@ -145,16 +145,28 @@
return 0;
}
- if (!decompress(ssl, &decompressed, uncompressed_len, compressed) ||
- CRYPTO_BUFFER_len(decompressed.get()) != uncompressed_len) {
+ CRYPTO_BUFFER *decompressed_ptr = nullptr;
+ if (!decompress(ssl, &decompressed_ptr, uncompressed_len,
+ CBS_data(&compressed), CBS_len(&compressed))) {
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_DECOMPRESSION_FAILED);
ERR_add_error_dataf("alg=%d", static_cast<int>(alg_id));
return 0;
}
+ decompressed.reset(decompressed_ptr);
- CBS_init(&body, CRYPTO_BUFFER_data(decompressed.get()),
- CRYPTO_BUFFER_len(decompressed.get()));
+ if (CRYPTO_BUFFER_len(decompressed_ptr) != uncompressed_len) {
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_DECOMPRESSION_FAILED);
+ ERR_add_error_dataf(
+ "alg=%d got=%u expected=%u", static_cast<int>(alg_id),
+ static_cast<unsigned>(CRYPTO_BUFFER_len(decompressed_ptr)),
+ static_cast<unsigned>(uncompressed_len));
+ return 0;
+ }
+
+ CBS_init(&body, CRYPTO_BUFFER_data(decompressed_ptr),
+ CRYPTO_BUFFER_len(decompressed_ptr));
} else {
assert(msg.type == SSL3_MT_CERTIFICATE);
}
@@ -512,7 +524,7 @@
!CBB_add_u16(body, hs->cert_compression_alg_id) ||
!CBB_add_u24(body, msg.size()) ||
!CBB_add_u24_length_prefixed(body, &compressed) ||
- !alg->compress(ssl, &compressed, msg) ||
+ !alg->compress(ssl, &compressed, msg.data(), msg.size()) ||
!ssl_add_message_cbb(ssl, cbb.get())) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return 0;
