[or-cvs] Refactor crypto error handling to be more like TLS error ha...
Nick Mathewson
nickm at seul.org
Mon Apr 26 18:09:52 UTC 2004
Update of /home/or/cvsroot/src/common
In directory moria.mit.edu:/tmp/cvs-serv11146/src/common
Modified Files:
crypto.c crypto.h
Log Message:
Refactor crypto error handling to be more like TLS error handling:
crypto_perror is a no-no, since an operation can set more than one
error.
Also, fix a bug in the unix crypto_seed_rng: mixing stdio with
/dev/urandom is a bad idea, since fopen can make all kinds of weird
extraneous syscalls (mmap, fcntl, stat64, etc.) and since fread tends
to buffer data in big chunks, thus depleting the entropy pool.
Index: crypto.c
===================================================================
RCS file: /home/or/cvsroot/src/common/crypto.c,v
retrieving revision 1.80
retrieving revision 1.81
diff -u -d -r1.80 -r1.81
--- crypto.c 25 Apr 2004 19:59:38 -0000 1.80
+++ crypto.c 26 Apr 2004 18:09:49 -0000 1.81
@@ -25,6 +25,15 @@
#ifdef HAVE_CTYPE_H
#include <ctype.h>
#endif
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+#ifdef HAVE_FCNTL_H
+#include <fcntl.h>
+#endif
+#ifdef HAVE_SYS_FCNTL_H
+#include <sys/fcntl.h>
+#endif
#include "crypto.h"
#include "log.h"
@@ -97,6 +106,25 @@
static int _crypto_global_initialized = 0;
+
+/* errors */
+static void
+crypto_log_errors(int severity, const char *doing)
+{
+ int err;
+ const char *msg, *lib, *func;
+ while ((err = ERR_get_error()) != 0) {
+ msg = (const char*)ERR_reason_error_string(err);
+ lib = (const char*)ERR_lib_error_string(err);
+ func = (const char*)ERR_func_error_string(err);
+ if (!msg) msg = "(null)";
+ if (doing) {
+ log(severity, "crypto error while %s: %s (in %s:%s)", doing, msg, lib,func);
+ } else {
+ log(severity, "crypto error: %s (in %s:%s)", msg, lib, func);
+ }
+ }
+}
int crypto_global_init()
{
if (!_crypto_global_initialized) {
@@ -199,12 +227,12 @@
}
if (crypto_cipher_set_key(crypto, key)) {
- log_fn(LOG_WARN, "Unable to set key: %s", crypto_perror());
+ crypto_log_errors(LOG_WARN, "setting symmetric key");
goto error;
}
if (crypto_cipher_set_iv(crypto, iv)) {
- log_fn(LOG_WARN, "Unable to set iv: %s", crypto_perror());
+ crypto_log_errors(LOG_WARN, "setting IV");
goto error;
}
@@ -213,10 +241,8 @@
else
r = crypto_cipher_decrypt_init_cipher(crypto);
- if (r) {
- log_fn(LOG_WARN, "Unable to initialize cipher: %s", crypto_perror());
+ if (r)
goto error;
- }
return crypto;
error:
@@ -251,21 +277,26 @@
if (env->key)
RSA_free(env->key);
env->key = RSA_generate_key(PK_BITS,65537, NULL, NULL);
- if (!env->key)
+ if (!env->key) {
+ crypto_log_errors(LOG_WARN, "generating RSA key");
return -1;
+ }
return 0;
}
-int crypto_pk_read_private_key_from_file(crypto_pk_env_t *env, FILE *src)
+static int crypto_pk_read_private_key_from_file(crypto_pk_env_t *env,
+ FILE *src)
{
tor_assert(env && src);
if (env->key)
RSA_free(env->key);
env->key = PEM_read_RSAPrivateKey(src, NULL, NULL, NULL);
- if (!env->key)
+ if (!env->key) {
+ crypto_log_errors(LOG_WARN, "reading private key from file");
return -1;
+ }
return 0;
}
@@ -288,33 +319,13 @@
/* read the private key */
if(crypto_pk_read_private_key_from_file(env, f_pr) < 0) {
- log_fn(LOG_WARN,"Error reading private key : %s",crypto_perror());
fclose(f_pr);
return -1;
}
fclose(f_pr);
/* check the private key */
- switch(crypto_pk_check_key(env)) {
- case 0:
- log_fn(LOG_WARN,"Private key read but is invalid : %s.", crypto_perror());
- return -1;
- case -1:
- log_fn(LOG_WARN,"Private key read but validity checking failed : %s",crypto_perror());
- return -1;
- /* case 1: fall through */
- }
- return 0;
-}
-
-int crypto_pk_read_public_key_from_file(crypto_pk_env_t *env, FILE *src)
-{
- tor_assert(env && src);
-
- if(env->key)
- RSA_free(env->key);
- env->key = PEM_read_RSAPublicKey(src, NULL, NULL, NULL);
- if (!env->key)
+ if (crypto_pk_check_key(env) <= 0)
return -1;
return 0;
@@ -331,8 +342,10 @@
/* Now you can treat b as if it were a file. Just use the
* PEM_*_bio_* functions instead of the non-bio variants.
*/
- if(!PEM_write_bio_RSAPublicKey(b, env->key))
+ if(!PEM_write_bio_RSAPublicKey(b, env->key)) {
+ crypto_log_errors(LOG_WARN, "writing public key to string");
return -1;
+ }
BIO_get_mem_ptr(b, &buf);
BIO_set_close(b, BIO_NOCLOSE); /* so BIO_free doesn't free buf */
@@ -360,8 +373,10 @@
RSA_free(env->key);
env->key = PEM_read_bio_RSAPublicKey(b, NULL, NULL, NULL);
BIO_free(b);
- if(!env->key)
+ if(!env->key) {
+ crypto_log_errors(LOG_WARN, "reading public key from string");
return -1;
+ }
return 0;
}
@@ -382,6 +397,7 @@
return -1;
if (PEM_write_bio_RSAPrivateKey(bio, env->key, NULL,NULL,0,NULL,NULL)
== 0) {
+ crypto_log_errors(LOG_WARN, "writing private key");
BIO_free(bio);
return -1;
}
@@ -395,34 +411,15 @@
return r;
}
-int crypto_pk_write_private_key_to_file(crypto_pk_env_t *env, FILE *dest)
-{
- tor_assert(env && dest);
-
- if (!env->key)
- return -1;
- if (PEM_write_RSAPrivateKey(dest, env->key, NULL, NULL, 0,0, NULL) == 0)
- return -1;
-
- return 0;
-}
-int crypto_pk_write_public_key_to_file(crypto_pk_env_t *env, FILE *dest)
-{
- tor_assert(env && dest);
-
- if (!env->key)
- return -1;
- if (PEM_write_RSAPublicKey(dest, env->key) == 0)
- return -1;
-
- return 0;
-}
-
int crypto_pk_check_key(crypto_pk_env_t *env)
{
+ int r;
tor_assert(env);
- return RSA_check_key(env->key);
+ r = RSA_check_key(env->key);
+ if (r <= 0)
+ crypto_log_errors(LOG_WARN,"checking RSA key");
+ return r;
}
int crypto_pk_cmp_keys(crypto_pk_env_t *a, crypto_pk_env_t *b) {
@@ -459,37 +456,54 @@
int crypto_pk_public_encrypt(crypto_pk_env_t *env, const unsigned char *from, int fromlen, unsigned char *to, int padding)
{
+ int r;
tor_assert(env && from && to);
- return RSA_public_encrypt(fromlen, (unsigned char*)from, to, env->key,
+ r = RSA_public_encrypt(fromlen, (unsigned char*)from, to, env->key,
crypto_get_rsa_padding(padding));
+ if (r<0)
+ crypto_log_errors(LOG_WARN, "performing RSA encryption");
+ return r;
}
int crypto_pk_private_decrypt(crypto_pk_env_t *env, const unsigned char *from, int fromlen, unsigned char *to, int padding)
{
+ int r;
tor_assert(env && from && to && env->key);
if (!env->key->p)
/* Not a private key */
return -1;
- return RSA_private_decrypt(fromlen, (unsigned char*)from, to, env->key,
+ r = RSA_private_decrypt(fromlen, (unsigned char*)from, to, env->key,
crypto_get_rsa_padding(padding));
+ if (r<0)
+ crypto_log_errors(LOG_WARN, "performing RSA decryption");
+ return r;
}
int crypto_pk_public_checksig(crypto_pk_env_t *env, const unsigned char *from, int fromlen, unsigned char *to)
{
+ int r;
tor_assert(env && from && to);
- return RSA_public_decrypt(fromlen, (unsigned char*)from, to, env->key, RSA_PKCS1_PADDING);
+ r = RSA_public_decrypt(fromlen, (unsigned char*)from, to, env->key, RSA_PKCS1_PADDING);
+
+ if (r<0)
+ crypto_log_errors(LOG_WARN, "checking RSA signature");
+ return r;
}
int crypto_pk_private_sign(crypto_pk_env_t *env, const unsigned char *from, int fromlen, unsigned char *to)
{
+ int r;
tor_assert(env && from && to);
if (!env->key->p)
/* Not a private key */
return -1;
- return RSA_private_encrypt(fromlen, (unsigned char*)from, to, env->key, RSA_PKCS1_PADDING);
+ r = RSA_private_encrypt(fromlen, (unsigned char*)from, to, env->key, RSA_PKCS1_PADDING);
+ if (r<0)
+ crypto_log_errors(LOG_WARN, "generating RSA signature");
+ return r;
}
/* Return 0 if sig is a correct signature for SHA1(data). Else return -1.
@@ -667,6 +681,7 @@
cp = buf = tor_malloc(len+1);
len = i2d_RSAPublicKey(pk->key, &cp);
if (len < 0) {
+ crypto_log_errors(LOG_WARN,"encoding public key");
tor_free(buf);
return -1;
}
@@ -696,8 +711,10 @@
memcpy(buf,str,len);
rsa = d2i_RSAPublicKey(NULL, &cp, len);
tor_free(buf);
- if (!rsa)
- return NULL; /* XXXX log openssl error */
+ if (!rsa) {
+ crypto_log_errors(LOG_WARN,"decoding public key");
+ return NULL;
+ }
return _crypto_new_pk_env_rsa(rsa);
}
@@ -715,6 +732,7 @@
buf = bufp = tor_malloc(len+1);
len = i2d_RSAPublicKey(pk->key, &bufp);
if (len < 0) {
+ crypto_log_errors(LOG_WARN,"encoding public key");
free(buf);
return -1;
}
@@ -987,6 +1005,7 @@
return res;
err:
+ crypto_log_errors(LOG_WARN, "creating DH object");
if (res && res->dh) DH_free(res->dh); /* frees p and g too */
if (res) free(res);
return NULL;
@@ -998,8 +1017,10 @@
}
int crypto_dh_generate_public(crypto_dh_env_t *dh)
{
- if (!DH_generate_key(dh->dh))
+ if (!DH_generate_key(dh->dh)) {
+ crypto_log_errors(LOG_WARN, "generating DH key");
return -1;
+ }
return 0;
}
int crypto_dh_get_public(crypto_dh_env_t *dh, char *pubkey, int pubkey_len)
@@ -1007,7 +1028,7 @@
int bytes;
tor_assert(dh);
if (!dh->dh->pub_key) {
- if (!DH_generate_key(dh->dh))
+ if (crypto_dh_generate_public(dh)<0)
return -1;
}
@@ -1053,6 +1074,7 @@
error:
secret_len = -1;
done:
+ crypto_log_errors(LOG_WARN, "completing DH handshake");
if (pubkey_bn)
BN_free(pubkey_bn);
tor_free(secret_tmp);
@@ -1104,16 +1126,16 @@
static char *filenames[] = {
"/dev/srandom", "/dev/urandom", "/dev/random", NULL
};
+ int fd;
int i, n;
char buf[DIGEST_LEN+1];
- FILE *f;
for (i = 0; filenames[i]; ++i) {
- f = fopen(filenames[i], "rb");
- if (!f) continue;
+ fd = open(filenames[i], O_RDONLY, 0);
+ if (fd<0) continue;
log_fn(LOG_INFO, "Seeding RNG from %s", filenames[i]);
- n = fread(buf, 1, DIGEST_LEN, f);
- fclose(f);
+ n = read(fd, buf, DIGEST_LEN);
+ close(fd);
if (n != DIGEST_LEN) {
log_fn(LOG_WARN, "Error reading from entropy source");
return -1;
@@ -1129,8 +1151,12 @@
int crypto_rand(unsigned int n, unsigned char *to)
{
+ int r;
tor_assert(to);
- return (RAND_bytes(to, n) != 1);
+ r = RAND_bytes(to, n);
+ if (r == 0)
+ crypto_log_errors(LOG_WARN, "generating random data");
+ return (r != 1);
}
void crypto_pseudo_rand(unsigned int n, unsigned char *to)
@@ -1138,6 +1164,7 @@
tor_assert(to);
if (RAND_pseudo_bytes(to, n) == -1) {
log_fn(LOG_ERR, "RAND_pseudo_bytes failed unexpectedly.");
+ crypto_log_errors(LOG_WARN, "generating random data");
exit(1);
}
}
@@ -1161,12 +1188,6 @@
}
}
-/* errors */
-const char *crypto_perror()
-{
- return (const char *)ERR_reason_error_string(ERR_get_error());
-}
-
int
base64_encode(char *dest, int destlen, const char *src, int srclen)
{
Index: crypto.h
===================================================================
RCS file: /home/or/cvsroot/src/common/crypto.h,v
retrieving revision 1.40
retrieving revision 1.41
diff -u -d -r1.40 -r1.41
--- crypto.h 8 Apr 2004 20:56:33 -0000 1.40
+++ crypto.h 26 Apr 2004 18:09:49 -0000 1.41
@@ -42,13 +42,9 @@
/* public key crypto */
int crypto_pk_generate_key(crypto_pk_env_t *env);
-int crypto_pk_read_private_key_from_file(crypto_pk_env_t *env, FILE *src);
-int crypto_pk_read_public_key_from_file(crypto_pk_env_t *env, FILE *src);
int crypto_pk_write_public_key_to_string(crypto_pk_env_t *env, char **dest, int *len);
int crypto_pk_read_public_key_from_string(crypto_pk_env_t *env, const char *src, int len);
-int crypto_pk_write_private_key_to_file(crypto_pk_env_t *env, FILE *dest);
int crypto_pk_write_private_key_to_filename(crypto_pk_env_t *env, const char *fname);
-int crypto_pk_write_public_key_to_file(crypto_pk_env_t *env, FILE *dest);
int crypto_pk_check_key(crypto_pk_env_t *env);
int crypto_pk_read_private_key_from_filename(crypto_pk_env_t *env, const char *keyfile);
@@ -128,8 +124,6 @@
void crypto_pseudo_rand(unsigned int n, unsigned char *to);
int crypto_pseudo_rand_int(unsigned int max);
-/* errors */
-const char *crypto_perror();
#endif
/*
More information about the tor-commits
mailing list