summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorjsing <>2022-02-01 17:13:10 +0000
committerjsing <>2022-02-01 17:13:10 +0000
commit494d008fc41fd07dcc927e26c8c23d91c2f1564d (patch)
tree094581913aa4d49534ea38296879b81b985d64b5
parenta24e6b334919c85f647d6b7188f92923394678bb (diff)
downloadopenbsd-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.h14
-rw-r--r--src/lib/libtls/tls_signer.c197
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);
80typedef ssize_t (*tls_write_cb)(struct tls *_ctx, const void *_buf, 80typedef 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);
82typedef int (*tls_sign_cb)(void *_cb_arg, const char *_hash, 82typedef 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
86int tls_init(void); 86int 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);
225int tls_signer_add_keypair_mem(struct tls_signer *_signer, const uint8_t *_cert, 225int 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);
227int tls_signer_sign(struct tls_signer *_signer, const char *_hash, 227int 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
179static int 181static int
180tls_sign_rsa(struct tls_signer *signer, struct tls_signer_key *skey, 182tls_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
216static int 220static int
217tls_sign_ecdsa(struct tls_signer *signer, struct tls_signer_key *skey, 221tls_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
249int 259int
250tls_signer_sign(struct tls_signer *signer, const char *hash, 260tls_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
276static int 291static int
277tls_rsa_priv_enc(int srclen, const unsigned char *src, unsigned char *to, 292tls_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
300RSA_METHOD * 333RSA_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
353ECDSA_METHOD * 398ECDSA_METHOD *