OpenSystemsDevelopment / qpdf

Commit 4229457068d6a28cc11b506f127a7bb650ab18c1

Authored by Jay Berkenbilt
1 parent 25687ddd

Security: use a secure random number generator 

If not available, give an error.  The user may also configure qpdf to
use an insecure random number generator.
Inline Side-by-side
Showing 8 changed files with 187 additions and 20 deletions
ChangeLog
  View file @4229457
1 1 2013-10-05 Jay Berkenbilt <ejb@ql.org>
2 2  
  3 + * Use cryptographically secure random number generation when
  4 + available. See additional notes in README.
  5 +
3 6 * Replace some assert() calls with std::logic_error exceptions.
4 7 Ideally there shouldn't be assert() calls outside of testing.
5 8 This change may make a few more potential code errors in handling
... ...
... ... @@ -167,3 +167,22 @@ the test suite fails, test failure detail will be included in the
167 167 build output. Otherwise, you will have to have access to the
168 168 qtest.log file from the build to view test failures. The debian
169 169 packages for qpdf enable this option, for example.
  170 +
  171 +
  172 +Random Number Generation
  173 +========================
  174 +
  175 +When the qpdf detects either the Windows cryptography API or the
  176 +existence of /dev/urandom, /dev/arandom, or /dev/random, it uses them
  177 +to generate cryptography secure random numbers. If none of these
  178 +conditions are true, the build will fail with an error. It is
  179 +possible to configure qpdf with the --enable-insecure-random option,
  180 +in which case it will generate random numbers with stdlib's random()
  181 +or rand() calls instead. These random numbers are not cryptography
  182 +secure, but the qpdf library is fully functional using them. Using
  183 +non-secure random numbers means that it's easier in some cases to
  184 +guess encryption keys. If you're not generating encrypted files,
  185 +there's no advantage to using secure random numbers.
  186 +
  187 +If you are building qpdf on a platform that qpdf doesn't know how to
  188 +generate secure random numbers on, a patch would be welcome.
... ...
... ... @@ -76,12 +76,11 @@ General
76 76 and replace the /Pages key of the root dictionary with the new
77 77 tree.
78 78  
79   - * Improve the random number seed to make it more secure so that we
80   - have stronger random numbers, particularly when multiple files are
81   - generated in the same second. This code may need to be
82   - OS-specific. Probably we should add a method in QUtil to seed with
83   - a strong random number and call this automatically the first time
84   - QUtil::random() is called.
  79 + * Secure random number generation could be made more efficient by
  80 + using a local static to ensure a single random device or crypt
  81 + provider as long as this can be done in a thread-safe fashion. In
  82 + the initial implementation, this is being skipped to avoid having
  83 + to add any dependencies on threading libraries.
85 84  
86 85 * Study what's required to support savable forms that can be saved by
87 86 Adobe Reader. Does this require actually signing the document with
... ...
configure.ac
  View file @4229457
... ... @@ -16,6 +16,23 @@ AC_PROG_CXX
16 16 AC_HEADER_STDC
17 17 LT_INIT([win32-dll])
18 18  
  19 +AC_ARG_ENABLE(insecure-random,
  20 + AS_HELP_STRING([--enable-insecure-random],
  21 + [whether to use stdlib's random number generator (default is no)]),
  22 + [if test "$enableval" = "yes"; then
  23 + qpdf_INSECURE_RANDOM=1;
  24 + else
  25 + qpdf_INSECURE_RANDOM=0;
  26 + fi], [qpdf_INSECURE_RANDOM=0])
  27 +if test "$qpdf_INSECURE_RANDOM" = "1"; then
  28 + AC_MSG_RESULT(yes)
  29 + AC_DEFINE([USE_INSECURE_RANDOM], [1], [Whether to use inscure random numbers])
  30 +else
  31 + AC_MSG_RESULT(no)
  32 +fi
  33 +
  34 +AX_RANDOM_DEVICE
  35 +
19 36 USE_EXTERNAL_LIBS=0
20 37 AC_MSG_CHECKING(for whether to use external libraries distribution)
21 38 AC_ARG_ENABLE(external-libs,
... ... @@ -54,6 +71,23 @@ if test &quot;$BUILD_INTERNAL_LIBS&quot; = &quot;0&quot;; then
54 71 AC_SEARCH_LIBS(pcre_compile,pcre,,[MISSING_PCRE=1; MISSING_ANY=1])
55 72 fi
56 73  
  74 +if test "x$qpdf_INSECURE_RANDOM" != "x1"; then
  75 + OLIBS=$LIBS
  76 + LIBS="$LIBS Advapi32.lib"
  77 + AC_MSG_CHECKING(for Advapi32 library)
  78 + AC_LINK_IFELSE([AC_LANG_PROGRAM(
  79 + [[#pragma comment(lib, "crypt32.lib")
  80 + #include <windows.h>
  81 + #include <Wincrypt.h>
  82 + HCRYPTPROV cp;]],
  83 + [CryptAcquireContext(&cp, NULL, NULL, PROV_RSA_FULL, 0);]
  84 + )],
  85 + [AC_MSG_RESULT(yes)
  86 + LIBS="$OLIBS -lAdvapi32"],
  87 + [AC_MSG_RESULT(no)
  88 + LIBS=$OLIBS])
  89 +fi
  90 +
57 91 QPDF_LARGE_FILE_TEST_PATH=
58 92 AC_SUBST(QPDF_LARGE_FILE_TEST_PATH)
59 93 AC_ARG_WITH(large-file-test-path,
... ...
copy_dlls
  View file @4229457
... ... @@ -20,7 +20,7 @@ while (&lt;O&gt;)
20 20 {
21 21 my $dll = $1;
22 22 $dll =~ tr/A-Z/a-z/;
23   - next if $dll =~ m/^(kernel32|user32|msvcrt)\.dll$/;
  23 + next if $dll =~ m/^(kernel32|user32|msvcrt|advapi32)\.dll$/;
24 24 $dlls{$dll} = 1;
25 25 }
26 26 elsif (m/^Magic.*\((PE.+?)\)/)
... ...
include/qpdf/QUtil.hh
  View file @4229457
... ... @@ -108,12 +108,18 @@ namespace QUtil
108 108 QPDF_DLL
109 109 std::string toUTF8(unsigned long uval);
110 110  
111   - // Wrapper around random from stdlib. Calls srandom automatically
112   - // the first time it is called.
  111 + // If secure random number generation is supported on your
  112 + // platform and qpdf was not compiled with insecure random number
  113 + // generation, this returns a crytographically secure random
  114 + // number. Otherwise it falls back to random from stdlib and
  115 + // calls srandom automatically the first time it is called.
113 116 QPDF_DLL
114 117 long random();
115 118  
116   - // Wrapper around srandom from stdlib.
  119 + // Wrapper around srandom from stdlib. Seeds the standard library
  120 + // weak random number generator, which is not used if secure
  121 + // random number generation is being used. You never need to call
  122 + // this method as it is called automatically if needed.
117 123 QPDF_DLL
118 124 void srandom(unsigned int seed);
119 125  
... ...
libqpdf/QUtil.cc
  View file @4229457
... ... @@ -17,6 +17,7 @@
17 17 #include <Windows.h>
18 18 #include <direct.h>
19 19 #include <io.h>
  20 +#include <Wincrypt.h>
20 21 #else
21 22 #include <unistd.h>
22 23 #endif
... ... @@ -377,6 +378,8 @@ QUtil::toUTF8(unsigned long uval)
377 378 return result;
378 379 }
379 380  
  381 +#ifdef USE_INSECURE_RANDOM
  382 +
380 383 long
381 384 QUtil::random()
382 385 {
... ... @@ -390,28 +393,100 @@ QUtil::random()
390 393 seeded_random = true;
391 394 }
392 395  
393   -#ifdef HAVE_RANDOM
  396 +# ifdef HAVE_RANDOM
394 397 return ::random();
395   -#else
  398 +# else
396 399 return rand();
397   -#endif
  400 +# endif
398 401 }
399 402  
400 403 void
401   -QUtil::srandom(unsigned int seed)
  404 +QUtil::initializeWithRandomBytes(unsigned char* data, size_t len)
402 405 {
403   -#ifdef HAVE_RANDOM
404   - ::srandom(seed);
  406 + for (size_t i = 0; i < len; ++i)
  407 + {
  408 + data[i] = static_cast<unsigned char>((QUtil::random() & 0xff0) >> 4);
  409 + }
  410 +}
  411 +
405 412 #else
406   - srand(seed);
407   -#endif
  413 +
  414 +long
  415 +QUtil::random()
  416 +{
  417 + long result = 0L;
  418 + initializeWithRandomBytes(
  419 + reinterpret_cast<unsigned char*>(&result),
  420 + sizeof(result));
  421 + return result;
408 422 }
409 423  
  424 +#ifdef _WIN32
  425 +class WindowsCryptProvider
  426 +{
  427 + public:
  428 + WindowsCryptProvider()
  429 + {
  430 + if (! CryptAcquireContext(&crypt_prov, NULL, NULL, PROV_RSA_FULL, 0))
  431 + {
  432 + throw std::runtime_error("unable to acquire crypt context");
  433 + }
  434 + }
  435 + ~WindowsCryptProvider()
  436 + {
  437 + // Ignore error
  438 + CryptReleaseContext(crypt_prov, 0);
  439 + }
  440 +
  441 + HCRYPTPROV crypt_prov;
  442 +};
  443 +#endif
  444 +
410 445 void
411 446 QUtil::initializeWithRandomBytes(unsigned char* data, size_t len)
412 447 {
413   - for (size_t i = 0; i < len; ++i)
  448 +#if defined(_WIN32)
  449 +
  450 + // Optimization: make the WindowsCryptProvider static as long as
  451 + // it can be done in a thread-safe fashion.
  452 + WindowsCryptProvider c;
  453 + if (! CryptGenRandom(c.crypt_prov, len, reinterpret_cast<BYTE*>(data)))
414 454 {
415   - data[i] = static_cast<unsigned char>((QUtil::random() & 0xff0) >> 4);
  455 + throw std::runtime_error("unable to generate secure random data");
  456 + }
  457 +
  458 +#elif defined(RANDOM_DEVICE)
  459 +
  460 + // Optimization: wrap the file open and close in a class so that
  461 + // the file is closed in a destructor, then make this static to
  462 + // keep the file handle open. Only do this if it can be done in a
  463 + // thread-safe fashion.
  464 + FILE* f = QUtil::safe_fopen(RANDOM_DEVICE, "rb");
  465 + size_t fr = fread(data, 1, len, f);
  466 + fclose(f);
  467 + if (fr != len)
  468 + {
  469 + throw std::runtime_error(
  470 + "unable to read " +
  471 + QUtil::int_to_string(len) +
  472 + " bytes from " + std::string(RANDOM_DEVICE));
416 473 }
  474 +
  475 +#else
  476 +
  477 +# error "Don't know how to generate secure random numbers on this platform. See random number generation in the top-level README"
  478 +
  479 +#endif
  480 +}
  481 +
  482 +#endif
  483 +
  484 +void
  485 +QUtil::srandom(unsigned int seed)
  486 +{
  487 +#ifdef HAVE_RANDOM
  488 + ::srandom(seed);
  489 +#else
  490 + srand(seed);
  491 +#endif
417 492 }
... ...
m4/ax_random_device.m4 0 → 100644
  View file @4229457
  1 +dnl @synopsis AX_RANDOM_DEVICE
  2 +dnl
  3 +dnl This macro will check for a random device, allowing the user to explicitly
  4 +dnl set the path. The user uses '--with-random=FILE' as an argument to
  5 +dnl configure.
  6 +dnl
  7 +dnl If A random device is found then HAVE_RANDOM_DEVICE is set to 1 and
  8 +dnl RANDOM_DEVICE contains the path.
  9 +dnl
  10 +dnl @category Miscellaneous
  11 +dnl @author Martin Ebourne
  12 +dnl @version 2005/07/01
  13 +dnl @license AllPermissive
  14 +
  15 +AC_DEFUN([AX_RANDOM_DEVICE], [
  16 + AC_ARG_WITH([random],
  17 + [AC_HELP_STRING([--with-random=FILE], [Use FILE as random number seed [auto-detected]])],
  18 + [RANDOM_DEVICE="$withval"],
  19 + [AC_CHECK_FILE("/dev/urandom", [RANDOM_DEVICE="/dev/urandom";],
  20 + [AC_CHECK_FILE("/dev/arandom", [RANDOM_DEVICE="/dev/arandom";],
  21 + [AC_CHECK_FILE("/dev/random", [RANDOM_DEVICE="/dev/random";])]
  22 + )])
  23 + ])
  24 + if test "x$RANDOM_DEVICE" != "x" ; then
  25 + AC_DEFINE([HAVE_RANDOM_DEVICE], 1,
  26 + [Define to 1 (and set RANDOM_DEVICE) if a random device is available])
  27 + AC_SUBST([RANDOM_DEVICE])
  28 + AC_DEFINE_UNQUOTED([RANDOM_DEVICE], ["$RANDOM_DEVICE"],
  29 + [Define to the filename of the random device (and set HAVE_RANDOM_DEVICE)])
  30 + fi
  31 +])dnl
... ...