Skip to content

Commit

Permalink
clang-tidy conformance and updated tests for FIPS mode
Browse files Browse the repository at this point in the history
  • Loading branch information
ctjhai committed Jan 23, 2023
1 parent 54ffe0b commit 20bd9cf
Show file tree
Hide file tree
Showing 12 changed files with 130 additions and 32 deletions.
10 changes: 6 additions & 4 deletions lib/hpke/src/digest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,11 @@ Digest::hmac_for_hkdf_extract(const bytes& key, const bytes& data) const
make_typed_unique(EVP_MAC_fetch(nullptr, OSSL_MAC_NAME_HMAC, nullptr));
auto ctx = make_typed_unique(EVP_MAC_CTX_new(mac.get()));
auto digest_name = openssl_digest_name(id);
OSSL_PARAM params[2] = { OSSL_PARAM_construct_utf8_string(
OSSL_ALG_PARAM_DIGEST, digest_name.data(), 0),
OSSL_PARAM_construct_end() };
std::array<OSSL_PARAM, 2> params = {
OSSL_PARAM_construct_utf8_string(
OSSL_ALG_PARAM_DIGEST, digest_name.data(), 0),
OSSL_PARAM_construct_end()
};
#else
const auto* type = openssl_digest_type(id);
auto ctx = make_typed_unique(HMAC_CTX_new());
Expand Down Expand Up @@ -155,7 +157,7 @@ Digest::hmac_for_hkdf_extract(const bytes& key, const bytes& data) const
}

#if defined(WITH_OPENSSL3)
if (!EVP_MAC_init(ctx.get(), key_data, key_size, params)) {
if (1 != EVP_MAC_init(ctx.get(), key_data, key_size, params.data())) {
#else
if (1 != HMAC_Init_ex(ctx.get(), key_data, key_size, type, nullptr)) {
#endif
Expand Down
50 changes: 27 additions & 23 deletions lib/hpke/src/group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ struct ECKeyGroup : public EVPGroup
typed_unique_ptr<EVP_PKEY> keypair_evp_key(
const typed_unique_ptr<BIGNUM>& priv) const
{
auto name = OBJ_nid2sn(curve_nid);
const auto* name = OBJ_nid2sn(curve_nid);
if (name == nullptr) {
throw std::runtime_error("Unsupported algorithm");
}
Expand All @@ -185,7 +185,7 @@ struct ECKeyGroup : public EVPGroup
nullptr,
0,
nullptr);
if (!pt_size) {
if (0 == pt_size) {
throw openssl_error();
}

Expand All @@ -201,12 +201,13 @@ struct ECKeyGroup : public EVPGroup

auto builder = make_typed_unique(OSSL_PARAM_BLD_new());
if (builder == nullptr ||
!OSSL_PARAM_BLD_push_utf8_string(
builder.get(), OSSL_PKEY_PARAM_GROUP_NAME, name, 0) ||
!OSSL_PARAM_BLD_push_BN(
builder.get(), OSSL_PKEY_PARAM_PRIV_KEY, priv.get()) ||
!OSSL_PARAM_BLD_push_octet_string(
builder.get(), OSSL_PKEY_PARAM_PUB_KEY, pub.data(), pub.size())) {
1 != OSSL_PARAM_BLD_push_utf8_string(
builder.get(), OSSL_PKEY_PARAM_GROUP_NAME, name, 0) ||
1 != OSSL_PARAM_BLD_push_BN(
builder.get(), OSSL_PKEY_PARAM_PRIV_KEY, priv.get()) ||
1 !=
OSSL_PARAM_BLD_push_octet_string(
builder.get(), OSSL_PKEY_PARAM_PUB_KEY, pub.data(), pub.size())) {
throw openssl_error();
}

Expand Down Expand Up @@ -234,7 +235,7 @@ struct ECKeyGroup : public EVPGroup

typed_unique_ptr<EVP_PKEY> public_evp_key(const bytes& pub) const
{
auto name = OBJ_nid2sn(curve_nid);
const auto* name = OBJ_nid2sn(curve_nid);
if (name == nullptr) {
throw std::runtime_error("Unsupported algorithm");
}
Expand All @@ -247,10 +248,11 @@ struct ECKeyGroup : public EVPGroup

auto builder = make_typed_unique(OSSL_PARAM_BLD_new());
if (builder == nullptr ||
!OSSL_PARAM_BLD_push_utf8_string(
builder.get(), OSSL_PKEY_PARAM_GROUP_NAME, name, 0) ||
!OSSL_PARAM_BLD_push_octet_string(
builder.get(), OSSL_PKEY_PARAM_PUB_KEY, pub.data(), pub.size())) {
1 != OSSL_PARAM_BLD_push_utf8_string(
builder.get(), OSSL_PKEY_PARAM_GROUP_NAME, name, 0) ||
1 !=
OSSL_PARAM_BLD_push_octet_string(
builder.get(), OSSL_PKEY_PARAM_PUB_KEY, pub.data(), pub.size())) {
throw openssl_error();
}

Expand Down Expand Up @@ -337,7 +339,7 @@ struct ECKeyGroup : public EVPGroup
const auto& rpk = dynamic_cast<const PublicKey&>(pk);
#if defined(WITH_OPENSSL3)
OSSL_PARAM* param = nullptr;
if (!EVP_PKEY_todata(rpk.pkey.get(), EVP_PKEY_PUBLIC_KEY, &param)) {
if (1 != EVP_PKEY_todata(rpk.pkey.get(), EVP_PKEY_PUBLIC_KEY, &param)) {
throw openssl_error();
}
auto param_ptr = make_typed_unique(param);
Expand All @@ -349,14 +351,13 @@ struct ECKeyGroup : public EVPGroup
}

size_t len = 0;
if (!OSSL_PARAM_get_octet_string(pk_param, nullptr, 0, &len)) {
if (1 != OSSL_PARAM_get_octet_string(pk_param, nullptr, 0, &len)) {
return bytes({}, 0);
}

bytes buf(len);
auto* buf_ptr = buf.data();
if (!OSSL_PARAM_get_octet_string(
pk_param, reinterpret_cast<void**>(&buf_ptr), len, nullptr)) {
void* data_ptr = buf.data();
if (1 != OSSL_PARAM_get_octet_string(pk_param, &data_ptr, len, nullptr)) {
return bytes({}, 0);
}

Expand All @@ -370,7 +371,9 @@ struct ECKeyGroup : public EVPGroup
return bytes({}, 0);
}
auto point = make_typed_unique(EC_POINT_new(group.get()));
if (!EC_POINT_oct2point(group.get(), point.get(), buf_ptr, len, nullptr)) {
const auto* oct_ptr = static_cast<const unsigned char*>(data_ptr);
if (1 !=
EC_POINT_oct2point(group.get(), point.get(), oct_ptr, len, nullptr)) {
return bytes({}, 0);
}
len = EC_POINT_point2oct(group.get(),
Expand All @@ -379,7 +382,7 @@ struct ECKeyGroup : public EVPGroup
nullptr,
0,
nullptr);
if (!len) {
if (0 == len) {
return bytes({}, 0);
}
bytes out(len);
Expand Down Expand Up @@ -438,7 +441,7 @@ struct ECKeyGroup : public EVPGroup
const auto& rsk = dynamic_cast<const PrivateKey&>(sk);
#if defined(WITH_OPENSSL3)
OSSL_PARAM* param = nullptr;
if (!EVP_PKEY_todata(rsk.pkey.get(), EVP_PKEY_KEYPAIR, &param)) {
if (1 != EVP_PKEY_todata(rsk.pkey.get(), EVP_PKEY_KEYPAIR, &param)) {
throw openssl_error();
}
auto param_ptr = make_typed_unique(param);
Expand All @@ -450,7 +453,7 @@ struct ECKeyGroup : public EVPGroup
}

BIGNUM* d = nullptr;
if (!OSSL_PARAM_get_BN(sk_param, &d)) {
if (1 != OSSL_PARAM_get_BN(sk_param, &d)) {
return bytes({}, 0);
}
auto d_ptr = make_typed_unique(d);
Expand All @@ -473,7 +476,8 @@ struct ECKeyGroup : public EVPGroup
const bytes& skm) const override
{
#if defined(WITH_OPENSSL3)
auto priv = make_typed_unique(BN_bin2bn(skm.data(), skm.size(), nullptr));
auto priv = make_typed_unique(
BN_bin2bn(skm.data(), static_cast<int>(skm.size()), nullptr));
if (priv == nullptr) {
throw std::runtime_error("Unable to deserialize the private key");
}
Expand Down
7 changes: 7 additions & 0 deletions lib/hpke/test/aead.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ TEST_CASE("AEAD Known-Answer")
};

for (const auto& tc : cases) {
if (fips() && fips_disable(tc.id)) {
continue;
}
const auto& aead = select_aead(tc.id);

auto encrypted = aead.seal(tc.key, tc.nonce, tc.aad, tc.plaintext);
Expand All @@ -100,6 +103,10 @@ TEST_CASE("AEAD Round-Trip")
const auto aad = from_hex("04050607");

for (const auto& id : ids) {
if (fips() && fips_disable(id)) {
continue;
}

const auto& aead = select_aead(id);
auto key = bytes(aead.key_size, 0xA0);
auto nonce = bytes(aead.nonce_size, 0xA1);
Expand Down
11 changes: 6 additions & 5 deletions lib/hpke/test/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,14 @@ FIPS_mode()
static int
FIPS_mode_set(int enable)
{
if (enable && fips_prov == nullptr) {
auto retval = 0;
if (enable != 0 && fips_prov == nullptr) {
fips_prov = OSSL_PROVIDER_load(nullptr, "fips");
return fips_prov != nullptr;
} else if (!enable && fips_prov != nullptr) {
return OSSL_PROVIDER_unload(fips_prov);
retval = (fips_prov != nullptr) ? 1 : 0;
} else if (enable == 0 && fips_prov != nullptr) {
retval = OSSL_PROVIDER_unload(fips_prov);
}
return 0;
return retval;
}
#endif

Expand Down
23 changes: 23 additions & 0 deletions lib/mls_vectors/test/fips.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#include "fips.h"
#include <openssl/evp.h>
#include <openssl/provider.h>
#include <set>

using namespace mls;

bool
fips()
{
return OSSL_PROVIDER_available(nullptr, "fips") == 1 ||
EVP_default_properties_is_fips_enabled(nullptr) == 1;
}

bool
is_fips_approved(CipherSuite::ID id)
{
static const auto disallowed = std::set<CipherSuite::ID>{
CipherSuite::ID::X25519_CHACHA20POLY1305_SHA256_Ed25519,
CipherSuite::ID::X448_CHACHA20POLY1305_SHA512_Ed448,
};
return disallowed.count(id) == 0;
}
7 changes: 7 additions & 0 deletions lib/mls_vectors/test/fips.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#include <mls/crypto.h>

bool
fips();

bool
is_fips_approved(mls::CipherSuite::ID id);
9 changes: 9 additions & 0 deletions lib/mls_vectors/test/mls_vectors.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <doctest/doctest.h>
#include <mls_vectors/mls_vectors.h>
#include <tls/tls_syntax.h>
#include "fips.h"

using namespace mls_vectors;

Expand All @@ -22,6 +23,10 @@ TEST_CASE("Tree Math")
TEST_CASE("Encryption Keys")
{
for (auto suite : supported_suites) {
if (fips() && !is_fips_approved(suite.cipher_suite())) {
continue;
}

const auto tv = EncryptionTestVector::create(suite, 15, 10);
REQUIRE(tv.verify() == std::nullopt);
}
Expand All @@ -46,6 +51,10 @@ TEST_CASE("Transcript")
TEST_CASE("TreeKEM")
{
for (auto suite : supported_suites) {
if (fips() && !is_fips_approved(suite.cipher_suite())) {
continue;
}

const auto tv = TreeKEMTestVector::create(suite, 10);
REQUIRE(tv.verify() == std::nullopt);
}
Expand Down
5 changes: 5 additions & 0 deletions test/crypto.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "fips.h"
#include <doctest/doctest.h>
#include <mls/crypto.h>

Expand All @@ -12,6 +13,10 @@ TEST_CASE("Basic HPKE")
auto original = random_bytes(100);

for (auto suite_id : all_supported_suites) {
if (fips() && !is_fips_approved(suite_id)) {
continue;
}

auto suite = CipherSuite{ suite_id };
auto s = bytes{ 0, 1, 2, 3 };

Expand Down
23 changes: 23 additions & 0 deletions test/fips.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#include "fips.h"
#include <openssl/evp.h>
#include <openssl/provider.h>
#include <set>

using namespace mls;

bool
fips()
{
return OSSL_PROVIDER_available(nullptr, "fips") == 1 ||
EVP_default_properties_is_fips_enabled(nullptr) == 1;
}

bool
is_fips_approved(CipherSuite::ID id)
{
static const auto disallowed = std::set<CipherSuite::ID>{
CipherSuite::ID::X25519_CHACHA20POLY1305_SHA256_Ed25519,
CipherSuite::ID::X448_CHACHA20POLY1305_SHA512_Ed448,
};
return disallowed.count(id) == 0;
}
7 changes: 7 additions & 0 deletions test/fips.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#include <mls/crypto.h>

bool
fips();

bool
is_fips_approved(mls::CipherSuite::ID id);
5 changes: 5 additions & 0 deletions test/key_schedule.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "fips.h"
#include <doctest/doctest.h>
#include <mls/state.h>
#include <mls_vectors/mls_vectors.h>
Expand All @@ -8,6 +9,10 @@ using namespace mls_vectors;
TEST_CASE("Encryption Keys Interop")
{
for (auto suite : all_supported_suites) {
if (fips() && !is_fips_approved(suite)) {
continue;
}

const auto tv = EncryptionTestVector::create(suite, 15, 10);
REQUIRE(tv.verify() == std::nullopt);
}
Expand Down
5 changes: 5 additions & 0 deletions test/treekem.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "fips.h"
#include <doctest/doctest.h>
#include <hpke/random.h>
#include <mls/common.h>
Expand Down Expand Up @@ -266,6 +267,10 @@ TEST_CASE_FIXTURE(TreeKEMTest, "TreeKEM encap/decap")
TEST_CASE("TreeKEM Interop")
{
for (auto suite : all_supported_suites) {
if (fips() && !is_fips_approved(suite)) {
continue;
}

auto tv = TreeKEMTestVector::create(suite, 10);
tv.initialize_trees();
REQUIRE(tv.verify() == std::nullopt);
Expand Down

0 comments on commit 20bd9cf

Please sign in to comment.