Skip to content

Commit c78fb5f

Browse files
philljjphilljj
committed
dh: fix subgroup check in wc_DhAgree.
1 parent fdbfb66 commit c78fb5f

3 files changed

Lines changed: 149 additions & 4 deletions

File tree

‎tests/api/test_dh.c‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,146 @@ int test_wc_DhPublicKeyDecode(void)
7474
return EXPECT_RESULT();
7575
}
7676

77+
/*
78+
* Tests that wc_DhAgree rejects peer public keys that are not in the expected
79+
* subgroup when the group order q is known (SP 800-56Ar3 section 5.6.2.3.1).
80+
* This test not compatible with WOLFSSL_SP_MATH, because p vector is not 2048,
81+
* 3072, or 4096 bits.
82+
*/
83+
int test_wc_DhAgree_subgroup_check(void)
84+
{
85+
EXPECT_DECLS;
86+
#if !defined(NO_DH) && !defined(WOLFSSL_SP_MATH) && !defined(HAVE_SELFTEST) && \
87+
(!defined(HAVE_FIPS) || FIPS_VERSION3_GT(7,0,0)
88+
DhKey key;
89+
WC_RNG rng;
90+
byte agree[64];
91+
92+
/* DSA-style 512-bit group where p - 1 == k * q, where q is
93+
* a 161-bit prime, and g generates the order-q subgroup.
94+
*
95+
* y_bad has order 3 mod p (y^3 mod p == 1). It passes the bounds check
96+
* (2 <= y <= p-2), but is not in the order-q subgroup (y^q mod p != 1).
97+
* */
98+
const byte dsa_dh_p[] = {
99+
0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
100+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
101+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
102+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
103+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
104+
0x00, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x00,
105+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
106+
0x00, 0x00, 0x00, 0xab, 0x80, 0x00, 0x00, 0x47
107+
};
108+
const byte dsa_dh_q[] = {
109+
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
110+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
111+
0x00, 0x00, 0x00, 0x00, 0x07
112+
};
113+
const byte dsa_dh_g[] = {
114+
0x63, 0x23, 0x85, 0x16, 0x66, 0xc1, 0xf1, 0xf0,
115+
0xc3, 0xa0, 0x95, 0xd0, 0x4b, 0x68, 0x92, 0xf3,
116+
0x64, 0x09, 0xbd, 0x89, 0x57, 0x85, 0xfa, 0x44,
117+
0x86, 0xc1, 0x64, 0x86, 0x1e, 0xa1, 0x91, 0x33,
118+
0xd8, 0xaf, 0xb5, 0xa6, 0x28, 0xed, 0xc2, 0x7c,
119+
0x37, 0xa1, 0x5b, 0x0b, 0xf7, 0x6a, 0xa4, 0x61,
120+
0x75, 0x08, 0x16, 0x12, 0xe5, 0x62, 0xc4, 0x89,
121+
0x5a, 0x9c, 0x1b, 0x76, 0xe1, 0x5f, 0x63, 0x21
122+
};
123+
/* Order-3 element: y^3 = 1 mod p, y^q mod p != 1 */
124+
const byte dsa_dh_y_bad[] = {
125+
0x41, 0x61, 0x8f, 0xcd, 0xb7, 0x76, 0xb7, 0xd2,
126+
0x38, 0x46, 0x98, 0x16, 0xa3, 0xfd, 0xd2, 0xa2,
127+
0x71, 0x6a, 0x0b, 0x66, 0x87, 0x3f, 0x6c, 0x15,
128+
0x28, 0x46, 0x79, 0xfb, 0x64, 0x4c, 0x2a, 0x28,
129+
0xe6, 0x65, 0x44, 0x8e, 0xf8, 0x30, 0x4e, 0x73,
130+
0xec, 0x8b, 0x4b, 0xd2, 0x82, 0xd1, 0x87, 0x20,
131+
0x35, 0x47, 0x42, 0xe6, 0x94, 0x49, 0x7c, 0x73,
132+
0xbc, 0x47, 0xcd, 0xcb, 0x88, 0xb8, 0xed, 0x62
133+
};
134+
/* A {priv, pub} keypair generated from these parameters.
135+
* This pair would pass a partial subgroup test. The pub key
136+
* is not actually needed for this test. */
137+
const byte priv[] = {
138+
0xc7, 0x82, 0xd1, 0xc4, 0x13, 0xa8, 0x3f, 0xe0,
139+
0xef, 0xdc, 0xf4, 0x10, 0xf9, 0xf4, 0xc7, 0x0e,
140+
0x30, 0xbc, 0x65, 0x14
141+
};
142+
/*
143+
byte pub[] = {
144+
0x28, 0x2e, 0x2b, 0xf4, 0x4d, 0x83, 0xf5, 0xd8,
145+
0x77, 0x5a, 0xea, 0x7c, 0x4d, 0x89, 0x98, 0x1b,
146+
0xe9, 0x61, 0x69, 0x35, 0x39, 0xdd, 0x1b, 0x2d,
147+
0x5f, 0x35, 0x29, 0x25, 0xd9, 0xec, 0xcc, 0x46,
148+
0x39, 0xfe, 0x3e, 0xe4, 0x67, 0xc9, 0x47, 0xe1,
149+
0xc4, 0x5b, 0x02, 0x95, 0x61, 0x4a, 0x23, 0xdc,
150+
0x3a, 0xf6, 0xce, 0x18, 0x99, 0x1e, 0xe0, 0x72,
151+
0xeb, 0x53, 0x1f, 0xf0, 0xc9, 0x50, 0x9a, 0xc7
152+
};
153+
*/
154+
word32 privSz = sizeof(priv);
155+
word32 agreeSz;
156+
157+
#if defined(WOLFSSL_PUBLIC_MP) && !defined(WOLFSSL_SMALL_STACK)
158+
/* optional sanity checks for the mp used in this dh test. */
159+
const byte expt[1] = {0x03};
160+
mp_int p[1], e[1], q[1];
161+
mp_int g[1], y[1], r[1];
162+
int isPrime = 0;
163+
/* init mp */
164+
ExpectIntEQ(mp_init_multi(p, q, g, y, r, e), MP_OKAY);
165+
/* load mp values */
166+
ExpectIntEQ(mp_read_unsigned_bin(p, dsa_dh_p, sizeof(dsa_dh_p)), MP_OKAY);
167+
ExpectIntEQ(mp_read_unsigned_bin(e, expt, sizeof(expt)), MP_OKAY);
168+
ExpectIntEQ(mp_read_unsigned_bin(q, dsa_dh_q, sizeof(dsa_dh_q)), MP_OKAY);
169+
ExpectIntEQ(mp_read_unsigned_bin(g, dsa_dh_g, sizeof(dsa_dh_g)), MP_OKAY);
170+
ExpectIntEQ(mp_read_unsigned_bin(y, dsa_dh_y_bad, sizeof(dsa_dh_y_bad)),
171+
MP_OKAY);
172+
/* p, q are prime */
173+
ExpectIntEQ(mp_prime_is_prime(p, 8, &isPrime), MP_OKAY);
174+
ExpectIntEQ(isPrime, 1);
175+
ExpectIntEQ(mp_prime_is_prime(q, 8, &isPrime), MP_OKAY);
176+
ExpectIntEQ(isPrime, 1);
177+
/* (y ^ 3) mod p == 1*/
178+
ExpectIntEQ(mp_exptmod(y, e, p, r), MP_OKAY);
179+
ExpectIntEQ(mp_cmp_d(r, 1), MP_EQ);
180+
/* (g ^ q) mod p == 1*/
181+
ExpectIntEQ(mp_exptmod(g, q, p, r), MP_OKAY);
182+
ExpectIntEQ(mp_cmp_d(r, 1), MP_EQ);
183+
/* (y ^ q) mod p != 1*/
184+
ExpectIntEQ(mp_exptmod(y, q, p, r), MP_OKAY);
185+
ExpectIntNE(mp_cmp_d(r, 1), MP_EQ);
186+
/* clear them */
187+
mp_clear(p); mp_clear(e); mp_clear(q);
188+
mp_clear(g); mp_clear(y); mp_clear(r);
189+
#endif /* WOLFSSL_PUBLIC_MP && !WOLFSSL_SMALL_STACK */
190+
191+
XMEMSET(&key, 0, sizeof(DhKey));
192+
XMEMSET(&rng, 0, sizeof(WC_RNG));
193+
194+
ExpectIntEQ(wc_InitRng(&rng), 0);
195+
ExpectIntEQ(wc_InitDhKey(&key), 0);
196+
197+
/* Set DH parameters. */
198+
ExpectIntEQ(wc_DhSetCheckKey(&key, dsa_dh_p, sizeof(dsa_dh_p),
199+
dsa_dh_g, sizeof(dsa_dh_g), dsa_dh_q, sizeof(dsa_dh_q),
200+
0, &rng), 0);
201+
202+
/* sanity: agree with g (valid subgroup element). This should succeed */
203+
agreeSz = sizeof(agree);
204+
ExpectIntEQ(wc_DhAgree(&key, agree, &agreeSz, priv, privSz,
205+
dsa_dh_g, sizeof(dsa_dh_g)), 0);
206+
207+
/* y_bad is not in the order-q subgroup. This should be rejected with
208+
* error DH_CHECK_PUB_E.*/
209+
agreeSz = sizeof(agree);
210+
ExpectIntEQ(wc_DhAgree(&key, agree, &agreeSz, priv, privSz,
211+
dsa_dh_y_bad, sizeof(dsa_dh_y_bad)),
212+
DH_CHECK_PUB_E);
213+
214+
wc_FreeDhKey(&key);
215+
wc_FreeRng(&rng);
216+
#endif /* !NO_DH && !defined(WOLFSSL_SP_MATH) && !HAVE_SELFTEST && etc... */
217+
return EXPECT_RESULT();
218+
}
219+

‎tests/api/test_dh.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@
2525
#include <tests/api/api_decl.h>
2626

2727
int test_wc_DhPublicKeyDecode(void);
28+
int test_wc_DhAgree_subgroup_check(void);
2829

29-
#define TEST_DH_DECLS \
30-
TEST_DECL_GROUP("dh", test_wc_DhPublicKeyDecode)
30+
#define TEST_DH_DECLS \
31+
TEST_DECL_GROUP("dh", test_wc_DhPublicKeyDecode), \
32+
TEST_DECL_GROUP("dh", test_wc_DhAgree_subgroup_check)
3133

3234
#endif /* WOLFCRYPT_TEST_DH_H */

‎wolfcrypt/src/dh.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1612,7 +1612,7 @@ static int _ffc_validate_public_key(DhKey* key, const byte* pub, word32 pubSz,
16121612
}
16131613

16141614
/* SP 800-56Ar3, section 5.6.2.3.1, process step 2 */
1615-
if (ret == 0 && prime != NULL) {
1615+
if (ret == 0 && mp_iszero(q) == MP_NO) {
16161616
#ifdef WOLFSSL_HAVE_SP_DH
16171617
#ifndef WOLFSSL_SP_NO_2048
16181618
if (mp_count_bits(&key->p) == 2048) {
@@ -2053,7 +2053,7 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
20532053
}
20542054
#endif
20552055
/* Always validate peer public key (2 <= y <= p-2) per SP 800-56A */
2056-
if (wc_DhCheckPubKey(key, otherPub, pubSz) != 0) {
2056+
if (wc_DhCheckPubKey_ex(key, otherPub, pubSz, NULL, 0) != 0) {
20572057
WOLFSSL_MSG("wc_DhAgree wc_DhCheckPubKey failed");
20582058
return DH_CHECK_PUB_E;
20592059
}

0 commit comments

Comments
 (0)