Skip to content

Commit 2726764

Browse files
committed
upstream: refactor sshkey_private_deserialize
feedback/ok markus@ OpenBSD-Commit-ID: f5ca6932fdaf840a5e8250becb38315a29b5fc9f
1 parent 2519a70 commit 2726764

9 files changed

+259
-280
lines changed

ssh-dss.c

+23-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: ssh-dss.c,v 1.47 2022/10/28 00:44:17 djm Exp $ */
1+
/* $OpenBSD: ssh-dss.c,v 1.48 2022/10/28 00:44:44 djm Exp $ */
22
/*
33
* Copyright (c) 2000 Markus Friedl. All rights reserved.
44
*
@@ -235,6 +235,27 @@ ssh_dss_deserialize_public(const char *ktype, struct sshbuf *b,
235235
return ret;
236236
}
237237

238+
static int
239+
ssh_dss_deserialize_private(const char *ktype, struct sshbuf *b,
240+
struct sshkey *key)
241+
{
242+
int r;
243+
BIGNUM *dsa_priv_key = NULL;
244+
245+
if (!sshkey_is_cert(key)) {
246+
if ((r = ssh_dss_deserialize_public(ktype, b, key)) != 0)
247+
return r;
248+
}
249+
250+
if ((r = sshbuf_get_bignum2(b, &dsa_priv_key)) != 0)
251+
return r;
252+
if (!DSA_set0_key(key->dsa, NULL, dsa_priv_key)) {
253+
BN_clear_free(dsa_priv_key);
254+
return SSH_ERR_LIBCRYPTO_ERROR;
255+
}
256+
return 0;
257+
}
258+
238259
static int
239260
ssh_dss_sign(struct sshkey *key,
240261
u_char **sigp, size_t *lenp,
@@ -403,6 +424,7 @@ static const struct sshkey_impl_funcs sshkey_dss_funcs = {
403424
/* .ssh_serialize_public = */ ssh_dss_serialize_public,
404425
/* .ssh_deserialize_public = */ ssh_dss_deserialize_public,
405426
/* .ssh_serialize_private = */ ssh_dss_serialize_private,
427+
/* .ssh_deserialize_private = */ ssh_dss_deserialize_private,
406428
/* .generate = */ ssh_dss_generate,
407429
/* .copy_public = */ ssh_dss_copy_public,
408430
/* .sign = */ ssh_dss_sign,

ssh-ecdsa-sk.c

+19-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: ssh-ecdsa-sk.c,v 1.16 2022/10/28 00:44:17 djm Exp $ */
1+
/* $OpenBSD: ssh-ecdsa-sk.c,v 1.17 2022/10/28 00:44:44 djm Exp $ */
22
/*
33
* Copyright (c) 2000 Markus Friedl. All rights reserved.
44
* Copyright (c) 2010 Damien Miller. All rights reserved.
@@ -137,6 +137,23 @@ ssh_ecdsa_sk_deserialize_public(const char *ktype, struct sshbuf *b,
137137
return 0;
138138
}
139139

140+
static int
141+
ssh_ecdsa_sk_deserialize_private(const char *ktype, struct sshbuf *b,
142+
struct sshkey *key)
143+
{
144+
int r;
145+
146+
if (!sshkey_is_cert(key)) {
147+
if ((r = sshkey_ecdsa_funcs.deserialize_public(ktype,
148+
b, key)) != 0)
149+
return r;
150+
}
151+
if ((r = sshkey_private_deserialize_sk(b, key)) != 0)
152+
return r;
153+
154+
return 0;
155+
}
156+
140157
/*
141158
* Check FIDO/W3C webauthn signatures clientData field against the expected
142159
* format and prepare a hash of it for use in signature verification.
@@ -405,6 +422,7 @@ static const struct sshkey_impl_funcs sshkey_ecdsa_sk_funcs = {
405422
/* .ssh_serialize_public = */ ssh_ecdsa_sk_serialize_public,
406423
/* .ssh_deserialize_public = */ ssh_ecdsa_sk_deserialize_public,
407424
/* .ssh_serialize_private = */ ssh_ecdsa_sk_serialize_private,
425+
/* .ssh_deserialize_private = */ ssh_ecdsa_sk_deserialize_private,
408426
/* .generate = */ NULL,
409427
/* .copy_public = */ ssh_ecdsa_sk_copy_public,
410428
/* .sign = */ NULL,

ssh-ecdsa.c

+47-27
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: ssh-ecdsa.c,v 1.24 2022/10/28 00:44:17 djm Exp $ */
1+
/* $OpenBSD: ssh-ecdsa.c,v 1.25 2022/10/28 00:44:44 djm Exp $ */
22
/*
33
* Copyright (c) 2000 Markus Friedl. All rights reserved.
44
* Copyright (c) 2010 Damien Miller. All rights reserved.
@@ -157,50 +157,69 @@ static int
157157
ssh_ecdsa_deserialize_public(const char *ktype, struct sshbuf *b,
158158
struct sshkey *key)
159159
{
160-
int ret = SSH_ERR_INTERNAL_ERROR;
160+
int r;
161161
char *curve = NULL;
162-
EC_POINT *q = NULL;
163162

164-
key->ecdsa_nid = sshkey_ecdsa_nid_from_name(ktype);
165-
if (sshbuf_get_cstring(b, &curve, NULL) != 0) {
166-
ret = SSH_ERR_INVALID_FORMAT;
163+
if ((key->ecdsa_nid = sshkey_ecdsa_nid_from_name(ktype)) == -1)
164+
return SSH_ERR_INVALID_ARGUMENT;
165+
if ((r = sshbuf_get_cstring(b, &curve, NULL)) != 0)
167166
goto out;
168-
}
169167
if (key->ecdsa_nid != sshkey_curve_name_to_nid(curve)) {
170-
ret = SSH_ERR_EC_CURVE_MISMATCH;
168+
r = SSH_ERR_EC_CURVE_MISMATCH;
171169
goto out;
172170
}
173171
EC_KEY_free(key->ecdsa);
172+
key->ecdsa = NULL;
174173
if ((key->ecdsa = EC_KEY_new_by_curve_name(key->ecdsa_nid)) == NULL) {
175-
ret = SSH_ERR_EC_CURVE_INVALID;
174+
r = SSH_ERR_LIBCRYPTO_ERROR;
176175
goto out;
177176
}
178-
if ((q = EC_POINT_new(EC_KEY_get0_group(key->ecdsa))) == NULL) {
179-
ret = SSH_ERR_ALLOC_FAIL;
177+
if ((r = sshbuf_get_eckey(b, key->ecdsa)) != 0)
180178
goto out;
181-
}
182-
if (sshbuf_get_ec(b, q, EC_KEY_get0_group(key->ecdsa)) != 0) {
183-
ret = SSH_ERR_INVALID_FORMAT;
179+
if (sshkey_ec_validate_public(EC_KEY_get0_group(key->ecdsa),
180+
EC_KEY_get0_public_key(key->ecdsa)) != 0) {
181+
r = SSH_ERR_KEY_INVALID_EC_VALUE;
184182
goto out;
185183
}
186-
if (sshkey_ec_validate_public(EC_KEY_get0_group(key->ecdsa), q) != 0) {
187-
ret = SSH_ERR_KEY_INVALID_EC_VALUE;
188-
goto out;
184+
/* success */
185+
r = 0;
186+
#ifdef DEBUG_PK
187+
sshkey_dump_ec_point(EC_KEY_get0_group(key->ecdsa),
188+
EC_KEY_get0_public_key(key->ecdsa));
189+
#endif
190+
out:
191+
free(curve);
192+
if (r != 0) {
193+
EC_KEY_free(key->ecdsa);
194+
key->ecdsa = NULL;
189195
}
190-
if (EC_KEY_set_public_key(key->ecdsa, q) != 1) {
191-
/* XXX assume it is a allocation error */
192-
ret = SSH_ERR_ALLOC_FAIL;
196+
return r;
197+
}
198+
199+
static int
200+
ssh_ecdsa_deserialize_private(const char *ktype, struct sshbuf *b,
201+
struct sshkey *key)
202+
{
203+
int r;
204+
BIGNUM *exponent = NULL;
205+
206+
if (!sshkey_is_cert(key)) {
207+
if ((r = ssh_ecdsa_deserialize_public(ktype, b, key)) != 0)
208+
return r;
209+
}
210+
if ((r = sshbuf_get_bignum2(b, &exponent)) != 0)
211+
goto out;
212+
if (EC_KEY_set_private_key(key->ecdsa, exponent) != 1) {
213+
r = SSH_ERR_LIBCRYPTO_ERROR;
193214
goto out;
194215
}
195-
#ifdef DEBUG_PK
196-
sshkey_dump_ec_point(EC_KEY_get0_group(key->ecdsa), q);
197-
#endif
216+
if ((r = sshkey_ec_validate_private(key->ecdsa)) != 0)
217+
goto out;
198218
/* success */
199-
ret = 0;
219+
r = 0;
200220
out:
201-
free(curve);
202-
EC_POINT_free(q);
203-
return ret;
221+
BN_clear_free(exponent);
222+
return r;
204223
}
205224

206225
/* ARGSUSED */
@@ -367,6 +386,7 @@ const struct sshkey_impl_funcs sshkey_ecdsa_funcs = {
367386
/* .ssh_serialize_public = */ ssh_ecdsa_serialize_public,
368387
/* .ssh_deserialize_public = */ ssh_ecdsa_deserialize_public,
369388
/* .ssh_serialize_private = */ ssh_ecdsa_serialize_private,
389+
/* .ssh_deserialize_private = */ ssh_ecdsa_deserialize_private,
370390
/* .generate = */ ssh_ecdsa_generate,
371391
/* .copy_public = */ ssh_ecdsa_copy_public,
372392
/* .sign = */ ssh_ecdsa_sign,

ssh-ed25519-sk.c

+15-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: ssh-ed25519-sk.c,v 1.14 2022/10/28 00:44:17 djm Exp $ */
1+
/* $OpenBSD: ssh-ed25519-sk.c,v 1.15 2022/10/28 00:44:44 djm Exp $ */
22
/*
33
* Copyright (c) 2019 Markus Friedl. All rights reserved.
44
*
@@ -108,6 +108,19 @@ ssh_ed25519_sk_deserialize_public(const char *ktype, struct sshbuf *b,
108108
return 0;
109109
}
110110

111+
static int
112+
ssh_ed25519_sk_deserialize_private(const char *ktype, struct sshbuf *b,
113+
struct sshkey *key)
114+
{
115+
int r;
116+
117+
if ((r = sshkey_ed25519_funcs.deserialize_public(ktype, b, key)) != 0)
118+
return r;
119+
if ((r = sshkey_private_deserialize_sk(b, key)) != 0)
120+
return r;
121+
return 0;
122+
}
123+
111124
static int
112125
ssh_ed25519_sk_verify(const struct sshkey *key,
113126
const u_char *sig, size_t siglen,
@@ -243,6 +256,7 @@ static const struct sshkey_impl_funcs sshkey_ed25519_sk_funcs = {
243256
/* .ssh_serialize_public = */ ssh_ed25519_sk_serialize_public,
244257
/* .ssh_deserialize_public = */ ssh_ed25519_sk_deserialize_public,
245258
/* .ssh_serialize_private = */ ssh_ed25519_sk_serialize_private,
259+
/* .ssh_deserialize_private = */ ssh_ed25519_sk_deserialize_private,
246260
/* .generate = */ NULL,
247261
/* .copy_public = */ ssh_ed25519_sk_copy_public,
248262
/* .sign = */ NULL,

ssh-ed25519.c

+27-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: ssh-ed25519.c,v 1.18 2022/10/28 00:44:17 djm Exp $ */
1+
/* $OpenBSD: ssh-ed25519.c,v 1.19 2022/10/28 00:44:44 djm Exp $ */
22
/*
33
* Copyright (c) 2013 Markus Friedl <[email protected]>
44
*
@@ -117,6 +117,31 @@ ssh_ed25519_deserialize_public(const char *ktype, struct sshbuf *b,
117117
return 0;
118118
}
119119

120+
static int
121+
ssh_ed25519_deserialize_private(const char *ktype, struct sshbuf *b,
122+
struct sshkey *key)
123+
{
124+
int r;
125+
size_t sklen = 0;
126+
u_char *ed25519_sk = NULL;
127+
128+
if ((r = ssh_ed25519_deserialize_public(NULL, b, key)) != 0)
129+
goto out;
130+
if ((r = sshbuf_get_string(b, &ed25519_sk, &sklen)) != 0)
131+
goto out;
132+
if (sklen != ED25519_SK_SZ) {
133+
r = SSH_ERR_INVALID_FORMAT;
134+
goto out;
135+
}
136+
key->ed25519_sk = ed25519_sk;
137+
ed25519_sk = NULL; /* transferred */
138+
/* success */
139+
r = 0;
140+
out:
141+
freezero(ed25519_sk, sklen);
142+
return r;
143+
}
144+
120145
static int
121146
ssh_ed25519_sign(struct sshkey *key,
122147
u_char **sigp, size_t *lenp,
@@ -256,6 +281,7 @@ const struct sshkey_impl_funcs sshkey_ed25519_funcs = {
256281
/* .ssh_serialize_public = */ ssh_ed25519_serialize_public,
257282
/* .ssh_deserialize_public = */ ssh_ed25519_deserialize_public,
258283
/* .ssh_serialize_private = */ ssh_ed25519_serialize_private,
284+
/* .ssh_deserialize_private = */ ssh_ed25519_deserialize_private,
259285
/* .generate = */ ssh_ed25519_generate,
260286
/* .copy_public = */ ssh_ed25519_copy_public,
261287
/* .sign = */ ssh_ed25519_sign,

ssh-rsa.c

+56-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: ssh-rsa.c,v 1.76 2022/10/28 00:44:17 djm Exp $ */
1+
/* $OpenBSD: ssh-rsa.c,v 1.77 2022/10/28 00:44:44 djm Exp $ */
22
/*
33
* Copyright (c) 2000, 2003 Markus Friedl <[email protected]>
44
*
@@ -233,6 +233,60 @@ ssh_rsa_deserialize_public(const char *ktype, struct sshbuf *b,
233233
return ret;
234234
}
235235

236+
static int
237+
ssh_rsa_deserialize_private(const char *ktype, struct sshbuf *b,
238+
struct sshkey *key)
239+
{
240+
int r;
241+
BIGNUM *rsa_n = NULL, *rsa_e = NULL, *rsa_d = NULL;
242+
BIGNUM *rsa_iqmp = NULL, *rsa_p = NULL, *rsa_q = NULL;
243+
244+
/* Note: can't reuse ssh_rsa_deserialize_public: e, n vs. n, e */
245+
if (!sshkey_is_cert(key)) {
246+
if ((r = sshbuf_get_bignum2(b, &rsa_n)) != 0 ||
247+
(r = sshbuf_get_bignum2(b, &rsa_e)) != 0)
248+
goto out;
249+
if (!RSA_set0_key(key->rsa, rsa_n, rsa_e, NULL)) {
250+
r = SSH_ERR_LIBCRYPTO_ERROR;
251+
goto out;
252+
}
253+
rsa_n = rsa_e = NULL; /* transferred */
254+
}
255+
if ((r = sshbuf_get_bignum2(b, &rsa_d)) != 0 ||
256+
(r = sshbuf_get_bignum2(b, &rsa_iqmp)) != 0 ||
257+
(r = sshbuf_get_bignum2(b, &rsa_p)) != 0 ||
258+
(r = sshbuf_get_bignum2(b, &rsa_q)) != 0)
259+
goto out;
260+
if (!RSA_set0_key(key->rsa, NULL, NULL, rsa_d)) {
261+
r = SSH_ERR_LIBCRYPTO_ERROR;
262+
goto out;
263+
}
264+
rsa_d = NULL; /* transferred */
265+
if (!RSA_set0_factors(key->rsa, rsa_p, rsa_q)) {
266+
r = SSH_ERR_LIBCRYPTO_ERROR;
267+
goto out;
268+
}
269+
rsa_p = rsa_q = NULL; /* transferred */
270+
if ((r = sshkey_check_rsa_length(key, 0)) != 0)
271+
goto out;
272+
if ((r = ssh_rsa_complete_crt_parameters(key, rsa_iqmp)) != 0)
273+
goto out;
274+
if (RSA_blinding_on(key->rsa, NULL) != 1) {
275+
r = SSH_ERR_LIBCRYPTO_ERROR;
276+
goto out;
277+
}
278+
/* success */
279+
r = 0;
280+
out:
281+
BN_clear_free(rsa_n);
282+
BN_clear_free(rsa_e);
283+
BN_clear_free(rsa_d);
284+
BN_clear_free(rsa_p);
285+
BN_clear_free(rsa_q);
286+
BN_clear_free(rsa_iqmp);
287+
return r;
288+
}
289+
236290
static const char *
237291
rsa_hash_alg_ident(int hash_alg)
238292
{
@@ -652,6 +706,7 @@ static const struct sshkey_impl_funcs sshkey_rsa_funcs = {
652706
/* .ssh_serialize_public = */ ssh_rsa_serialize_public,
653707
/* .ssh_deserialize_public = */ ssh_rsa_deserialize_public,
654708
/* .ssh_serialize_private = */ ssh_rsa_serialize_private,
709+
/* .ssh_deserialize_private = */ ssh_rsa_deserialize_private,
655710
/* .generate = */ ssh_rsa_generate,
656711
/* .copy_public = */ ssh_rsa_copy_public,
657712
/* .sign = */ ssh_rsa_sign,

0 commit comments

Comments
 (0)