Skip to content

Commit 443d7f7

Browse files
nebeiddavidben
andauthored
BoringSSL: Test key import in EVPTest a bit more extensively (#3058)
### Description of changes: **BoringSSL commit google/boringssl@afb970dea4e32a996c67169b5becab199b09c179**: "Test key import in EVPTest a bit more extensively" **Changes (2 files):** - **crypto/evp_extra/evp_test.cc** — Rewrote `ImportKey` to import keys via multiple methods (SPKI/PKCS#8 parsing + raw key import), then check all properties on all constructed keys. Removed unused `FileTest *t` parameter from `GetDigest`/`GetKeyType`. Added `Bits` checking, `EVP_PKEY_cmp` cross-key comparison, and `KeyRole` enum. - **crypto/evp_extra/evp_tests.txt** — Renamed `ExpectRawPrivate` → `RawPrivate`, `ExpectRawPublic` → `RawPublic`. Removed all `ExpectNoRawPrivate`/`ExpectNoRawPublic` lines (absence of attribute now implies no raw key). Added missing `RawPublic`/`RawPrivate` to Ed25519 and X25519 entries. ### Adaptations for AWS-LC: - C++11 compatible (no structured bindings, no `std::optional`/`std::string_view`) - Preserved AWS-LC's `PKCS8VersionOut` and `ExpectFromExplicitParams` handling - Skipped `ECCurve`/`EVP_PKEY_get_ec_curve_nid` (not available in AWS-LC) By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license. --------- Co-authored-by: David Benjamin <davidben@google.com>
1 parent 44766fa commit 443d7f7

5 files changed

Lines changed: 278 additions & 148 deletions

File tree

crypto/evp_extra/evp_test.cc

Lines changed: 221 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,13 @@ OPENSSL_MSVC_PRAGMA(warning(pop))
4040
#include "../test/wycheproof_util.h"
4141

4242

43-
// evp_test dispatches between multiple test types. PrivateKey tests take a key
44-
// name parameter and single block, decode it as a PEM private key, and save it
45-
// under that key name. Decrypt, Sign, and Verify tests take a previously
46-
// imported key name as parameter and test their respective operations.
43+
// evp_test dispatches between multiple test types. PublicKey and PrivateKey
44+
// tests take a key name parameter and key information. If the test is
45+
// successful, the key is saved under that key name. Decrypt, Sign, and Verify
46+
// tests take a previously imported key name as parameter and test their
47+
// respective operations.
4748

48-
static const EVP_MD *GetDigest(FileTest *t, const std::string &name) {
49+
static const EVP_MD *GetDigest(const std::string &name) {
4950
if (name == "MD5") {
5051
return EVP_md5();
5152
} else if (name == "SHA1") {
@@ -79,7 +80,7 @@ static const EVP_MD *GetDigest(FileTest *t, const std::string &name) {
7980
return nullptr;
8081
}
8182

82-
static int GetKeyType(FileTest *t, const std::string &name) {
83+
static int GetKeyType(const std::string &name) {
8384
if (name == "RSA") {
8485
return EVP_PKEY_RSA;
8586
}
@@ -122,115 +123,250 @@ static bool GetRSAPadding(FileTest *t, int *out, const std::string &name) {
122123

123124
using KeyMap = std::map<std::string, bssl::UniquePtr<EVP_PKEY>>;
124125

125-
static bool ImportKey(FileTest *t, KeyMap *key_map,
126-
EVP_PKEY *(*parse_func)(CBS *cbs),
126+
enum class KeyRole { kPublic, kPrivate };
127+
128+
static void CheckRSAParam(FileTest *t, const std::string &attr_name,
129+
const EVP_PKEY *pkey,
130+
const BIGNUM *(*rsa_getter)(const RSA *)) {
131+
SCOPED_TRACE(attr_name);
132+
if (t->HasAttribute(attr_name)) {
133+
bssl::UniquePtr<BIGNUM> want =
134+
HexToBIGNUM(t->GetAttributeOrDie(attr_name).c_str());
135+
ASSERT_TRUE(want);
136+
137+
const RSA *rsa = EVP_PKEY_get0_RSA(pkey);
138+
ASSERT_TRUE(rsa);
139+
const BIGNUM *got = rsa_getter(rsa);
140+
ASSERT_TRUE(got);
141+
EXPECT_EQ(BN_cmp(want.get(), got), 0)
142+
<< "wanted: " << BIGNUMToHex(want.get())
143+
<< "\ngot: " << BIGNUMToHex(got);
144+
}
145+
// We have many test RSA keys so, for now, don't require that all RSA keys
146+
// list out these parameters. That is, the absence of an RSA parameter does
147+
// not currently assert that we omit them.
148+
}
149+
150+
static bool ImportKey(FileTest *t, KeyMap *key_map, KeyRole key_role,
127151
int (*marshal_func)(CBB *cbb, const EVP_PKEY *key)) {
152+
auto parse_func = key_role == KeyRole::kPublic ? &EVP_parse_public_key
153+
: &EVP_parse_private_key;
154+
if (key_role == KeyRole::kPublic) {
155+
marshal_func = &EVP_marshal_public_key;
156+
}
157+
158+
// This test will first import the key from all available methods, then check
159+
// that all properties on all keys match.
160+
std::vector<std::pair<std::string, bssl::UniquePtr<EVP_PKEY>>> keys;
161+
162+
// Parse from SPKI or PKCS#8.
128163
std::vector<uint8_t> input;
129164
if (!t->GetBytes(&input, "Input")) {
130165
return false;
131166
}
132-
133167
CBS cbs;
134168
CBS_init(&cbs, input.data(), input.size());
135-
bssl::UniquePtr<EVP_PKEY> pkey(parse_func(&cbs));
136-
if (!pkey) {
169+
bssl::UniquePtr<EVP_PKEY> new_key(parse_func(&cbs));
170+
if (new_key == nullptr || CBS_len(&cbs) != 0) {
137171
return false;
138172
}
173+
keys.emplace_back(key_role == KeyRole::kPublic ? "spki" : "pkcs8",
174+
std::move(new_key));
139175

140-
std::string key_type;
141-
if (!t->GetAttribute(&key_type, "Type")) {
176+
std::string key_type_str;
177+
if (!t->GetAttribute(&key_type_str, "Type")) {
142178
return false;
143179
}
144-
EXPECT_EQ(GetKeyType(t, key_type), EVP_PKEY_id(pkey.get()));
180+
int key_type = GetKeyType(key_type_str);
145181

146-
if (EVP_PKEY_id(pkey.get()) == EVP_PKEY_EC) {
147-
EC_KEY *ec_key = EVP_PKEY_get0_EC_KEY(pkey.get());
148-
OPENSSL_BEGIN_ALLOW_DEPRECATED
149-
if (t->HasAttribute("ExpectFromExplicitParams")) {
150-
EXPECT_EQ(1, EC_KEY_decoded_from_explicit_params(ec_key));
151-
} else {
152-
EXPECT_EQ(0, EC_KEY_decoded_from_explicit_params(ec_key));
182+
// Import as a raw key.
183+
if (key_role == KeyRole::kPublic && t->HasAttribute("RawPublic")) {
184+
std::vector<uint8_t> raw;
185+
if (!t->GetBytes(&raw, "RawPublic")) {
186+
return false;
153187
}
154-
OPENSSL_END_ALLOW_DEPRECATED
155-
}
156-
157-
// The key must re-encode correctly.
158-
bssl::ScopedCBB cbb;
159-
uint8_t *der;
160-
size_t der_len;
161-
if (!CBB_init(cbb.get(), 0) ||
162-
!marshal_func(cbb.get(), pkey.get()) ||
163-
!CBB_finish(cbb.get(), &der, &der_len)) {
164-
return false;
165-
}
166-
bssl::UniquePtr<uint8_t> free_der(der);
167-
168-
std::vector<uint8_t> output = input;
169-
if (t->HasAttribute("Output") &&
170-
!t->GetBytes(&output, "Output")) {
171-
return false;
172-
}
173-
EXPECT_EQ(Bytes(output), Bytes(der, der_len))
174-
<< "Re-encoding the key did not match.";
175-
176-
if (t->HasAttribute("ExpectNoRawPrivate")) {
177-
size_t len;
178-
EXPECT_FALSE(EVP_PKEY_get_raw_private_key(pkey.get(), nullptr, &len));
179-
} else if (t->HasAttribute("ExpectRawPrivate")) {
180-
std::vector<uint8_t> expected;
181-
if (!t->GetBytes(&expected, "ExpectRawPrivate")) {
188+
new_key.reset(
189+
EVP_PKEY_new_raw_public_key(key_type, nullptr, raw.data(), raw.size()));
190+
if (new_key == nullptr) {
182191
return false;
183192
}
184-
193+
keys.emplace_back("raw public", std::move(new_key));
194+
}
195+
if (key_role == KeyRole::kPrivate && t->HasAttribute("RawPrivate")) {
185196
std::vector<uint8_t> raw;
186-
size_t len;
187-
if (!EVP_PKEY_get_raw_private_key(pkey.get(), nullptr, &len)) {
197+
if (!t->GetBytes(&raw, "RawPrivate")) {
188198
return false;
189199
}
190-
raw.resize(len);
191-
if (!EVP_PKEY_get_raw_private_key(pkey.get(), raw.data(), &len)) {
200+
new_key.reset(EVP_PKEY_new_raw_private_key(key_type, nullptr, raw.data(),
201+
raw.size()));
202+
if (new_key == nullptr) {
192203
return false;
193204
}
194-
raw.resize(len);
195-
EXPECT_EQ(Bytes(raw), Bytes(expected));
196-
197-
// Short buffers should be rejected.
198-
raw.resize(len - 1);
199-
len = raw.size();
200-
EXPECT_FALSE(EVP_PKEY_get_raw_private_key(pkey.get(), raw.data(), &len));
205+
keys.emplace_back("raw private", std::move(new_key));
206+
}
207+
208+
// Import RSA key from parameters.
209+
if (key_type == EVP_PKEY_RSA) {
210+
if (key_role == KeyRole::kPublic && t->HasAttribute("RSAParamN") &&
211+
t->HasAttribute("RSAParamE")) {
212+
bssl::UniquePtr<BIGNUM> n =
213+
HexToBIGNUM(t->GetAttributeOrDie("RSAParamN").c_str());
214+
bssl::UniquePtr<BIGNUM> e =
215+
HexToBIGNUM(t->GetAttributeOrDie("RSAParamE").c_str());
216+
if (n == nullptr || e == nullptr) {
217+
return false;
218+
}
219+
bssl::UniquePtr<RSA> rsa(RSA_new_public_key(n.get(), e.get()));
220+
new_key.reset(EVP_PKEY_new());
221+
if (rsa == nullptr || new_key == nullptr ||
222+
!EVP_PKEY_set1_RSA(new_key.get(), rsa.get())) {
223+
return false;
224+
}
225+
keys.emplace_back("RSA public params", std::move(new_key));
226+
}
227+
if (key_role == KeyRole::kPrivate && t->HasAttribute("RSAParamN") &&
228+
t->HasAttribute("RSAParamE") && t->HasAttribute("RSAParamD") &&
229+
t->HasAttribute("RSAParamP") && t->HasAttribute("RSAParamQ") &&
230+
t->HasAttribute("RSAParamDMP1") && t->HasAttribute("RSAParamDMQ1") &&
231+
t->HasAttribute("RSAParamIQMP")) {
232+
bssl::UniquePtr<BIGNUM> n =
233+
HexToBIGNUM(t->GetAttributeOrDie("RSAParamN").c_str());
234+
bssl::UniquePtr<BIGNUM> e =
235+
HexToBIGNUM(t->GetAttributeOrDie("RSAParamE").c_str());
236+
bssl::UniquePtr<BIGNUM> d =
237+
HexToBIGNUM(t->GetAttributeOrDie("RSAParamD").c_str());
238+
bssl::UniquePtr<BIGNUM> p =
239+
HexToBIGNUM(t->GetAttributeOrDie("RSAParamP").c_str());
240+
bssl::UniquePtr<BIGNUM> q =
241+
HexToBIGNUM(t->GetAttributeOrDie("RSAParamQ").c_str());
242+
bssl::UniquePtr<BIGNUM> dmp1 =
243+
HexToBIGNUM(t->GetAttributeOrDie("RSAParamDMP1").c_str());
244+
bssl::UniquePtr<BIGNUM> dmq1 =
245+
HexToBIGNUM(t->GetAttributeOrDie("RSAParamDMQ1").c_str());
246+
bssl::UniquePtr<BIGNUM> iqmp =
247+
HexToBIGNUM(t->GetAttributeOrDie("RSAParamIQMP").c_str());
248+
if (n == nullptr || e == nullptr) {
249+
return false;
250+
}
251+
bssl::UniquePtr<RSA> rsa(RSA_new_private_key(n.get(), e.get(), d.get(),
252+
p.get(), q.get(), dmp1.get(),
253+
dmq1.get(), iqmp.get()));
254+
new_key.reset(EVP_PKEY_new());
255+
if (rsa == nullptr || new_key == nullptr ||
256+
!EVP_PKEY_set1_RSA(new_key.get(), rsa.get())) {
257+
return false;
258+
}
259+
keys.emplace_back("RSA private params", std::move(new_key));
260+
}
201261
}
202262

203-
if (t->HasAttribute("ExpectNoRawPublic")) {
204-
size_t len;
205-
EXPECT_FALSE(EVP_PKEY_get_raw_public_key(pkey.get(), nullptr, &len));
206-
} else if (t->HasAttribute("ExpectRawPublic")) {
207-
std::vector<uint8_t> expected;
208-
if (!t->GetBytes(&expected, "ExpectRawPublic")) {
209-
return false;
263+
// Check properties of the keys.
264+
for (const auto &entry : keys) {
265+
const std::string &name = entry.first;
266+
const bssl::UniquePtr<EVP_PKEY> &pkey = entry.second;
267+
SCOPED_TRACE(name);
268+
269+
EXPECT_EQ(key_type, EVP_PKEY_id(pkey.get()));
270+
271+
if (t->HasAttribute("Bits")) {
272+
EXPECT_EQ(EVP_PKEY_bits(pkey.get()),
273+
atoi(t->GetAttributeOrDie("Bits").c_str()));
210274
}
211275

212-
std::vector<uint8_t> raw;
213-
size_t len;
214-
if (!EVP_PKEY_get_raw_public_key(pkey.get(), nullptr, &len)) {
276+
if (EVP_PKEY_id(pkey.get()) == EVP_PKEY_EC) {
277+
EC_KEY *ec_key = EVP_PKEY_get0_EC_KEY(pkey.get());
278+
OPENSSL_BEGIN_ALLOW_DEPRECATED
279+
if (t->HasAttribute("ExpectFromExplicitParams")) {
280+
EXPECT_EQ(1, EC_KEY_decoded_from_explicit_params(ec_key));
281+
} else {
282+
EXPECT_EQ(0, EC_KEY_decoded_from_explicit_params(ec_key));
283+
}
284+
OPENSSL_END_ALLOW_DEPRECATED
285+
}
286+
287+
CheckRSAParam(t, "RSAParamN", pkey.get(), RSA_get0_n);
288+
CheckRSAParam(t, "RSAParamE", pkey.get(), RSA_get0_e);
289+
CheckRSAParam(t, "RSAParamD", pkey.get(), RSA_get0_d);
290+
CheckRSAParam(t, "RSAParamP", pkey.get(), RSA_get0_p);
291+
CheckRSAParam(t, "RSAParamQ", pkey.get(), RSA_get0_q);
292+
CheckRSAParam(t, "RSAParamDMP1", pkey.get(), RSA_get0_dmp1);
293+
CheckRSAParam(t, "RSAParamDMQ1", pkey.get(), RSA_get0_dmq1);
294+
CheckRSAParam(t, "RSAParamIQMP", pkey.get(), RSA_get0_iqmp);
295+
296+
// All keys must compare equal.
297+
EXPECT_EQ(EVP_PKEY_cmp(pkey.get(), keys.front().second.get()), 1);
298+
299+
// The key must re-encode correctly.
300+
bssl::ScopedCBB cbb;
301+
if (!CBB_init(cbb.get(), 0) || !marshal_func(cbb.get(), pkey.get())) {
215302
return false;
216303
}
217-
raw.resize(len);
218-
if (!EVP_PKEY_get_raw_public_key(pkey.get(), raw.data(), &len)) {
304+
std::vector<uint8_t> output = input;
305+
if (t->HasAttribute("Output") && !t->GetBytes(&output, "Output")) {
219306
return false;
220307
}
221-
raw.resize(len);
222-
EXPECT_EQ(Bytes(raw), Bytes(expected));
308+
EXPECT_EQ(Bytes(output), Bytes(CBB_data(cbb.get()), CBB_len(cbb.get())))
309+
<< "Re-encoding the key did not match.";
310+
311+
if (t->HasAttribute("RawPrivate")) {
312+
std::vector<uint8_t> expected;
313+
if (!t->GetBytes(&expected, "RawPrivate")) {
314+
return false;
315+
}
316+
317+
std::vector<uint8_t> raw;
318+
size_t len;
319+
if (!EVP_PKEY_get_raw_private_key(pkey.get(), nullptr, &len)) {
320+
return false;
321+
}
322+
raw.resize(len);
323+
if (!EVP_PKEY_get_raw_private_key(pkey.get(), raw.data(), &len)) {
324+
return false;
325+
}
326+
raw.resize(len);
327+
EXPECT_EQ(Bytes(raw), Bytes(expected));
328+
329+
// Short buffers should be rejected.
330+
raw.resize(len - 1);
331+
len = raw.size();
332+
EXPECT_FALSE(EVP_PKEY_get_raw_private_key(pkey.get(), raw.data(), &len));
333+
} else {
334+
size_t len;
335+
EXPECT_FALSE(EVP_PKEY_get_raw_private_key(pkey.get(), nullptr, &len));
336+
}
337+
338+
if (t->HasAttribute("RawPublic")) {
339+
std::vector<uint8_t> expected;
340+
if (!t->GetBytes(&expected, "RawPublic")) {
341+
return false;
342+
}
223343

224-
// Short buffers should be rejected.
225-
raw.resize(len - 1);
226-
len = raw.size();
227-
EXPECT_FALSE(EVP_PKEY_get_raw_public_key(pkey.get(), raw.data(), &len));
344+
std::vector<uint8_t> raw;
345+
size_t len;
346+
if (!EVP_PKEY_get_raw_public_key(pkey.get(), nullptr, &len)) {
347+
return false;
348+
}
349+
raw.resize(len);
350+
if (!EVP_PKEY_get_raw_public_key(pkey.get(), raw.data(), &len)) {
351+
return false;
352+
}
353+
raw.resize(len);
354+
EXPECT_EQ(Bytes(raw), Bytes(expected));
355+
356+
// Short buffers should be rejected.
357+
raw.resize(len - 1);
358+
len = raw.size();
359+
EXPECT_FALSE(EVP_PKEY_get_raw_public_key(pkey.get(), raw.data(), &len));
360+
} else {
361+
size_t len;
362+
EXPECT_FALSE(EVP_PKEY_get_raw_public_key(pkey.get(), nullptr, &len));
363+
}
228364
}
229365

230366
// Save the key for future tests.
231367
const std::string &key_name = t->GetParameter();
232368
EXPECT_EQ(0u, key_map->count(key_name)) << "Duplicate key: " << key_name;
233-
(*key_map)[key_name] = std::move(pkey);
369+
(*key_map)[key_name] = std::move(keys.front().second);
234370
return true;
235371
}
236372

@@ -304,13 +440,13 @@ static bool SetupContext(FileTest *t, KeyMap *key_map, EVP_PKEY_CTX *ctx) {
304440
return false;
305441
}
306442
if (t->HasAttribute("MGF1Digest")) {
307-
const EVP_MD *digest = GetDigest(t, t->GetAttributeOrDie("MGF1Digest"));
443+
const EVP_MD *digest = GetDigest(t->GetAttributeOrDie("MGF1Digest"));
308444
if (digest == nullptr || !EVP_PKEY_CTX_set_rsa_mgf1_md(ctx, digest)) {
309445
return false;
310446
}
311447
}
312448
if (t->HasAttribute("OAEPDigest")) {
313-
const EVP_MD *digest = GetDigest(t, t->GetAttributeOrDie("OAEPDigest"));
449+
const EVP_MD *digest = GetDigest(t->GetAttributeOrDie("OAEPDigest"));
314450
if (digest == nullptr || !EVP_PKEY_CTX_set_rsa_oaep_md(ctx, digest)) {
315451
return false;
316452
}
@@ -439,11 +575,11 @@ static bool TestEVP(FileTest *t, KeyMap *key_map) {
439575
return false;
440576
}
441577
}
442-
return ImportKey(t, key_map, EVP_parse_private_key, marshal_func);
578+
return ImportKey(t, key_map, KeyRole::kPrivate, marshal_func);
443579
}
444580

445581
if (t->GetType() == "PublicKey") {
446-
return ImportKey(t, key_map, EVP_parse_public_key, EVP_marshal_public_key);
582+
return ImportKey(t, key_map, KeyRole::kPublic, EVP_marshal_public_key);
447583
}
448584

449585
if (t->GetType() == "DHKey") {
@@ -490,7 +626,7 @@ static bool TestEVP(FileTest *t, KeyMap *key_map) {
490626

491627
const EVP_MD *digest = nullptr;
492628
if (t->HasAttribute("Digest")) {
493-
digest = GetDigest(t, t->GetAttributeOrDie("Digest"));
629+
digest = GetDigest(t->GetAttributeOrDie("Digest"));
494630
if (digest == nullptr) {
495631
return false;
496632
}

0 commit comments

Comments
 (0)