From 044a2c5e0d9119a8c1e01bb33535c7fc9651db8b Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 11:57:54 -0400 Subject: [PATCH 01/21] Use the epoch mask when purging cached keys --- src/sframe.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sframe.cpp b/src/sframe.cpp index b389cfc..ae86e16 100644 --- a/src/sframe.cpp +++ b/src/sframe.cpp @@ -359,10 +359,10 @@ MLSContext::remove_epoch(EpochID epoch_id) void MLSContext::purge_epoch(EpochID epoch_id) { - const auto drop_bits = epoch_id & epoch_bits; + const auto drop_bits = epoch_id & epoch_mask; keys.erase_if_key( - [&](const auto& epoch) { return (epoch & epoch_bits) == drop_bits; }); + [&](const auto& epoch) { return (epoch & epoch_mask) == drop_bits; }); } Result From 264a3ca3c4e615667aaba19687f53ae498a5afe2 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 11:58:10 -0400 Subject: [PATCH 02/21] Stop protecting frames after counter exhaustion --- src/sframe.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/sframe.cpp b/src/sframe.cpp index ae86e16..a7aeef2 100644 --- a/src/sframe.cpp +++ b/src/sframe.cpp @@ -3,6 +3,8 @@ #include "crypto.h" #include "header.h" +#include + namespace SFRAME_NAMESPACE { /// @@ -142,6 +144,11 @@ Context::protect(KeyID key_id, { SFRAME_VOID_OR_RETURN(require_key(key_id)); auto& key_record = keys.at(key_id); + if (key_record.counter == std::numeric_limits::max()) { + return SFrameError(SFrameErrorType::invalid_parameter_error, + "Counter exhausted"); + } + const auto counter = key_record.counter; key_record.counter += 1; From b9310bd7d6dfb1f225eb03b97d952ac4ab14690a Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 11:58:25 -0400 Subject: [PATCH 03/21] Enforce key direction on protect and unprotect --- src/sframe.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/sframe.cpp b/src/sframe.cpp index a7aeef2..a565f2e 100644 --- a/src/sframe.cpp +++ b/src/sframe.cpp @@ -144,6 +144,11 @@ Context::protect(KeyID key_id, { SFRAME_VOID_OR_RETURN(require_key(key_id)); auto& key_record = keys.at(key_id); + if (key_record.usage != KeyUsage::protect) { + return SFrameError(SFrameErrorType::invalid_key_usage_error, + "Key is not valid for protect"); + } + if (key_record.counter == std::numeric_limits::max()) { return SFrameError(SFrameErrorType::invalid_parameter_error, "Counter exhausted"); @@ -192,6 +197,10 @@ Context::protect_inner(const Header& header, SFRAME_VOID_OR_RETURN(require_key(header.key_id)); const auto& key_and_salt = keys.at(header.key_id); + if (key_and_salt.usage != KeyUsage::unprotect) { + return SFrameError(SFrameErrorType::invalid_key_usage_error, + "Key is not valid for unprotect"); + } SFRAME_VALUE_OR_RETURN(aad, form_aad(header, metadata)); const auto nonce = form_nonce(header.counter, key_and_salt.salt); From 1249207a7fbf1c0b642259150e8810a8fbbf40b6 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 11:58:33 -0400 Subject: [PATCH 04/21] Validate full header length before parsing fields --- src/header.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/header.cpp b/src/header.cpp index 58a072f..8225197 100644 --- a/src/header.cpp +++ b/src/header.cpp @@ -150,6 +150,11 @@ Header::parse(input_bytes buffer) } const auto cfg = ConfigByte{ buffer[0] }; + if (cfg.encoded_size() > buffer.size()) { + return SFrameError(SFrameErrorType::buffer_too_small_error, + "Ciphertext too small to decode header"); + } + const auto after_cfg = buffer.subspan(1); SFRAME_VALUE_OR_RETURN(kid_result, cfg.kid.read(after_cfg)); const auto [key_id, after_kid] = kid_result; From 2bfd4bac095d74ee4cd71c9c39a45f7b421d91cd Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 11:59:04 -0400 Subject: [PATCH 05/21] Reject oversized inputs in CTR mode --- src/crypto_boringssl.cpp | 21 +++++++++++++++++++++ src/crypto_openssl11.cpp | 21 +++++++++++++++++++++ src/crypto_openssl3.cpp | 21 +++++++++++++++++++++ 3 files changed, 63 insertions(+) diff --git a/src/crypto_boringssl.cpp b/src/crypto_boringssl.cpp index 96dd5ba..4dff820 100644 --- a/src/crypto_boringssl.cpp +++ b/src/crypto_boringssl.cpp @@ -11,6 +11,8 @@ #include #include +#include + namespace SFRAME_NAMESPACE { /// @@ -207,6 +209,23 @@ ctr_crypt(CipherSuite suite, return Result::ok(); } +static Result +validate_ctr_size(size_t size) +{ + static constexpr size_t max_ctr_size = size_t(uint64_t(1) << 32) * 16; + if (size > max_ctr_size) { + return SFrameError(SFrameErrorType::invalid_parameter_error, + "CTR input too large"); + } + + if (size > INT_MAX) { + return SFrameError(SFrameErrorType::invalid_parameter_error, + "Input too large for OpenSSL"); + } + + return Result::ok(); +} + static Result seal_ctr(CipherSuite suite, input_bytes key, @@ -216,6 +235,7 @@ seal_ctr(CipherSuite suite, input_bytes pt) { SFRAME_VALUE_OR_RETURN(tag_size, cipher_overhead(suite)); + SFRAME_VOID_OR_RETURN(validate_ctr_size(pt.size())); if (ct.size() < pt.size() + tag_size) { return SFrameError(SFrameErrorType::buffer_too_small_error, "Ciphertext buffer too small"); @@ -334,6 +354,7 @@ open_ctr(CipherSuite suite, } auto inner_ct_size = ct.size() - tag_size; + SFRAME_VOID_OR_RETURN(validate_ctr_size(inner_ct_size)); auto inner_ct = ct.subspan(0, inner_ct_size); auto tag = ct.subspan(inner_ct_size, tag_size); diff --git a/src/crypto_openssl11.cpp b/src/crypto_openssl11.cpp index 9b33785..c4cf527 100644 --- a/src/crypto_openssl11.cpp +++ b/src/crypto_openssl11.cpp @@ -9,6 +9,8 @@ #include #include +#include + namespace SFRAME_NAMESPACE { /// @@ -258,6 +260,23 @@ ctr_crypt(CipherSuite suite, return Result::ok(); } +static Result +validate_ctr_size(size_t size) +{ + static constexpr size_t max_ctr_size = size_t(uint64_t(1) << 32) * 16; + if (size > max_ctr_size) { + return SFrameError(SFrameErrorType::invalid_parameter_error, + "CTR input too large"); + } + + if (size > INT_MAX) { + return SFrameError(SFrameErrorType::invalid_parameter_error, + "Input too large for OpenSSL"); + } + + return Result::ok(); +} + static Result seal_ctr(CipherSuite suite, input_bytes key, @@ -267,6 +286,7 @@ seal_ctr(CipherSuite suite, input_bytes pt) { SFRAME_VALUE_OR_RETURN(tag_size, cipher_overhead(suite)); + SFRAME_VOID_OR_RETURN(validate_ctr_size(pt.size())); if (ct.size() < pt.size() + tag_size) { return SFrameError(SFrameErrorType::buffer_too_small_error, "Ciphertext buffer too small"); @@ -385,6 +405,7 @@ open_ctr(CipherSuite suite, } auto inner_ct_size = ct.size() - tag_size; + SFRAME_VOID_OR_RETURN(validate_ctr_size(inner_ct_size)); auto inner_ct = ct.subspan(0, inner_ct_size); auto tag = ct.subspan(inner_ct_size, tag_size); diff --git a/src/crypto_openssl3.cpp b/src/crypto_openssl3.cpp index 36fb125..fc03375 100644 --- a/src/crypto_openssl3.cpp +++ b/src/crypto_openssl3.cpp @@ -11,6 +11,8 @@ #include #include +#include + namespace SFRAME_NAMESPACE { /// @@ -247,6 +249,23 @@ ctr_crypt(CipherSuite suite, return Result::ok(); } +static Result +validate_ctr_size(size_t size) +{ + static constexpr size_t max_ctr_size = size_t(uint64_t(1) << 32) * 16; + if (size > max_ctr_size) { + return SFrameError(SFrameErrorType::invalid_parameter_error, + "CTR input too large"); + } + + if (size > INT_MAX) { + return SFrameError(SFrameErrorType::invalid_parameter_error, + "Input too large for OpenSSL"); + } + + return Result::ok(); +} + static Result seal_ctr(CipherSuite suite, input_bytes key, @@ -256,6 +275,7 @@ seal_ctr(CipherSuite suite, input_bytes pt) { SFRAME_VALUE_OR_RETURN(tag_size, cipher_overhead(suite)); + SFRAME_VOID_OR_RETURN(validate_ctr_size(pt.size())); if (ct.size() < pt.size() + tag_size) { return SFrameError(SFrameErrorType::buffer_too_small_error, "Ciphertext buffer too small"); @@ -374,6 +394,7 @@ open_ctr(CipherSuite suite, } auto inner_ct_size = ct.size() - tag_size; + SFRAME_VOID_OR_RETURN(validate_ctr_size(inner_ct_size)); auto inner_ct = ct.subspan(0, inner_ct_size); auto tag = ct.subspan(inner_ct_size, tag_size); From 370fad4fa4dd008574f7706547e061c3977d90cf Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 11:59:13 -0400 Subject: [PATCH 06/21] Use an empty HKDF salt when deriving SFrame keys --- src/sframe.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sframe.cpp b/src/sframe.cpp index a565f2e..7088521 100644 --- a/src/sframe.cpp +++ b/src/sframe.cpp @@ -59,12 +59,12 @@ KeyRecord::from_base_key(CipherSuite suite, SFRAME_VALUE_OR_RETURN(key_size, cipher_key_size(suite)); SFRAME_VALUE_OR_RETURN(nonce_size, cipher_nonce_size(suite)); - const auto empty_byte_string = owned_bytes<1>(); + const auto empty_salt_storage = owned_bytes<1>(); + const auto empty_salt = input_bytes(empty_salt_storage).first(0); const auto key_label = sframe_key_label(suite, key_id); const auto salt_label = sframe_salt_label(suite, key_id); - SFRAME_VALUE_OR_RETURN(secret, - hkdf_extract(suite, empty_byte_string, base_key)); + SFRAME_VALUE_OR_RETURN(secret, hkdf_extract(suite, empty_salt, base_key)); SFRAME_VALUE_OR_RETURN(key, hkdf_expand(suite, secret, key_label, key_size)); SFRAME_VALUE_OR_RETURN(salt, hkdf_expand(suite, secret, salt_label, nonce_size)); From 673c7ea5a5b41e59220311bbe507dcbc92fc322a Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 11:59:23 -0400 Subject: [PATCH 07/21] Document the tradeoffs of the 32-bit tag suite --- include/sframe/sframe.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/sframe/sframe.h b/include/sframe/sframe.h index 397d03e..a1cdce8 100644 --- a/include/sframe/sframe.h +++ b/include/sframe/sframe.h @@ -71,6 +71,8 @@ enum class CipherSuite : uint16_t { AES_128_CTR_HMAC_SHA256_80 = 1, AES_128_CTR_HMAC_SHA256_64 = 2, + // This truncated-HMAC variant offers less forgery resistance than the + // larger-tag suites and should only be used for compatibility. AES_128_CTR_HMAC_SHA256_32 = 3, AES_GCM_128_SHA256 = 4, AES_GCM_256_SHA512 = 5, From ccf45cb7cbbccc1a8cc28efb5012351431cc2a00 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 11:59:38 -0400 Subject: [PATCH 08/21] Zero-fill integer buffers before encoding --- src/header.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/header.cpp b/src/header.cpp index 8225197..22986a7 100644 --- a/src/header.cpp +++ b/src/header.cpp @@ -1,5 +1,7 @@ #include "header.h" +#include + namespace SFRAME_NAMESPACE { static size_t @@ -23,6 +25,7 @@ void encode_uint(uint64_t val, output_bytes buffer) { size_t size = buffer.size(); + std::fill(buffer.begin(), buffer.end(), uint8_t(0)); for (size_t i = 0; i < size && i < 8; i++) { buffer[size - i - 1] = uint8_t(val >> (8 * i)); } From 306ccda4a9a884765cadbb888584190a0003ccf9 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 11:59:46 -0400 Subject: [PATCH 09/21] Reject empty integer encodings when decoding headers --- src/header.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/header.cpp b/src/header.cpp index 22986a7..b1bc9c1 100644 --- a/src/header.cpp +++ b/src/header.cpp @@ -34,6 +34,11 @@ encode_uint(uint64_t val, output_bytes buffer) static Result decode_uint(input_bytes data) { + if (data.empty()) { + return SFrameError(SFrameErrorType::invalid_parameter_error, + "Integer encoding is empty"); + } + if (!data.empty() && data[0] == 0) { return SFrameError(SFrameErrorType::invalid_parameter_error, "Integer is not minimally encoded"); From 041440ecbefd41b226ca018e0d926fb616aad0b3 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 12:00:01 -0400 Subject: [PATCH 10/21] Initialize fixed-capacity vectors before copying data --- include/sframe/vector.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/sframe/vector.h b/include/sframe/vector.h index 79f23e0..019da4e 100644 --- a/include/sframe/vector.h +++ b/include/sframe/vector.h @@ -29,12 +29,14 @@ class vector constexpr vector(std::initializer_list content) { + std::fill(_data.begin(), _data.end(), T()); resize(content.size()); std::copy(content.begin(), content.end(), _data.begin()); } constexpr vector(gsl::span content) { + std::fill(_data.begin(), _data.end(), T()); resize(content.size()); std::copy(content.begin(), content.end(), _data.begin()); } @@ -44,6 +46,7 @@ class vector template constexpr vector(const vector& content) { + std::fill(_data.begin(), _data.end(), T()); resize(content.size()); std::copy(content.begin(), content.end(), _data.begin()); } From 37e4586eb2481e6258d3dfbac26fb378e321b0a6 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 12:00:11 -0400 Subject: [PATCH 11/21] Document that replay handling stays with the caller --- include/sframe/sframe.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/sframe/sframe.h b/include/sframe/sframe.h index a1cdce8..f19c1e8 100644 --- a/include/sframe/sframe.h +++ b/include/sframe/sframe.h @@ -126,6 +126,8 @@ class Context output_bytes ciphertext, input_bytes plaintext, input_bytes metadata); + // This API authenticates and decrypts a frame, but callers must layer their + // own replay protection if they need it. Result unprotect(output_bytes plaintext, input_bytes ciphertext, input_bytes metadata); @@ -180,6 +182,8 @@ class MLSContext : protected Context input_bytes plaintext, input_bytes metadata); + // This API authenticates and decrypts a frame, but callers must layer their + // own replay protection if they need it. Result unprotect(output_bytes plaintext, input_bytes ciphertext, input_bytes metadata); From 0a9de4d1e672a787d7d4326b850e25ddbc47ed26 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 12:00:50 -0400 Subject: [PATCH 12/21] Hide STL base classes behind the wrapper types --- include/sframe/map.h | 23 +++++++++++++++-------- include/sframe/vector.h | 32 ++++++++++++++++++++++++-------- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/include/sframe/map.h b/include/sframe/map.h index b6f8617..914504c 100644 --- a/include/sframe/map.h +++ b/include/sframe/map.h @@ -83,22 +83,29 @@ class map : private vector>, N> namespace SFRAME_NAMESPACE { -// NOTE: NOT RECOMMENDED FOR USE OUTSIDE THIS LIBRARY -// -// We have used public inheritance from std::map to simplify the interface -// here. This works fine for the use cases we have within this library. If you -// choose to use this map type outside this library, you MUST NOT store it as a -// std::map pointer or reference. This will cause memory leaks, because the -// destructor ~std::map is not virtual. template -class map : public std::map +class map : private std::map { private: using parent = std::map; public: + template + void emplace(Args&&... args) + { + parent::emplace(std::forward(args)...); + } + + auto find(const K& key) { return parent::find(key); } + auto find(const K& key) const { return parent::find(key); } + bool contains(const K& key) const { return this->count(key) > 0; } + const V& at(const K& key) const { return parent::at(key); } + V& at(const K& key) { return parent::at(key); } + + void erase(const K& key) { parent::erase(key); } + template void erase_if_key(F&& f) { diff --git a/include/sframe/vector.h b/include/sframe/vector.h index 019da4e..43aebd5 100644 --- a/include/sframe/vector.h +++ b/include/sframe/vector.h @@ -98,15 +98,8 @@ class vector namespace SFRAME_NAMESPACE { -// NOTE: NOT RECOMMENDED FOR USE OUTSIDE THIS LIBRARY -// -// We have used public inheritance from std::vector to simplify the interface -// here. This works fine for the use cases we have within this library. If you -// choose to use this vector type outside this library, you MUST NOT store it as -// a std::vector pointer or reference. This will cause memory leaks, because -// the destructor ~std::vector is not virtual. template -class vector : public std::vector +class vector : private std::vector { private: using parent = std::vector; @@ -133,12 +126,35 @@ class vector : public std::vector { } + T* data() { return parent::data(); } + const T* data() const { return parent::data(); } + + auto begin() const { return parent::begin(); } + auto begin() { return parent::begin(); } + + auto end() const { return parent::end(); } + auto end() { return parent::end(); } + + auto size() const { return parent::size(); } + auto capacity() const { return parent::capacity(); } + void resize(size_t size) { parent::resize(size); } + + auto& operator[](size_t i) { return parent::operator[](i); } + const auto& operator[](size_t i) const { return parent::operator[](i); } + void append(gsl::span content) { const auto start = this->size(); this->resize(start + content.size()); std::copy(content.begin(), content.end(), this->begin() + start); } + + operator gsl::span() const + { + return gsl::span(parent::data(), parent::size()); + } + + operator gsl::span() { return gsl::span(parent::data(), parent::size()); } }; } // namespace SFRAME_NAMESPACE From 4a103f196b12cdd7d6644484ec805b0d728367eb Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 12:02:02 -0400 Subject: [PATCH 13/21] Clear stale OpenSSL errors before backend operations --- src/crypto_boringssl.cpp | 12 ++++++++++++ src/crypto_openssl11.cpp | 13 +++++++++++++ src/crypto_openssl3.cpp | 12 ++++++++++++ 3 files changed, 37 insertions(+) diff --git a/src/crypto_boringssl.cpp b/src/crypto_boringssl.cpp index 4dff820..798b913 100644 --- a/src/crypto_boringssl.cpp +++ b/src/crypto_boringssl.cpp @@ -26,6 +26,12 @@ crypto_error::crypto_error() } #endif +static void +clear_openssl_errors() +{ + ERR_clear_error(); +} + static Result openssl_digest_type(CipherSuite suite) { @@ -71,6 +77,7 @@ openssl_cipher(CipherSuite suite) Result> hkdf_extract(CipherSuite suite, input_bytes salt, input_bytes ikm) { + clear_openssl_errors(); SFRAME_VALUE_OR_RETURN(md, openssl_digest_type(suite)); auto out = owned_bytes(EVP_MD_size(md)); auto out_len = size_t(out.size()); @@ -90,6 +97,7 @@ hkdf_extract(CipherSuite suite, input_bytes salt, input_bytes ikm) Result> hkdf_expand(CipherSuite suite, input_bytes prk, input_bytes info, size_t size) { + clear_openssl_errors(); SFRAME_VALUE_OR_RETURN(md, openssl_digest_type(suite)); auto out = owned_bytes(size); if (1 != HKDF_expand(out.data(), @@ -117,6 +125,7 @@ compute_tag(CipherSuite suite, input_bytes ct, size_t tag_size) { + clear_openssl_errors(); using scoped_hmac_ctx = std::unique_ptr; auto ctx = scoped_hmac_ctx(HMAC_CTX_new(), HMAC_CTX_free); @@ -175,6 +184,7 @@ ctr_crypt(CipherSuite suite, output_bytes out, input_bytes in) { + clear_openssl_errors(); if (out.size() != in.size()) { return SFrameError(SFrameErrorType::buffer_too_small_error, "CTR size mismatch"); @@ -267,6 +277,7 @@ seal_aead(CipherSuite suite, input_bytes aad, input_bytes pt) { + clear_openssl_errors(); SFRAME_VALUE_OR_RETURN(tag_size, cipher_overhead(suite)); if (ct.size() < pt.size() + tag_size) { return SFrameError(SFrameErrorType::buffer_too_small_error, @@ -386,6 +397,7 @@ open_aead(CipherSuite suite, input_bytes aad, input_bytes ct) { + clear_openssl_errors(); SFRAME_VALUE_OR_RETURN(tag_size, cipher_overhead(suite)); if (ct.size() < tag_size) { return SFrameError(SFrameErrorType::buffer_too_small_error, diff --git a/src/crypto_openssl11.cpp b/src/crypto_openssl11.cpp index c4cf527..9b5c6c8 100644 --- a/src/crypto_openssl11.cpp +++ b/src/crypto_openssl11.cpp @@ -32,6 +32,12 @@ crypto_error::crypto_error() } #endif +static void +clear_openssl_errors() +{ + ERR_clear_error(); +} + static Result openssl_digest_type(CipherSuite suite) { @@ -90,6 +96,7 @@ struct HMAC static Result create(CipherSuite suite, input_bytes key) { + clear_openssl_errors(); SFRAME_VALUE_OR_RETURN(type, openssl_digest_type(suite)); auto ctx = scoped_hmac_ctx(HMAC_CTX_new(), HMAC_CTX_free); @@ -146,6 +153,7 @@ struct HMAC Result> hkdf_extract(CipherSuite suite, input_bytes salt, input_bytes ikm) { + clear_openssl_errors(); SFRAME_VALUE_OR_RETURN(h, HMAC::create(suite, salt)); SFRAME_VOID_OR_RETURN(h.write(ikm)); @@ -158,6 +166,7 @@ hkdf_extract(CipherSuite suite, input_bytes salt, input_bytes ikm) Result> hkdf_expand(CipherSuite suite, input_bytes prk, input_bytes info, size_t size) { + clear_openssl_errors(); // Ensure that we need only one hash invocation if (size > max_hkdf_extract_size) { return SFrameError(SFrameErrorType::invalid_parameter_error, @@ -201,6 +210,7 @@ compute_tag(CipherSuite suite, input_bytes ct, size_t tag_size) { + clear_openssl_errors(); auto len_block = owned_bytes<24>(); auto len_view = output_bytes(len_block); encode_uint(aad.size(), len_view.first(8)); @@ -226,6 +236,7 @@ ctr_crypt(CipherSuite suite, output_bytes out, input_bytes in) { + clear_openssl_errors(); if (out.size() != in.size()) { return SFrameError(SFrameErrorType::buffer_too_small_error, "CTR size mismatch"); @@ -318,6 +329,7 @@ seal_aead(CipherSuite suite, input_bytes aad, input_bytes pt) { + clear_openssl_errors(); SFRAME_VALUE_OR_RETURN(tag_size, cipher_overhead(suite)); if (ct.size() < pt.size() + tag_size) { return SFrameError(SFrameErrorType::buffer_too_small_error, @@ -437,6 +449,7 @@ open_aead(CipherSuite suite, input_bytes aad, input_bytes ct) { + clear_openssl_errors(); SFRAME_VALUE_OR_RETURN(tag_size, cipher_overhead(suite)); if (ct.size() < tag_size) { return SFrameError(SFrameErrorType::buffer_too_small_error, diff --git a/src/crypto_openssl3.cpp b/src/crypto_openssl3.cpp index fc03375..67b054e 100644 --- a/src/crypto_openssl3.cpp +++ b/src/crypto_openssl3.cpp @@ -26,6 +26,12 @@ crypto_error::crypto_error() } #endif +static void +clear_openssl_errors() +{ + ERR_clear_error(); +} + static Result openssl_cipher(CipherSuite suite) { @@ -75,6 +81,7 @@ using scoped_evp_kdf_ctx = Result> hkdf_extract(CipherSuite suite, input_bytes salt, input_bytes ikm) { + clear_openssl_errors(); auto mode = EVP_KDF_HKDF_MODE_EXTRACT_ONLY; SFRAME_VALUE_OR_RETURN(digest_name, openssl_digest_name(suite)); auto* salt_ptr = @@ -111,6 +118,7 @@ hkdf_extract(CipherSuite suite, input_bytes salt, input_bytes ikm) Result> hkdf_expand(CipherSuite suite, input_bytes prk, input_bytes info, size_t size) { + clear_openssl_errors(); auto mode = EVP_KDF_HKDF_MODE_EXPAND_ONLY; SFRAME_VALUE_OR_RETURN(digest_name, openssl_digest_name(suite)); auto* prk_ptr = const_cast(reinterpret_cast(prk.data())); @@ -152,6 +160,7 @@ compute_tag(CipherSuite suite, input_bytes ct, size_t tag_size) { + clear_openssl_errors(); using scoped_evp_mac = std::unique_ptr; using scoped_evp_mac_ctx = std::unique_ptr; @@ -215,6 +224,7 @@ ctr_crypt(CipherSuite suite, output_bytes out, input_bytes in) { + clear_openssl_errors(); if (out.size() != in.size()) { return SFrameError(SFrameErrorType::buffer_too_small_error, "CTR size mismatch"); @@ -307,6 +317,7 @@ seal_aead(CipherSuite suite, input_bytes aad, input_bytes pt) { + clear_openssl_errors(); SFRAME_VALUE_OR_RETURN(tag_size, cipher_overhead(suite)); if (ct.size() < pt.size() + tag_size) { return SFrameError(SFrameErrorType::buffer_too_small_error, @@ -426,6 +437,7 @@ open_aead(CipherSuite suite, input_bytes aad, input_bytes ct) { + clear_openssl_errors(); SFRAME_VALUE_OR_RETURN(tag_size, cipher_overhead(suite)); if (ct.size() < tag_size) { return SFrameError(SFrameErrorType::buffer_too_small_error, From 00cfa4f6d5ada296308870c25b7cc49d672be6a1 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 12:02:15 -0400 Subject: [PATCH 14/21] Clarify the scope of the HKDF FIPS override --- src/crypto_openssl11.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crypto_openssl11.cpp b/src/crypto_openssl11.cpp index 9b5c6c8..1e4b9d5 100644 --- a/src/crypto_openssl11.cpp +++ b/src/crypto_openssl11.cpp @@ -104,7 +104,7 @@ struct HMAC // Some FIPS-enabled libraries are overly conservative in their // interpretation of NIST SP 800-131A, which requires HMAC keys to be at // least 112 bits long. That document does not impose that requirement on - // HKDF, so we disable FIPS enforcement for purposes of HKDF. + // HKDF, so this override is limited to the HKDF helper paths in this file. // // https://doi.org/10.6028/NIST.SP.800-131Ar2 static const auto fips_min_hmac_key_len = 14; From cac4c28fc6146ed4fa3ddf25f1a93d0bbc835bf3 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 12:02:29 -0400 Subject: [PATCH 15/21] Document the lifetime requirement for error messages --- include/sframe/result.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/sframe/result.h b/include/sframe/result.h index 5a896c3..0a74c20 100644 --- a/include/sframe/result.h +++ b/include/sframe/result.h @@ -45,6 +45,8 @@ class SFrameError private: SFrameErrorType type_; + // Message storage is borrowed; callers must pass a string with static or + // otherwise stable lifetime. const char* message_ = nullptr; }; From cc75c7f88dd83442018ab20b2912bbd60a1be163 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 12:02:56 -0400 Subject: [PATCH 16/21] Explain why CTR finalization uses a null output buffer --- src/crypto_boringssl.cpp | 2 ++ src/crypto_openssl11.cpp | 2 ++ src/crypto_openssl3.cpp | 2 ++ 3 files changed, 6 insertions(+) diff --git a/src/crypto_boringssl.cpp b/src/crypto_boringssl.cpp index 798b913..e600a76 100644 --- a/src/crypto_boringssl.cpp +++ b/src/crypto_boringssl.cpp @@ -212,6 +212,8 @@ ctr_crypt(CipherSuite suite, return SFrameErrorType::crypto_error; } + // CTR is a streaming mode, so finalization does not emit more bytes and a + // null output pointer is fine here. if (1 != EVP_EncryptFinal(ctx.get(), nullptr, &outlen)) { return SFrameErrorType::crypto_error; } diff --git a/src/crypto_openssl11.cpp b/src/crypto_openssl11.cpp index 1e4b9d5..629f40b 100644 --- a/src/crypto_openssl11.cpp +++ b/src/crypto_openssl11.cpp @@ -264,6 +264,8 @@ ctr_crypt(CipherSuite suite, return SFrameErrorType::crypto_error; } + // CTR is a streaming mode, so finalization does not emit more bytes and a + // null output pointer is fine here. if (1 != EVP_EncryptFinal(ctx.get(), nullptr, &outlen)) { return SFrameErrorType::crypto_error; } diff --git a/src/crypto_openssl3.cpp b/src/crypto_openssl3.cpp index 67b054e..67adba5 100644 --- a/src/crypto_openssl3.cpp +++ b/src/crypto_openssl3.cpp @@ -252,6 +252,8 @@ ctr_crypt(CipherSuite suite, return SFrameErrorType::crypto_error; } + // CTR is a streaming mode, so finalization does not emit more bytes and a + // null output pointer is fine here. if (1 != EVP_EncryptFinal(ctx.get(), nullptr, &outlen)) { return SFrameErrorType::crypto_error; } From 22ebec7aadd837aae4040670bcdfd5c2969a9149 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 12:03:36 -0400 Subject: [PATCH 17/21] Check OpenSSL size conversions before narrowing to int --- src/crypto_boringssl.cpp | 27 +++++++++++++++++++-------- src/crypto_openssl11.cpp | 27 +++++++++++++++++++-------- src/crypto_openssl3.cpp | 25 ++++++++++++++++++------- 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/src/crypto_boringssl.cpp b/src/crypto_boringssl.cpp index e600a76..6b2e0fc 100644 --- a/src/crypto_boringssl.cpp +++ b/src/crypto_boringssl.cpp @@ -32,6 +32,17 @@ clear_openssl_errors() ERR_clear_error(); } +static Result +checked_int(size_t size) +{ + if (size > INT_MAX) { + return SFrameError(SFrameErrorType::invalid_parameter_error, + "Input too large for OpenSSL"); + } + + return static_cast(size); +} + static Result openssl_digest_type(CipherSuite suite) { @@ -133,7 +144,7 @@ compute_tag(CipherSuite suite, // Guard against sending nullptr to HMAC_Init_ex const auto* key_data = auth_key.data(); - auto key_size = static_cast(auth_key.size()); + SFRAME_VALUE_OR_RETURN(key_size, checked_int(auth_key.size())); const auto non_null_zero_length_key = uint8_t(0); if (key_data == nullptr) { key_data = &non_null_zero_length_key; @@ -206,7 +217,7 @@ ctr_crypt(CipherSuite suite, } int outlen = 0; - auto in_size_int = static_cast(in.size()); + SFRAME_VALUE_OR_RETURN(in_size_int, checked_int(in.size())); if (1 != EVP_EncryptUpdate( ctx.get(), out.data(), &outlen, in.data(), in_size_int)) { return SFrameErrorType::crypto_error; @@ -297,7 +308,7 @@ seal_aead(CipherSuite suite, } int outlen = 0; - auto aad_size_int = static_cast(aad.size()); + SFRAME_VALUE_OR_RETURN(aad_size_int, checked_int(aad.size())); if (aad.size() > 0) { if (1 != EVP_EncryptUpdate( ctx.get(), nullptr, &outlen, aad.data(), aad_size_int)) { @@ -305,7 +316,7 @@ seal_aead(CipherSuite suite, } } - auto pt_size_int = static_cast(pt.size()); + SFRAME_VALUE_OR_RETURN(pt_size_int, checked_int(pt.size())); if (1 != EVP_EncryptUpdate( ctx.get(), ct.data(), &outlen, pt.data(), pt_size_int)) { return SFrameErrorType::crypto_error; @@ -319,7 +330,7 @@ seal_aead(CipherSuite suite, auto tag = ct.subspan(pt.size(), tag_size); auto tag_ptr = const_cast(static_cast(tag.data())); - auto tag_size_downcast = static_cast(tag.size()); + SFRAME_VALUE_OR_RETURN(tag_size_downcast, checked_int(tag.size())); if (1 != EVP_CIPHER_CTX_ctrl( ctx.get(), EVP_CTRL_GCM_GET_TAG, tag_size_downcast, tag_ptr)) { return SFrameErrorType::crypto_error; @@ -424,14 +435,14 @@ open_aead(CipherSuite suite, auto tag = ct.subspan(inner_ct_size, tag_size); auto tag_ptr = const_cast(static_cast(tag.data())); - auto tag_size_downcast = static_cast(tag.size()); + SFRAME_VALUE_OR_RETURN(tag_size_downcast, checked_int(tag.size())); if (1 != EVP_CIPHER_CTX_ctrl( ctx.get(), EVP_CTRL_GCM_SET_TAG, tag_size_downcast, tag_ptr)) { return SFrameErrorType::crypto_error; } int out_size; - auto aad_size_int = static_cast(aad.size()); + SFRAME_VALUE_OR_RETURN(aad_size_int, checked_int(aad.size())); if (aad.size() > 0) { if (1 != EVP_DecryptUpdate( ctx.get(), nullptr, &out_size, aad.data(), aad_size_int)) { @@ -439,7 +450,7 @@ open_aead(CipherSuite suite, } } - auto inner_ct_size_int = static_cast(inner_ct_size); + SFRAME_VALUE_OR_RETURN(inner_ct_size_int, checked_int(inner_ct_size)); if (1 != EVP_DecryptUpdate( ctx.get(), pt.data(), &out_size, ct.data(), inner_ct_size_int)) { return SFrameErrorType::crypto_error; diff --git a/src/crypto_openssl11.cpp b/src/crypto_openssl11.cpp index 629f40b..349867d 100644 --- a/src/crypto_openssl11.cpp +++ b/src/crypto_openssl11.cpp @@ -38,6 +38,17 @@ clear_openssl_errors() ERR_clear_error(); } +static Result +checked_int(size_t size) +{ + if (size > INT_MAX) { + return SFrameError(SFrameErrorType::invalid_parameter_error, + "Input too large for OpenSSL"); + } + + return static_cast(size); +} + static Result openssl_digest_type(CipherSuite suite) { @@ -108,7 +119,7 @@ struct HMAC // // https://doi.org/10.6028/NIST.SP.800-131Ar2 static const auto fips_min_hmac_key_len = 14; - auto key_size = static_cast(key.size()); + SFRAME_VALUE_OR_RETURN(key_size, checked_int(key.size())); if (FIPS_mode() != 0 && key_size < fips_min_hmac_key_len) { HMAC_CTX_set_flags(ctx.get(), EVP_MD_CTX_FLAG_NON_FIPS_ALLOW); } @@ -258,7 +269,7 @@ ctr_crypt(CipherSuite suite, } int outlen = 0; - auto in_size_int = static_cast(in.size()); + SFRAME_VALUE_OR_RETURN(in_size_int, checked_int(in.size())); if (1 != EVP_EncryptUpdate( ctx.get(), out.data(), &outlen, in.data(), in_size_int)) { return SFrameErrorType::crypto_error; @@ -349,7 +360,7 @@ seal_aead(CipherSuite suite, } int outlen = 0; - auto aad_size_int = static_cast(aad.size()); + SFRAME_VALUE_OR_RETURN(aad_size_int, checked_int(aad.size())); if (aad.size() > 0) { if (1 != EVP_EncryptUpdate( ctx.get(), nullptr, &outlen, aad.data(), aad_size_int)) { @@ -357,7 +368,7 @@ seal_aead(CipherSuite suite, } } - auto pt_size_int = static_cast(pt.size()); + SFRAME_VALUE_OR_RETURN(pt_size_int, checked_int(pt.size())); if (1 != EVP_EncryptUpdate( ctx.get(), ct.data(), &outlen, pt.data(), pt_size_int)) { return SFrameErrorType::crypto_error; @@ -371,7 +382,7 @@ seal_aead(CipherSuite suite, auto tag = ct.subspan(pt.size(), tag_size); auto tag_ptr = const_cast(static_cast(tag.data())); - auto tag_size_downcast = static_cast(tag.size()); + SFRAME_VALUE_OR_RETURN(tag_size_downcast, checked_int(tag.size())); if (1 != EVP_CIPHER_CTX_ctrl( ctx.get(), EVP_CTRL_GCM_GET_TAG, tag_size_downcast, tag_ptr)) { return SFrameErrorType::crypto_error; @@ -476,14 +487,14 @@ open_aead(CipherSuite suite, auto tag = ct.subspan(inner_ct_size, tag_size); auto tag_ptr = const_cast(static_cast(tag.data())); - auto tag_size_downcast = static_cast(tag.size()); + SFRAME_VALUE_OR_RETURN(tag_size_downcast, checked_int(tag.size())); if (1 != EVP_CIPHER_CTX_ctrl( ctx.get(), EVP_CTRL_GCM_SET_TAG, tag_size_downcast, tag_ptr)) { return SFrameErrorType::crypto_error; } int out_size; - auto aad_size_int = static_cast(aad.size()); + SFRAME_VALUE_OR_RETURN(aad_size_int, checked_int(aad.size())); if (aad.size() > 0) { if (1 != EVP_DecryptUpdate( ctx.get(), nullptr, &out_size, aad.data(), aad_size_int)) { @@ -491,7 +502,7 @@ open_aead(CipherSuite suite, } } - auto inner_ct_size_int = static_cast(inner_ct_size); + SFRAME_VALUE_OR_RETURN(inner_ct_size_int, checked_int(inner_ct_size)); if (1 != EVP_DecryptUpdate( ctx.get(), pt.data(), &out_size, ct.data(), inner_ct_size_int)) { return SFrameErrorType::crypto_error; diff --git a/src/crypto_openssl3.cpp b/src/crypto_openssl3.cpp index 67adba5..dc02a98 100644 --- a/src/crypto_openssl3.cpp +++ b/src/crypto_openssl3.cpp @@ -32,6 +32,17 @@ clear_openssl_errors() ERR_clear_error(); } +static Result +checked_int(size_t size) +{ + if (size > INT_MAX) { + return SFrameError(SFrameErrorType::invalid_parameter_error, + "Input too large for OpenSSL"); + } + + return static_cast(size); +} + static Result openssl_cipher(CipherSuite suite) { @@ -246,7 +257,7 @@ ctr_crypt(CipherSuite suite, } int outlen = 0; - auto in_size_int = static_cast(in.size()); + SFRAME_VALUE_OR_RETURN(in_size_int, checked_int(in.size())); if (1 != EVP_EncryptUpdate( ctx.get(), out.data(), &outlen, in.data(), in_size_int)) { return SFrameErrorType::crypto_error; @@ -337,7 +348,7 @@ seal_aead(CipherSuite suite, } int outlen = 0; - auto aad_size_int = static_cast(aad.size()); + SFRAME_VALUE_OR_RETURN(aad_size_int, checked_int(aad.size())); if (aad.size() > 0) { if (1 != EVP_EncryptUpdate( ctx.get(), nullptr, &outlen, aad.data(), aad_size_int)) { @@ -345,7 +356,7 @@ seal_aead(CipherSuite suite, } } - auto pt_size_int = static_cast(pt.size()); + SFRAME_VALUE_OR_RETURN(pt_size_int, checked_int(pt.size())); if (1 != EVP_EncryptUpdate( ctx.get(), ct.data(), &outlen, pt.data(), pt_size_int)) { return SFrameErrorType::crypto_error; @@ -359,7 +370,7 @@ seal_aead(CipherSuite suite, auto tag = ct.subspan(pt.size(), tag_size); auto tag_ptr = const_cast(static_cast(tag.data())); - auto tag_size_downcast = static_cast(tag.size()); + SFRAME_VALUE_OR_RETURN(tag_size_downcast, checked_int(tag.size())); if (1 != EVP_CIPHER_CTX_ctrl( ctx.get(), EVP_CTRL_GCM_GET_TAG, tag_size_downcast, tag_ptr)) { return SFrameErrorType::crypto_error; @@ -464,14 +475,14 @@ open_aead(CipherSuite suite, auto tag = ct.subspan(inner_ct_size, tag_size); auto tag_ptr = const_cast(static_cast(tag.data())); - auto tag_size_downcast = static_cast(tag.size()); + SFRAME_VALUE_OR_RETURN(tag_size_downcast, checked_int(tag.size())); if (1 != EVP_CIPHER_CTX_ctrl( ctx.get(), EVP_CTRL_GCM_SET_TAG, tag_size_downcast, tag_ptr)) { return SFrameErrorType::crypto_error; } int out_size; - auto aad_size_int = static_cast(aad.size()); + SFRAME_VALUE_OR_RETURN(aad_size_int, checked_int(aad.size())); if (aad.size() > 0) { if (1 != EVP_DecryptUpdate( ctx.get(), nullptr, &out_size, aad.data(), aad_size_int)) { @@ -479,7 +490,7 @@ open_aead(CipherSuite suite, } } - auto inner_ct_size_int = static_cast(inner_ct_size); + SFRAME_VALUE_OR_RETURN(inner_ct_size_int, checked_int(inner_ct_size)); if (1 != EVP_DecryptUpdate( ctx.get(), pt.data(), &out_size, ct.data(), inner_ct_size_int)) { return SFrameErrorType::crypto_error; From 05f3e99e0a8c03e0b08488a82f4843ceadde49dc Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 12:04:12 -0400 Subject: [PATCH 18/21] Harden release builds with compiler and linker flags --- CMakeLists.txt | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index b51961a..05c9de8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -49,6 +49,30 @@ elseif(MSVC) add_definitions(-D_CRT_SECURE_NO_WARNINGS) endif() +if ((CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "GNU") AND NOT WIN32) + set(SFRAME_RELEASE_COMPILE_FLAGS "-fstack-protector-strong -D_FORTIFY_SOURCE=2") + if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") + set(SFRAME_RELEASE_COMPILE_FLAGS + "${SFRAME_RELEASE_COMPILE_FLAGS} -ftrivial-auto-var-init=zero") + endif() + + foreach(config RELEASE RELWITHDEBINFO MINSIZEREL) + set(CMAKE_C_FLAGS_${config} "${CMAKE_C_FLAGS_${config}} ${SFRAME_RELEASE_COMPILE_FLAGS}") + set(CMAKE_CXX_FLAGS_${config} "${CMAKE_CXX_FLAGS_${config}} ${SFRAME_RELEASE_COMPILE_FLAGS}") + endforeach() + + if(APPLE) + foreach(config RELEASE RELWITHDEBINFO MINSIZEREL) + set(CMAKE_EXE_LINKER_FLAGS_${config} "${CMAKE_EXE_LINKER_FLAGS_${config}} -pie") + endforeach() + elseif(UNIX) + foreach(config RELEASE RELWITHDEBINFO MINSIZEREL) + set(CMAKE_EXE_LINKER_FLAGS_${config} + "${CMAKE_EXE_LINKER_FLAGS_${config}} -pie -Wl,-z,relro,-z,now") + endforeach() + endif() +endif() + if (SANITIZERS AND ((CMAKE_CXX_COMPILER_ID STREQUAL "Clang") OR (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")) AND NOT WIN32) set (SANITIZERS "-fsanitize=address -fsanitize=undefined") set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${SANITIZERS}") From d8c1e511852d5a4ca2c24b9a5ae3b9c3a16dd141 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 12:04:44 -0400 Subject: [PATCH 19/21] Finish the STL wrapper cleanup --- include/sframe/map.h | 4 ++-- include/sframe/vector.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/sframe/map.h b/include/sframe/map.h index 914504c..2a796b0 100644 --- a/include/sframe/map.h +++ b/include/sframe/map.h @@ -109,9 +109,9 @@ class map : private std::map template void erase_if_key(F&& f) { - for (auto iter = this->begin(); iter != this->end();) { + for (auto iter = parent::begin(); iter != parent::end();) { if (f(iter->first)) { - iter = this->erase(iter); + iter = parent::erase(iter); } else { ++iter; } diff --git a/include/sframe/vector.h b/include/sframe/vector.h index 43aebd5..d86065a 100644 --- a/include/sframe/vector.h +++ b/include/sframe/vector.h @@ -122,7 +122,7 @@ class vector : private std::vector template constexpr vector(const vector& content) - : parent(content) + : parent(content.begin(), content.end()) { } From 6dd3680b3f3ff49ab25609fd3ecbbd158a4696ed Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 12:06:51 -0400 Subject: [PATCH 20/21] Check unprotect usage in the decrypt path --- src/sframe.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sframe.cpp b/src/sframe.cpp index 7088521..60c28c1 100644 --- a/src/sframe.cpp +++ b/src/sframe.cpp @@ -197,10 +197,6 @@ Context::protect_inner(const Header& header, SFRAME_VOID_OR_RETURN(require_key(header.key_id)); const auto& key_and_salt = keys.at(header.key_id); - if (key_and_salt.usage != KeyUsage::unprotect) { - return SFrameError(SFrameErrorType::invalid_key_usage_error, - "Key is not valid for unprotect"); - } SFRAME_VALUE_OR_RETURN(aad, form_aad(header, metadata)); const auto nonce = form_nonce(header.counter, key_and_salt.salt); @@ -226,6 +222,10 @@ Context::unprotect_inner(const Header& header, SFRAME_VOID_OR_RETURN(require_key(header.key_id)); const auto& key_and_salt = keys.at(header.key_id); + if (key_and_salt.usage != KeyUsage::unprotect) { + return SFrameError(SFrameErrorType::invalid_key_usage_error, + "Key is not valid for unprotect"); + } SFRAME_VALUE_OR_RETURN(aad, form_aad(header, metadata)); const auto nonce = form_nonce(header.counter, key_and_salt.salt); From ee97c4dda32c0517d6101180469f2c2fe35ada31 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 27 Apr 2026 12:44:34 -0400 Subject: [PATCH 21/21] Apply PR review cleanup across shared crypto helpers --- CMakeLists.txt | 24 ------------------------ include/sframe/sframe.h | 6 ------ src/crypto.cpp | 38 ++++++++++++++++++++++++++++++++++++++ src/crypto.h | 6 ++++++ src/crypto_boringssl.cpp | 36 ------------------------------------ src/crypto_openssl11.cpp | 36 ------------------------------------ src/crypto_openssl3.cpp | 36 ------------------------------------ 7 files changed, 44 insertions(+), 138 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 05c9de8..b51961a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -49,30 +49,6 @@ elseif(MSVC) add_definitions(-D_CRT_SECURE_NO_WARNINGS) endif() -if ((CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "GNU") AND NOT WIN32) - set(SFRAME_RELEASE_COMPILE_FLAGS "-fstack-protector-strong -D_FORTIFY_SOURCE=2") - if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") - set(SFRAME_RELEASE_COMPILE_FLAGS - "${SFRAME_RELEASE_COMPILE_FLAGS} -ftrivial-auto-var-init=zero") - endif() - - foreach(config RELEASE RELWITHDEBINFO MINSIZEREL) - set(CMAKE_C_FLAGS_${config} "${CMAKE_C_FLAGS_${config}} ${SFRAME_RELEASE_COMPILE_FLAGS}") - set(CMAKE_CXX_FLAGS_${config} "${CMAKE_CXX_FLAGS_${config}} ${SFRAME_RELEASE_COMPILE_FLAGS}") - endforeach() - - if(APPLE) - foreach(config RELEASE RELWITHDEBINFO MINSIZEREL) - set(CMAKE_EXE_LINKER_FLAGS_${config} "${CMAKE_EXE_LINKER_FLAGS_${config}} -pie") - endforeach() - elseif(UNIX) - foreach(config RELEASE RELWITHDEBINFO MINSIZEREL) - set(CMAKE_EXE_LINKER_FLAGS_${config} - "${CMAKE_EXE_LINKER_FLAGS_${config}} -pie -Wl,-z,relro,-z,now") - endforeach() - endif() -endif() - if (SANITIZERS AND ((CMAKE_CXX_COMPILER_ID STREQUAL "Clang") OR (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")) AND NOT WIN32) set (SANITIZERS "-fsanitize=address -fsanitize=undefined") set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${SANITIZERS}") diff --git a/include/sframe/sframe.h b/include/sframe/sframe.h index f19c1e8..397d03e 100644 --- a/include/sframe/sframe.h +++ b/include/sframe/sframe.h @@ -71,8 +71,6 @@ enum class CipherSuite : uint16_t { AES_128_CTR_HMAC_SHA256_80 = 1, AES_128_CTR_HMAC_SHA256_64 = 2, - // This truncated-HMAC variant offers less forgery resistance than the - // larger-tag suites and should only be used for compatibility. AES_128_CTR_HMAC_SHA256_32 = 3, AES_GCM_128_SHA256 = 4, AES_GCM_256_SHA512 = 5, @@ -126,8 +124,6 @@ class Context output_bytes ciphertext, input_bytes plaintext, input_bytes metadata); - // This API authenticates and decrypts a frame, but callers must layer their - // own replay protection if they need it. Result unprotect(output_bytes plaintext, input_bytes ciphertext, input_bytes metadata); @@ -182,8 +178,6 @@ class MLSContext : protected Context input_bytes plaintext, input_bytes metadata); - // This API authenticates and decrypts a frame, but callers must layer their - // own replay protection if they need it. Result unprotect(output_bytes plaintext, input_bytes ciphertext, input_bytes metadata); diff --git a/src/crypto.cpp b/src/crypto.cpp index d090d86..8b8a8df 100644 --- a/src/crypto.cpp +++ b/src/crypto.cpp @@ -1,6 +1,10 @@ #include "crypto.h" #include "header.h" +#include + +#include + namespace SFRAME_NAMESPACE { Result @@ -94,4 +98,38 @@ cipher_overhead(CipherSuite suite) } } +Result +checked_int(size_t size) +{ + if (size > INT_MAX) { + return SFrameError(SFrameErrorType::invalid_parameter_error, + "Input too large for OpenSSL"); + } + + return static_cast(size); +} + +Result +validate_ctr_size(size_t size) +{ + static constexpr uint64_t max_ctr_size = uint64_t(1) << 36; + if (uint64_t(size) > max_ctr_size) { + return SFrameError(SFrameErrorType::invalid_parameter_error, + "CTR input too large"); + } + + auto size_int = checked_int(size); + if (size_int.is_err()) { + return size_int.error(); + } + + return Result::ok(); +} + +void +clear_openssl_errors() +{ + ERR_clear_error(); +} + } // namespace SFRAME_NAMESPACE diff --git a/src/crypto.h b/src/crypto.h index cf95e41..9f1a10e 100644 --- a/src/crypto.h +++ b/src/crypto.h @@ -15,6 +15,12 @@ Result cipher_nonce_size(CipherSuite suite); Result cipher_overhead(CipherSuite suite); +Result +checked_int(size_t size); +Result +validate_ctr_size(size_t size); +void +clear_openssl_errors(); /// /// HKDF diff --git a/src/crypto_boringssl.cpp b/src/crypto_boringssl.cpp index 6b2e0fc..0f1957b 100644 --- a/src/crypto_boringssl.cpp +++ b/src/crypto_boringssl.cpp @@ -11,8 +11,6 @@ #include #include -#include - namespace SFRAME_NAMESPACE { /// @@ -26,23 +24,6 @@ crypto_error::crypto_error() } #endif -static void -clear_openssl_errors() -{ - ERR_clear_error(); -} - -static Result -checked_int(size_t size) -{ - if (size > INT_MAX) { - return SFrameError(SFrameErrorType::invalid_parameter_error, - "Input too large for OpenSSL"); - } - - return static_cast(size); -} - static Result openssl_digest_type(CipherSuite suite) { @@ -232,23 +213,6 @@ ctr_crypt(CipherSuite suite, return Result::ok(); } -static Result -validate_ctr_size(size_t size) -{ - static constexpr size_t max_ctr_size = size_t(uint64_t(1) << 32) * 16; - if (size > max_ctr_size) { - return SFrameError(SFrameErrorType::invalid_parameter_error, - "CTR input too large"); - } - - if (size > INT_MAX) { - return SFrameError(SFrameErrorType::invalid_parameter_error, - "Input too large for OpenSSL"); - } - - return Result::ok(); -} - static Result seal_ctr(CipherSuite suite, input_bytes key, diff --git a/src/crypto_openssl11.cpp b/src/crypto_openssl11.cpp index 349867d..82c0b43 100644 --- a/src/crypto_openssl11.cpp +++ b/src/crypto_openssl11.cpp @@ -9,8 +9,6 @@ #include #include -#include - namespace SFRAME_NAMESPACE { /// @@ -32,23 +30,6 @@ crypto_error::crypto_error() } #endif -static void -clear_openssl_errors() -{ - ERR_clear_error(); -} - -static Result -checked_int(size_t size) -{ - if (size > INT_MAX) { - return SFrameError(SFrameErrorType::invalid_parameter_error, - "Input too large for OpenSSL"); - } - - return static_cast(size); -} - static Result openssl_digest_type(CipherSuite suite) { @@ -284,23 +265,6 @@ ctr_crypt(CipherSuite suite, return Result::ok(); } -static Result -validate_ctr_size(size_t size) -{ - static constexpr size_t max_ctr_size = size_t(uint64_t(1) << 32) * 16; - if (size > max_ctr_size) { - return SFrameError(SFrameErrorType::invalid_parameter_error, - "CTR input too large"); - } - - if (size > INT_MAX) { - return SFrameError(SFrameErrorType::invalid_parameter_error, - "Input too large for OpenSSL"); - } - - return Result::ok(); -} - static Result seal_ctr(CipherSuite suite, input_bytes key, diff --git a/src/crypto_openssl3.cpp b/src/crypto_openssl3.cpp index dc02a98..b19b58d 100644 --- a/src/crypto_openssl3.cpp +++ b/src/crypto_openssl3.cpp @@ -11,8 +11,6 @@ #include #include -#include - namespace SFRAME_NAMESPACE { /// @@ -26,23 +24,6 @@ crypto_error::crypto_error() } #endif -static void -clear_openssl_errors() -{ - ERR_clear_error(); -} - -static Result -checked_int(size_t size) -{ - if (size > INT_MAX) { - return SFrameError(SFrameErrorType::invalid_parameter_error, - "Input too large for OpenSSL"); - } - - return static_cast(size); -} - static Result openssl_cipher(CipherSuite suite) { @@ -272,23 +253,6 @@ ctr_crypt(CipherSuite suite, return Result::ok(); } -static Result -validate_ctr_size(size_t size) -{ - static constexpr size_t max_ctr_size = size_t(uint64_t(1) << 32) * 16; - if (size > max_ctr_size) { - return SFrameError(SFrameErrorType::invalid_parameter_error, - "CTR input too large"); - } - - if (size > INT_MAX) { - return SFrameError(SFrameErrorType::invalid_parameter_error, - "Input too large for OpenSSL"); - } - - return Result::ok(); -} - static Result seal_ctr(CipherSuite suite, input_bytes key,