From 9ee344f5cd5e935c60d3bf7c3ce9ee21895069db Mon Sep 17 00:00:00 2001 From: Rich Salz Date: Wed, 5 Jul 2017 16:08:19 -0400 Subject: [PATCH] Cleanup RAND_load_file,RAND_write_file Document an internal assumption that these are only for use with files, and return an error if not. That made the code much simpler. Leave it as writing 1024 bytes, even though we don't need more than 256 from a security perspective. But the amount isn't specified, now, so we can change it later if we want. Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/3864) --- crypto/err/openssl.txt | 5 + crypto/rand/rand_err.c | 6 + crypto/rand/randfile.c | 238 ++++++++++++++---------------------- doc/man3/RAND_load_file.pod | 38 +++--- include/openssl/randerr.h | 5 + 5 files changed, 130 insertions(+), 162 deletions(-) diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index 4eaef1ae11..0f25aaf743 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -861,6 +861,8 @@ PKCS7_F_PKCS7_SIGN_ADD_SIGNER:137:PKCS7_sign_add_signer PKCS7_F_PKCS7_SIMPLE_SMIMECAP:119:PKCS7_simple_smimecap PKCS7_F_PKCS7_VERIFY:117:PKCS7_verify RAND_F_RAND_BYTES:100:RAND_bytes +RAND_F_RAND_LOAD_FILE:101:RAND_load_file +RAND_F_RAND_WRITE_FILE:102:RAND_write_file RSA_F_CHECK_PADDING_MD:140:check_padding_md RSA_F_ENCODE_PKCS1:146:encode_pkcs1 RSA_F_INT_RSA_VERIFY:145:int_rsa_verify @@ -2096,7 +2098,10 @@ PKCS7_R_UNSUPPORTED_CIPHER_TYPE:111:unsupported cipher type PKCS7_R_UNSUPPORTED_CONTENT_TYPE:112:unsupported content type PKCS7_R_WRONG_CONTENT_TYPE:113:wrong content type PKCS7_R_WRONG_PKCS7_TYPE:114:wrong pkcs7 type +RAND_R_CANNOT_OPEN_FILE:102:Cannot open file RAND_R_FUNC_NOT_IMPLEMENTED:101:Function not implemented +RAND_R_FWRITE_ERROR:103:Error writing file +RAND_R_NOT_A_REGULAR_FILE:104:Not a regular file RAND_R_PRNG_NOT_SEEDED:100:PRNG not seeded RSA_R_ALGORITHM_MISMATCH:100:algorithm mismatch RSA_R_BAD_E_VALUE:101:bad e value diff --git a/crypto/rand/rand_err.c b/crypto/rand/rand_err.c index 6888ed9d9b..3513ac9162 100644 --- a/crypto/rand/rand_err.c +++ b/crypto/rand/rand_err.c @@ -15,12 +15,18 @@ static const ERR_STRING_DATA RAND_str_functs[] = { {ERR_PACK(ERR_LIB_RAND, RAND_F_RAND_BYTES, 0), "RAND_bytes"}, + {ERR_PACK(ERR_LIB_RAND, RAND_F_RAND_LOAD_FILE, 0), "RAND_load_file"}, + {ERR_PACK(ERR_LIB_RAND, RAND_F_RAND_WRITE_FILE, 0), "RAND_write_file"}, {0, NULL} }; static const ERR_STRING_DATA RAND_str_reasons[] = { + {ERR_PACK(ERR_LIB_RAND, 0, RAND_R_CANNOT_OPEN_FILE), "Cannot open file"}, {ERR_PACK(ERR_LIB_RAND, 0, RAND_R_FUNC_NOT_IMPLEMENTED), "Function not implemented"}, + {ERR_PACK(ERR_LIB_RAND, 0, RAND_R_FWRITE_ERROR), "Error writing file"}, + {ERR_PACK(ERR_LIB_RAND, 0, RAND_R_NOT_A_REGULAR_FILE), + "Not a regular file"}, {ERR_PACK(ERR_LIB_RAND, 0, RAND_R_PRNG_NOT_SEEDED), "PRNG not seeded"}, {0, NULL} }; diff --git a/crypto/rand/randfile.c b/crypto/rand/randfile.c index 1c2043e3a3..c60022c45b 100644 --- a/crypto/rand/randfile.c +++ b/crypto/rand/randfile.c @@ -27,6 +27,8 @@ #ifndef OPENSSL_NO_POSIX_IO # include # include +#endif + /* * Following should not be needed, and we could have been stricter * and demand S_IS*. But some systems just don't comply... Formally @@ -34,36 +36,18 @@ * would look like ((m) & MASK == TYPE), but since MASK availability * is as questionable, we settle for this poor-man fallback... */ -# if !defined(S_ISBLK) -# if defined(_S_IFBLK) -# define S_ISBLK(m) ((m) & _S_IFBLK) -# elif defined(S_IFBLK) -# define S_ISBLK(m) ((m) & S_IFBLK) -# elif defined(_WIN32) -# define S_ISBLK(m) 0 /* no concept of block devices on Windows */ -# endif +# if !defined(S_ISREG) +# define S_ISREG(m) ((m) & S_IFREG) # endif -# if !defined(S_ISCHR) -# if defined(_S_IFCHR) -# define S_ISCHR(m) ((m) & _S_IFCHR) -# elif defined(S_IFCHR) -# define S_ISCHR(m) ((m) & S_IFCHR) -# endif -# endif -#endif #ifdef _WIN32 # define stat _stat # define chmod _chmod # define open _open # define fdopen _fdopen -# define fstat _fstat -# define fileno _fileno #endif -#undef BUFSIZE -#define BUFSIZE 1024 -#define RAND_DATA 1024 +#define RAND_FILE_SIZE 1024 #ifdef OPENSSL_SYS_VMS /* @@ -72,23 +56,12 @@ * __FILE_ptr32 is a type provided by DEC C headers (types.h specifically) * to make sure the FILE* is a 32-bit pointer no matter what. We know that * stdio function return this type (a study of stdio.h proves it). - * Additionally, we create a similar char pointer type for the sake of - * vms_setbuf below. */ # if __INITIAL_POINTER_SIZE == 64 # pragma pointer_size save # pragma pointer_size 32 typedef char *char_ptr32; # pragma pointer_size restore -/* - * On VMS, setbuf() will only take 32-bit pointers, and a compilation - * with /POINTER_SIZE=64 will give off a MAYLOSEDATA2 warning here. - * Since we know that the FILE* really is a 32-bit pointer expanded to - * 64 bits, we also know it's safe to convert it back to a 32-bit pointer. - * As for the buffer parameter, we only use NULL here, so that passes as - * well... - */ -# define setbuf(fp,buf) (setbuf)((__FILE_ptr32)(fp), (char_ptr32)(buf)) # endif /* @@ -96,8 +69,9 @@ typedef char *char_ptr32; * passing in sharing options being disabled by /STANDARD=ANSI89 */ static __FILE_ptr32 (*const vms_fopen)(const char *, const char *, ...) = - (__FILE_ptr32 (*)(const char *, const char *, ...))fopen; -# define VMS_OPEN_ATTRS "shr=get,put,upd,del","ctx=bin,stm","rfm=stm","rat=none","mrs=0" + (__FILE_ptr32 (*)(const char *, const char *, ...))fopen; +# define VMS_OPEN_ATTRS \ + "shr=get,put,upd,del","ctx=bin,stm","rfm=stm","rat=none","mrs=0" # define openssl_fopen(fname,mode) vms_fopen((fname), (mode), VMS_OPEN_ATTRS) #endif @@ -106,88 +80,74 @@ static __FILE_ptr32 (*const vms_fopen)(const char *, const char *, ...) = /* * Note that these functions are intended for seed files only. Entropy - * devices and EGD sockets are handled in rand_unix.c + * devices and EGD sockets are handled in rand_unix.c If |bytes| is + * -1 read the complete file; otherwise read the specified amount. */ - int RAND_load_file(const char *file, long bytes) { - /*- - * If bytes >= 0, read up to 'bytes' bytes. - * if bytes == -1, read complete file. - */ - - unsigned char buf[BUFSIZE]; + unsigned char buf[RAND_FILE_SIZE]; #ifndef OPENSSL_NO_POSIX_IO struct stat sb; #endif - int i, ret = 0, n; - FILE *in = NULL; - - if (file == NULL) - return 0; + int i, n, ret = 0; + FILE *in; if (bytes == 0) - return ret; - - in = openssl_fopen(file, "rb"); - if (in == NULL) - goto err; + return 0; #ifndef OPENSSL_NO_POSIX_IO - /* - * struct stat can have padding and unused fields that may not be - * initialized in the call to stat(). We need to clear the entire - * structure before calling RAND_add() to avoid complaints from - * applications such as Valgrind. - */ - memset(&sb, 0, sizeof(sb)); - if (fstat(fileno(in), &sb) < 0) - goto err; - RAND_add(&sb, sizeof(sb), 0.0); - + if (stat(file, &sb) < 0 || !S_ISREG(sb.st_mode)) { + RANDerr(RAND_F_RAND_LOAD_FILE, RAND_R_NOT_A_REGULAR_FILE); + ERR_add_error_data(2, "Filename=", file); + return -1; + } #endif - for (;;) { + if ((in = openssl_fopen(file, "rb")) == NULL) { + RANDerr(RAND_F_RAND_LOAD_FILE, RAND_R_CANNOT_OPEN_FILE); + ERR_add_error_data(2, "Filename=", file); + return -1; + } + + for ( ; ; ) { if (bytes > 0) - n = (bytes < BUFSIZE) ? (int)bytes : BUFSIZE; + n = (bytes < RAND_FILE_SIZE) ? (int)bytes : RAND_FILE_SIZE; else - n = BUFSIZE; + n = RAND_FILE_SIZE; i = fread(buf, 1, n, in); if (i <= 0) break; - RAND_add(buf, i, (double)i); ret += i; - if (bytes > 0) { - bytes -= n; - if (bytes <= 0) - break; - } + + /* If given a bytecount, and we did it, break. */ + if (bytes > 0 && (bytes -= i) <= 0) + break; } - OPENSSL_cleanse(buf, BUFSIZE); - err: - if (in != NULL) - fclose(in); + + OPENSSL_cleanse(buf, sizeof(buf)); + fclose(in); return ret; } int RAND_write_file(const char *file) { - unsigned char buf[BUFSIZE]; - int i, ret = 0, rand_err = 0; + unsigned char buf[RAND_FILE_SIZE]; + int ret = -1; FILE *out = NULL; - int n; #ifndef OPENSSL_NO_POSIX_IO + struct stat sb; -# if defined(S_ISBLK) && defined(S_ISCHR) -# ifdef _WIN32 - /* - * Check for |file| being a driver as "ASCII-safe" on Windows, - * because driver paths are always ASCII. - */ -# endif -# endif + if (stat(file, &sb) >= 0 && !S_ISREG(sb.st_mode)) { + RANDerr(RAND_F_RAND_WRITE_FILE, RAND_R_NOT_A_REGULAR_FILE); + ERR_add_error_data(2, "Filename=", file); + return -1; + } #endif + /* Collect enough random data. */ + if (RAND_bytes(buf, (int)sizeof(buf)) != 1) + return -1; + #if defined(O_CREAT) && !defined(OPENSSL_NO_POSIX_IO) && \ !defined(OPENSSL_SYS_VMS) && !defined(OPENSSL_SYS_WINDOWS) { @@ -222,66 +182,54 @@ int RAND_write_file(const char *file) * application level. Also consider whether or not you NEED a persistent * rand file in a concurrent use situation. */ - out = openssl_fopen(file, "rb+"); #endif + if (out == NULL) out = openssl_fopen(file, "wb"); if (out == NULL) - goto err; + return -1; #if !defined(NO_CHMOD) && !defined(OPENSSL_NO_POSIX_IO) + /* + * Yes it's late to do this (see above comment), but better than nothing. + */ chmod(file, 0600); #endif - n = RAND_DATA; - for (;;) { - i = (n > BUFSIZE) ? BUFSIZE : n; - n -= BUFSIZE; - if (RAND_bytes(buf, i) <= 0) - rand_err = 1; - i = fwrite(buf, 1, i, out); - if (i <= 0) { - ret = 0; - break; - } - ret += i; - if (n <= 0) - break; - } + ret = fwrite(buf, 1, RAND_FILE_SIZE, out); fclose(out); - OPENSSL_cleanse(buf, BUFSIZE); - err: - return (rand_err ? -1 : ret); + OPENSSL_cleanse(buf, RAND_FILE_SIZE); + return ret; } const char *RAND_file_name(char *buf, size_t size) { char *s = NULL; + size_t len; int use_randfile = 1; #if defined(_WIN32) && defined(CP_UTF8) - DWORD len; - WCHAR *var, *val; - - if ((var = L"RANDFILE", - len = GetEnvironmentVariableW(var, NULL, 0)) == 0 - && (var = L"HOME", use_randfile = 0, - len = GetEnvironmentVariableW(var, NULL, 0)) == 0 - && (var = L"USERPROFILE", - len = GetEnvironmentVariableW(var, NULL, 0)) == 0) { - var = L"SYSTEMROOT", - len = GetEnvironmentVariableW(var, NULL, 0); + DWORD envlen; + WCHAR *var; + + /* Look up various environment variables. */ + if ((envlen = GetEnvironmentVariableW(var = L"RANDFILE", NULL, 0)) == 0) { + use_randfile = 0; + if ((envlen = GetEnvironmentVariableW(var = L"HOME", NULL, 0)) == 0 + && (envlen = GetEnvironmentVariableW(var = L"USERPROFILE", + NULL, 0)) == 0) + envlen = GetEnvironmentVariableW(var = L"SYSTEMROOT", NULL, 0); } - if (len != 0) { + /* If we got a value, allocate space to hold it and then get it. */ + if (envlen != 0) { int sz; + WCHAR *val = _alloca(envlen * sizeof(WCHAR)); - val = _alloca(len * sizeof(WCHAR)); - - if (GetEnvironmentVariableW(var, val, len) < len - && (sz = WideCharToMultiByte(CP_UTF8, 0, val, -1, NULL, 0, - NULL, NULL)) != 0) { + if (GetEnvironmentVariableW(var, val, envlen) < envlen + && (sz = WideCharToMultiByte(CP_UTF8, 0, val, -1, NULL, 0, + NULL, NULL)) != 0) { s = _alloca(sz); if (WideCharToMultiByte(CP_UTF8, 0, val, -1, s, sz, NULL, NULL) == 0) @@ -291,35 +239,33 @@ const char *RAND_file_name(char *buf, size_t size) #else if (OPENSSL_issetugid() != 0) { use_randfile = 0; - } else { - s = getenv("RANDFILE"); - if (s == NULL || *s == '\0') { - use_randfile = 0; - s = getenv("HOME"); - } + } else if ((s = getenv("RANDFILE")) == NULL || *s == '\0') { + use_randfile = 0; + s = getenv("HOME"); } #endif + #ifdef DEFAULT_HOME - if (!use_randfile && s == NULL) { + if (!use_randfile && s == NULL) s = DEFAULT_HOME; - } #endif - if (s != NULL && *s) { - size_t len = strlen(s); - - if (use_randfile && len + 1 < size) { - if (OPENSSL_strlcpy(buf, s, size) >= size) - return NULL; - } else if (len + strlen(RFILE) + 2 < size) { - OPENSSL_strlcpy(buf, s, size); + if (s == NULL || *s == '\0') + return NULL; + + len = strlen(s); + if (use_randfile) { + if (len + 1 >= size) + return NULL; + strcpy(buf, s); + } else { + if (len + 1 + strlen(RFILE) + 1 >= size) + return NULL; + strcpy(buf, s); #ifndef OPENSSL_SYS_VMS - OPENSSL_strlcat(buf, "/", size); + strcat(buf, "/"); #endif - OPENSSL_strlcat(buf, RFILE, size); - } - } else { - buf[0] = '\0'; /* no file name */ + strcat(buf, RFILE); } - return buf[0] ? buf : NULL; + return buf; } diff --git a/doc/man3/RAND_load_file.pod b/doc/man3/RAND_load_file.pod index eecaab94c0..8b5867ff89 100644 --- a/doc/man3/RAND_load_file.pod +++ b/doc/man3/RAND_load_file.pod @@ -8,51 +8,50 @@ RAND_load_file, RAND_write_file, RAND_file_name - PRNG seed file #include - const char *RAND_file_name(char *buf, size_t num); - int RAND_load_file(const char *filename, long max_bytes); int RAND_write_file(const char *filename); + const char *RAND_file_name(char *buf, size_t num); + =head1 DESCRIPTION +RAND_load_file() reads a number of bytes from file B and +adds them to the PRNG. If B is non-negative, +up to B are read; +if B is -1, the complete file is read. + +RAND_write_file() writes a number of random bytes (currently 256) to +file B which can be used to initialize the PRNG by calling +RAND_load_file() in a later session. + RAND_file_name() generates a default path for the random seed file. B points to a buffer of size B in which to store the filename. On all systems, if the environment variable B is set, its value will be used as the seed file name. - -Otherwise, the file is called ".rnd", found in platform dependent locations: +Otherwise, the file is called C<.rnd>, found in platform dependent locations: =over 4 =item On Windows (in order of preference) -%HOME%, %USERPROFILE%, %SYSTEMROOT%, C:\ + %HOME%, %USERPROFILE%, %SYSTEMROOT%, C:\ =item On VMS -SYS$LOGIN: + SYS$LOGIN: =item On all other systems -$HOME + $HOME =back If C<$HOME> (on non-Windows and non-VMS system) is not set either, or B is too small for the path name, an error occurs. -RAND_load_file() reads a number of bytes from file B and -adds them to the PRNG. If B is non-negative, -up to B are read; -if B is -1, the complete file is read. - -RAND_write_file() writes a number of random bytes (currently 1024) to -file B which can be used to initialize the PRNG by calling -RAND_load_file() in a later session. - =head1 RETURN VALUES RAND_load_file() returns the number of bytes read. @@ -67,6 +66,13 @@ error. L, L, L +=head1 HISTORY + +A comment in the source since at least OpenSSL version 1.0.2 said that +RAND_load_file() and RAND_write_file() were only intended for regular files, +and not really device special files such as C. This was +poorly enforced before OpenSSL version 1.1.1. + =head1 COPYRIGHT Copyright 2000-2016 The OpenSSL Project Authors. All Rights Reserved. diff --git a/include/openssl/randerr.h b/include/openssl/randerr.h index 5c9ab86507..244fd0e4b4 100644 --- a/include/openssl/randerr.h +++ b/include/openssl/randerr.h @@ -23,11 +23,16 @@ int ERR_load_RAND_strings(void); * RAND function codes. */ # define RAND_F_RAND_BYTES 100 +# define RAND_F_RAND_LOAD_FILE 101 +# define RAND_F_RAND_WRITE_FILE 102 /* * RAND reason codes. */ +# define RAND_R_CANNOT_OPEN_FILE 102 # define RAND_R_FUNC_NOT_IMPLEMENTED 101 +# define RAND_R_FWRITE_ERROR 103 +# define RAND_R_NOT_A_REGULAR_FILE 104 # define RAND_R_PRNG_NOT_SEEDED 100 #endif -- 2.25.1