diff options
author | jsing <> | 2022-02-01 17:13:10 +0000 |
---|---|---|
committer | jsing <> | 2022-02-01 17:13:10 +0000 |
commit | 494d008fc41fd07dcc927e26c8c23d91c2f1564d (patch) | |
tree | 094581913aa4d49534ea38296879b81b985d64b5 | |
parent | a24e6b334919c85f647d6b7188f92923394678bb (diff) | |
download | openbsd-494d008fc41fd07dcc927e26c8c23d91c2f1564d.tar.gz openbsd-494d008fc41fd07dcc927e26c8c23d91c2f1564d.tar.bz2 openbsd-494d008fc41fd07dcc927e26c8c23d91c2f1564d.zip |
Revise signer callback interface.
The current design of tls_sign_cb provides a pointer to a buffer where the
signature needs to be copied, however it fails to provide a length which
could result in buffer overwrites. Furthermore, tls_signer_sign() is
designed such that it allocates and returns ownership to the caller.
Revise tls_sign_cb so that the called function is expected to allocate a
buffer, returning ownership of the buffer (along with its length) to the
caller of the callback. This makes it far easier (and safer) to implement
a tls_sign_cb callback, plus tls_signer_sign can be directly plugged in
(with an appropriate cast).
While here, rename and reorder some arguments - while we will normally
sign a digest, there is no requirement for this to be the case hence use
'input' and 'input_len'. Move padding (an input) before the outputs and
add some additional bounds/return value checks.
This is technically an API/ABI break that would need a libtls major bump,
however since nothing is using the signer interface (outside of regress),
we'll ride the original minor bump.
With input from tb@
ok inoguchi@ tb@
-rw-r--r-- | src/lib/libtls/tls.h | 14 | ||||
-rw-r--r-- | src/lib/libtls/tls_signer.c | 197 |
2 files changed, 128 insertions, 83 deletions
diff --git a/src/lib/libtls/tls.h b/src/lib/libtls/tls.h index 22f04f4023..91166bf9a7 100644 --- a/src/lib/libtls/tls.h +++ b/src/lib/libtls/tls.h | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: tls.h,v 1.59 2022/01/25 21:51:24 eric Exp $ */ | 1 | /* $OpenBSD: tls.h,v 1.60 2022/02/01 17:13:10 jsing Exp $ */ |
2 | /* | 2 | /* |
3 | * Copyright (c) 2014 Joel Sing <jsing@openbsd.org> | 3 | * Copyright (c) 2014 Joel Sing <jsing@openbsd.org> |
4 | * | 4 | * |
@@ -79,9 +79,9 @@ typedef ssize_t (*tls_read_cb)(struct tls *_ctx, void *_buf, size_t _buflen, | |||
79 | void *_cb_arg); | 79 | void *_cb_arg); |
80 | typedef ssize_t (*tls_write_cb)(struct tls *_ctx, const void *_buf, | 80 | typedef ssize_t (*tls_write_cb)(struct tls *_ctx, const void *_buf, |
81 | size_t _buflen, void *_cb_arg); | 81 | size_t _buflen, void *_cb_arg); |
82 | typedef int (*tls_sign_cb)(void *_cb_arg, const char *_hash, | 82 | typedef int (*tls_sign_cb)(void *_cb_arg, const char *_pubkey_hash, |
83 | const uint8_t *_dgst, size_t _dgstlen, uint8_t *_psig, size_t *_psiglen, | 83 | const uint8_t *_input, size_t _input_len, int _padding_type, |
84 | int _padding); | 84 | uint8_t **_out_signature, size_t *_out_signature_len); |
85 | 85 | ||
86 | int tls_init(void); | 86 | int tls_init(void); |
87 | 87 | ||
@@ -224,9 +224,9 @@ int tls_signer_add_keypair_file(struct tls_signer *_signer, | |||
224 | const char *_cert_file, const char *_key_file); | 224 | const char *_cert_file, const char *_key_file); |
225 | int tls_signer_add_keypair_mem(struct tls_signer *_signer, const uint8_t *_cert, | 225 | int tls_signer_add_keypair_mem(struct tls_signer *_signer, const uint8_t *_cert, |
226 | size_t _cert_len, const uint8_t *_key, size_t _key_len); | 226 | size_t _cert_len, const uint8_t *_key, size_t _key_len); |
227 | int tls_signer_sign(struct tls_signer *_signer, const char *_hash, | 227 | int tls_signer_sign(struct tls_signer *_signer, const char *_pubkey_hash, |
228 | const uint8_t *_dgst, size_t _dgstlen, uint8_t **_psig, size_t *_psiglen, | 228 | const uint8_t *_input, size_t _input_len, int _padding_type, |
229 | int _padding); | 229 | uint8_t **_out_signature, size_t *_out_signature_len); |
230 | 230 | ||
231 | #ifdef __cplusplus | 231 | #ifdef __cplusplus |
232 | } | 232 | } |
diff --git a/src/lib/libtls/tls_signer.c b/src/lib/libtls/tls_signer.c index ca7e72f4ca..d6429762e9 100644 --- a/src/lib/libtls/tls_signer.c +++ b/src/lib/libtls/tls_signer.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: tls_signer.c,v 1.2 2022/01/29 02:03:19 inoguchi Exp $ */ | 1 | /* $OpenBSD: tls_signer.c,v 1.3 2022/02/01 17:13:10 jsing Exp $ */ |
2 | /* | 2 | /* |
3 | * Copyright (c) 2021 Eric Faurot <eric@openbsd.org> | 3 | * Copyright (c) 2021 Eric Faurot <eric@openbsd.org> |
4 | * | 4 | * |
@@ -17,7 +17,9 @@ | |||
17 | 17 | ||
18 | #include <limits.h> | 18 | #include <limits.h> |
19 | 19 | ||
20 | #include <openssl/ecdsa.h> | ||
20 | #include <openssl/err.h> | 21 | #include <openssl/err.h> |
22 | #include <openssl/rsa.h> | ||
21 | 23 | ||
22 | #include "tls.h" | 24 | #include "tls.h" |
23 | #include "tls_internal.h" | 25 | #include "tls_internal.h" |
@@ -178,83 +180,94 @@ tls_signer_add_keypair_file(struct tls_signer *signer, const char *cert_file, | |||
178 | 180 | ||
179 | static int | 181 | static int |
180 | tls_sign_rsa(struct tls_signer *signer, struct tls_signer_key *skey, | 182 | tls_sign_rsa(struct tls_signer *signer, struct tls_signer_key *skey, |
181 | const uint8_t *dgst, size_t dgstlen, uint8_t **psig, size_t *psiglen, | 183 | const uint8_t *input, size_t input_len, int padding_type, |
182 | int padding) | 184 | uint8_t **out_signature, size_t *out_signature_len) |
183 | { | 185 | { |
186 | int rsa_size, signature_len; | ||
187 | char *signature = NULL; | ||
184 | 188 | ||
185 | char *buf; | 189 | *out_signature = NULL; |
186 | int siglen, r; | 190 | *out_signature_len = 0; |
187 | 191 | ||
188 | *psig = NULL; | 192 | if (input_len > INT_MAX) { |
189 | *psiglen = 0; | 193 | tls_error_setx(&signer->error, "input too large"); |
190 | |||
191 | siglen = RSA_size(skey->rsa); | ||
192 | if (siglen <= 0) { | ||
193 | tls_error_setx(&signer->error, "incorrect RSA_size: %d", | ||
194 | siglen); | ||
195 | return (-1); | 194 | return (-1); |
196 | } | 195 | } |
197 | 196 | if ((rsa_size = RSA_size(skey->rsa)) <= 0) { | |
198 | if ((buf = malloc(siglen)) == NULL) { | 197 | tls_error_setx(&signer->error, "invalid RSA size: %d", |
199 | tls_error_set(&signer->error, "RSA sign"); | 198 | rsa_size); |
199 | return (-1); | ||
200 | } | ||
201 | if ((signature = calloc(1, rsa_size)) == NULL) { | ||
202 | tls_error_set(&signer->error, "RSA signature"); | ||
200 | return (-1); | 203 | return (-1); |
201 | } | 204 | } |
202 | 205 | ||
203 | r = RSA_private_encrypt((int)dgstlen, dgst, buf, skey->rsa, padding); | 206 | if ((signature_len = RSA_private_encrypt((int)input_len, input, |
204 | if (r == -1) { | 207 | signature, skey->rsa, padding_type)) <= 0) { |
205 | tls_error_setx(&signer->error, "RSA_private_encrypt failed"); | 208 | /* XXX - include further details from libcrypto. */ |
206 | free(buf); | 209 | tls_error_setx(&signer->error, "RSA signing failed"); |
210 | free(signature); | ||
207 | return (-1); | 211 | return (-1); |
208 | } | 212 | } |
209 | 213 | ||
210 | *psig = buf; | 214 | *out_signature = signature; |
211 | *psiglen = (size_t)r; | 215 | *out_signature_len = (size_t)signature_len; |
212 | 216 | ||
213 | return (0); | 217 | return (0); |
214 | } | 218 | } |
215 | 219 | ||
216 | static int | 220 | static int |
217 | tls_sign_ecdsa(struct tls_signer *signer, struct tls_signer_key *skey, | 221 | tls_sign_ecdsa(struct tls_signer *signer, struct tls_signer_key *skey, |
218 | const uint8_t *dgst, size_t dgstlen, uint8_t **psig, size_t *psiglen) | 222 | const uint8_t *input, size_t input_len, int padding_type, |
223 | uint8_t **out_signature, size_t *out_signature_len) | ||
219 | { | 224 | { |
220 | unsigned char *sig; | 225 | unsigned char *signature; |
221 | unsigned int siglen; | 226 | int signature_len; |
222 | 227 | ||
223 | *psig = NULL; | 228 | *out_signature = NULL; |
224 | *psiglen = 0; | 229 | *out_signature_len = 0; |
225 | 230 | ||
226 | siglen = ECDSA_size(skey->ecdsa); | 231 | if (input_len > INT_MAX) { |
227 | if (siglen == 0) { | 232 | tls_error_setx(&signer->error, "digest too large"); |
228 | tls_error_setx(&signer->error, "incorrect ECDSA_size: %u", | ||
229 | siglen); | ||
230 | return (-1); | 233 | return (-1); |
231 | } | 234 | } |
232 | if ((sig = malloc(siglen)) == NULL) { | 235 | if ((signature_len = ECDSA_size(skey->ecdsa)) <= 0) { |
233 | tls_error_set(&signer->error, "ECDSA sign"); | 236 | tls_error_setx(&signer->error, "invalid ECDSA size: %d", |
237 | signature_len); | ||
238 | return (-1); | ||
239 | } | ||
240 | if ((signature = calloc(1, signature_len)) == NULL) { | ||
241 | tls_error_set(&signer->error, "ECDSA signature"); | ||
234 | return (-1); | 242 | return (-1); |
235 | } | 243 | } |
236 | 244 | ||
237 | if (!ECDSA_sign(0, dgst, dgstlen, sig, &siglen, skey->ecdsa)) { | 245 | if (!ECDSA_sign(0, input, input_len, signature, &signature_len, |
238 | tls_error_setx(&signer->error, "ECDSA_sign failed"); | 246 | skey->ecdsa)) { |
239 | free(sig); | 247 | /* XXX - include further details from libcrypto. */ |
248 | tls_error_setx(&signer->error, "ECDSA signing failed"); | ||
249 | free(signature); | ||
240 | return (-1); | 250 | return (-1); |
241 | } | 251 | } |
242 | 252 | ||
243 | *psig = sig; | 253 | *out_signature = signature; |
244 | *psiglen = siglen; | 254 | *out_signature_len = signature_len; |
245 | 255 | ||
246 | return (0); | 256 | return (0); |
247 | } | 257 | } |
248 | 258 | ||
249 | int | 259 | int |
250 | tls_signer_sign(struct tls_signer *signer, const char *hash, | 260 | tls_signer_sign(struct tls_signer *signer, const char *pubkey_hash, |
251 | const uint8_t *dgst, size_t dgstlen, uint8_t **psig, size_t *psiglen, | 261 | const uint8_t *input, size_t input_len, int padding_type, |
252 | int padding) | 262 | uint8_t **out_signature, size_t *out_signature_len) |
253 | { | 263 | { |
254 | struct tls_signer_key *skey; | 264 | struct tls_signer_key *skey; |
255 | 265 | ||
266 | *out_signature = NULL; | ||
267 | *out_signature_len = 0; | ||
268 | |||
256 | for (skey = signer->keys; skey; skey = skey->next) | 269 | for (skey = signer->keys; skey; skey = skey->next) |
257 | if (!strcmp(hash, skey->hash)) | 270 | if (!strcmp(pubkey_hash, skey->hash)) |
258 | break; | 271 | break; |
259 | 272 | ||
260 | if (skey == NULL) { | 273 | if (skey == NULL) { |
@@ -263,38 +276,58 @@ tls_signer_sign(struct tls_signer *signer, const char *hash, | |||
263 | } | 276 | } |
264 | 277 | ||
265 | if (skey->rsa != NULL) | 278 | if (skey->rsa != NULL) |
266 | return tls_sign_rsa(signer, skey, dgst, dgstlen, psig, psiglen, | 279 | return tls_sign_rsa(signer, skey, input, input_len, |
267 | padding); | 280 | padding_type, out_signature, out_signature_len); |
268 | 281 | ||
269 | if (skey->ecdsa != NULL) | 282 | if (skey->ecdsa != NULL) |
270 | return tls_sign_ecdsa(signer, skey, dgst, dgstlen, psig, psiglen); | 283 | return tls_sign_ecdsa(signer, skey, input, input_len, |
284 | padding_type, out_signature, out_signature_len); | ||
271 | 285 | ||
272 | tls_error_setx(&signer->error, "unknown key type"); | 286 | tls_error_setx(&signer->error, "unknown key type"); |
287 | |||
273 | return (-1); | 288 | return (-1); |
274 | } | 289 | } |
275 | 290 | ||
276 | static int | 291 | static int |
277 | tls_rsa_priv_enc(int srclen, const unsigned char *src, unsigned char *to, | 292 | tls_rsa_priv_enc(int from_len, const unsigned char *from, unsigned char *to, |
278 | RSA *rsa, int padding) | 293 | RSA *rsa, int rsa_padding) |
279 | { | 294 | { |
280 | struct tls_config *config; | 295 | struct tls_config *config; |
281 | const char *hash; | 296 | uint8_t *signature = NULL; |
282 | size_t tolen; | 297 | size_t signature_len = 0; |
298 | const char *pubkey_hash; | ||
299 | |||
300 | /* | ||
301 | * This function is called via RSA_private_encrypt() and has to conform | ||
302 | * to its calling convention/signature. The caller is required to | ||
303 | * provide a 'to' buffer of at least RSA_size() bytes. | ||
304 | */ | ||
283 | 305 | ||
284 | hash = RSA_get_ex_data(rsa, 0); | 306 | pubkey_hash = RSA_get_ex_data(rsa, 0); |
285 | config = RSA_get_ex_data(rsa, 1); | 307 | config = RSA_get_ex_data(rsa, 1); |
286 | 308 | ||
287 | if (hash == NULL || config == NULL) | 309 | if (pubkey_hash == NULL || config == NULL) |
288 | return (-1); | 310 | goto err; |
289 | 311 | ||
290 | if (config->sign_cb(config->sign_cb_arg, hash, (const uint8_t *)src, | 312 | if (from_len < 0) |
291 | srclen, (uint8_t *)to, &tolen, padding) == -1) | 313 | goto err; |
292 | return (-1); | ||
293 | 314 | ||
294 | if (tolen > INT_MAX) | 315 | if (config->sign_cb(config->sign_cb_arg, pubkey_hash, from, from_len, |
295 | return (-1); | 316 | rsa_padding, &signature, &signature_len) == -1) |
317 | goto err; | ||
318 | |||
319 | if (signature_len > INT_MAX || (int)signature_len > RSA_size(rsa)) | ||
320 | goto err; | ||
321 | |||
322 | memcpy(to, signature, signature_len); | ||
323 | free(signature); | ||
296 | 324 | ||
297 | return ((int)tolen); | 325 | return ((int)signature_len); |
326 | |||
327 | err: | ||
328 | free(signature); | ||
329 | |||
330 | return (-1); | ||
298 | } | 331 | } |
299 | 332 | ||
300 | RSA_METHOD * | 333 | RSA_METHOD * |
@@ -324,30 +357,42 @@ tls_ecdsa_do_sign(const unsigned char *dgst, int dgst_len, const BIGNUM *inv, | |||
324 | const BIGNUM *rp, EC_KEY *eckey) | 357 | const BIGNUM *rp, EC_KEY *eckey) |
325 | { | 358 | { |
326 | struct tls_config *config; | 359 | struct tls_config *config; |
327 | ECDSA_SIG *sig = NULL; | 360 | ECDSA_SIG *ecdsa_sig = NULL; |
328 | const unsigned char *tsigbuf; | 361 | uint8_t *signature = NULL; |
329 | const char *hash; | 362 | size_t signature_len = 0; |
330 | char *sigbuf; | 363 | const unsigned char *p; |
331 | size_t siglen; | 364 | const char *pubkey_hash; |
332 | 365 | ||
333 | hash = ECDSA_get_ex_data(eckey, 0); | 366 | /* |
367 | * This function is called via ECDSA_do_sign_ex() and has to conform | ||
368 | * to its calling convention/signature. | ||
369 | */ | ||
370 | |||
371 | pubkey_hash = ECDSA_get_ex_data(eckey, 0); | ||
334 | config = ECDSA_get_ex_data(eckey, 1); | 372 | config = ECDSA_get_ex_data(eckey, 1); |
335 | 373 | ||
336 | if (hash == NULL || config == NULL) | 374 | if (pubkey_hash == NULL || config == NULL) |
337 | return (NULL); | 375 | goto err; |
338 | 376 | ||
339 | siglen = ECDSA_size(eckey); | 377 | if (dgst_len < 0) |
340 | if ((sigbuf = malloc(siglen)) == NULL) | 378 | goto err; |
341 | return (NULL); | ||
342 | 379 | ||
343 | if (config->sign_cb(config->sign_cb_arg, hash, dgst, dgst_len, sigbuf, | 380 | if (config->sign_cb(config->sign_cb_arg, pubkey_hash, dgst, dgst_len, |
344 | &siglen, 0) != -1) { | 381 | 0, &signature, &signature_len) == -1) |
345 | tsigbuf = sigbuf; | 382 | goto err; |
346 | sig = d2i_ECDSA_SIG(NULL, &tsigbuf, siglen); | 383 | |
347 | } | 384 | p = signature; |
348 | free(sigbuf); | 385 | if ((ecdsa_sig = d2i_ECDSA_SIG(NULL, &p, signature_len)) == NULL) |
386 | goto err; | ||
387 | |||
388 | free(signature); | ||
389 | |||
390 | return (ecdsa_sig); | ||
391 | |||
392 | err: | ||
393 | free(signature); | ||
349 | 394 | ||
350 | return (sig); | 395 | return (NULL); |
351 | } | 396 | } |
352 | 397 | ||
353 | ECDSA_METHOD * | 398 | ECDSA_METHOD * |