From 48d2795f312224f03dc88e99434865540ca71c7e Mon Sep 17 00:00:00 2001 From: David Steele Date: Tue, 6 Nov 2018 20:04:16 -0500 Subject: [PATCH] Merge crypto/random module into crypto/crypto. There wasn't enough code to justify a separate module/test and it seems to fit just fine in crypto/crypto. --- doc/xml/release.xml | 4 +++ lib/pgBackRest/Common/Cipher.pm | 2 +- lib/pgBackRest/LibCAuto.pm | 2 +- libc/LibC.xs | 2 +- libc/Makefile.PL | 1 - libc/build/lib/pgBackRestLibC/Build.pm | 2 +- libc/xs/crypto/random.xs | 4 +-- src/Makefile | 8 ++---- src/crypto/cipherBlock.c | 3 +-- src/crypto/crypto.c | 22 +++++++++++++++- src/crypto/crypto.h | 1 + src/crypto/random.c | 28 -------------------- src/crypto/random.h | 12 --------- src/perl/embed.auto.c | 4 +-- src/perl/libc.auto.c | 10 +++---- test/code-count/file-type.yaml | 8 ------ test/define.yaml | 9 +------ test/src/module/crypto/cryptoTest.c | 22 ++++++++++++++++ test/src/module/crypto/randomTest.c | 36 -------------------------- 19 files changed, 65 insertions(+), 115 deletions(-) delete mode 100644 src/crypto/random.c delete mode 100644 src/crypto/random.h delete mode 100644 test/src/module/crypto/randomTest.c diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 7124cc539..da073a84e 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -90,6 +90,10 @@

Change infoArchiveCheckPg() to display the version as a string (e.g. 9.4) instead of the integer representation (e.g. 90400) when throwing an error.

+ +

Merge crypto/random module into crypto/crypto.

+ +

Add cryptoError() and update crypto code to use it.

 diff --git a/lib/pgBackRest/Common/Cipher.pm b/lib/pgBackRest/Common/Cipher.pm index fabe24e56..d13fdc0a6 100644 --- a/lib/pgBackRest/Common/Cipher.pm +++ b/lib/pgBackRest/Common/Cipher.pm @@ -33,7 +33,7 @@ sub cipherPassGen ); # Create and base64 encode the key - my $strCipherPass = encodeToStr(ENCODE_TYPE_BASE64, randomBytes($iKeySizeInBytes)); + my $strCipherPass = encodeToStr(ENCODE_TYPE_BASE64, cryptoRandomBytes($iKeySizeInBytes)); # Return from function and log return values if any return logDebugReturn diff --git a/lib/pgBackRest/LibCAuto.pm b/lib/pgBackRest/LibCAuto.pm index ecfc278d8..951039c02 100644 --- a/lib/pgBackRest/LibCAuto.pm +++ b/lib/pgBackRest/LibCAuto.pm @@ -334,7 +334,7 @@ sub libcAutoExportTag random => [ - 'randomBytes', + 'cryptoRandomBytes', ], storage => diff --git a/libc/LibC.xs b/libc/LibC.xs index 4ee415edd..ef707924e 100644 --- a/libc/LibC.xs +++ b/libc/LibC.xs @@ -52,7 +52,7 @@ These includes are from the src directory. There is no Perl-specific code in th #include "config/define.h" #include "config/load.h" #include "config/parse.h" -#include "crypto/random.h" +#include "crypto/crypto.h" #include "perl/config.h" #include "postgres/pageChecksum.h" #include "storage/driver/posix/storage.h" diff --git a/libc/Makefile.PL b/libc/Makefile.PL index ce9a04ba6..92eb50b1c 100644 --- a/libc/Makefile.PL +++ b/libc/Makefile.PL @@ -83,7 +83,6 @@ my @stryCFile = 'crypto/cipherBlock.c', 'crypto/crypto.c', 'crypto/hash.c', - 'crypto/random.c', 'perl/config.c', 'postgres/pageChecksum.c', 'storage/driver/posix/storage.c', diff --git a/libc/build/lib/pgBackRestLibC/Build.pm b/libc/build/lib/pgBackRestLibC/Build.pm index b3490e857..bc1aeab4d 100644 --- a/libc/build/lib/pgBackRestLibC/Build.pm +++ b/libc/build/lib/pgBackRestLibC/Build.pm @@ -142,7 +142,7 @@ my $rhExport = 'random' => { &BLD_EXPORTTYPE_SUB => [qw( - randomBytes + cryptoRandomBytes )], }, diff --git a/libc/xs/crypto/random.xs b/libc/xs/crypto/random.xs index fe8957dde..8b0a46156 100644 --- a/libc/xs/crypto/random.xs +++ b/libc/xs/crypto/random.xs @@ -6,13 +6,13 @@ MODULE = pgBackRest::LibC PACKAGE = pgBackRest::LibC #################################################################################################################################### SV * -randomBytes(size) +cryptoRandomBytes(size) I32 size CODE: RETVAL = newSV(size); SvPOK_only(RETVAL); - randomBytes((unsigned char *)SvPV_nolen(RETVAL), size); + cryptoRandomBytes((unsigned char *)SvPV_nolen(RETVAL), size); SvCUR_set(RETVAL, size); OUTPUT: diff --git a/src/Makefile b/src/Makefile index 14b3ffe8d..59508ce6b 100644 --- a/src/Makefile +++ b/src/Makefile @@ -104,7 +104,6 @@ SRCS = \ crypto/cipherBlock.c \ crypto/hash.c \ crypto/crypto.c \ - crypto/random.c \ info/info.c \ info/infoArchive.c \ info/infoPg.c \ @@ -293,7 +292,7 @@ config/load.o: config/load.c command/command.h common/debug.h common/error.auto. config/parse.o: config/parse.c common/assert.h common/debug.h common/error.auto.h common/error.h common/ini.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/write.h common/lock.h common/log.h common/logLevel.h common/memContext.h common/regExp.h common/stackTrace.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h config/config.auto.h config/config.h config/define.auto.h config/define.h config/parse.auto.c config/parse.h storage/fileRead.h storage/fileWrite.h storage/helper.h storage/info.h storage/storage.h version.h $(CC) $(CFLAGS) -c config/parse.c -o config/parse.o -crypto/cipherBlock.o: crypto/cipherBlock.c common/debug.h common/error.auto.h common/error.h common/log.h common/logLevel.h common/memContext.h common/stackTrace.h common/type/convert.h crypto/cipher.h crypto/cipherBlock.h crypto/crypto.h crypto/random.h +crypto/cipherBlock.o: crypto/cipherBlock.c common/debug.h common/error.auto.h common/error.h common/log.h common/logLevel.h common/memContext.h common/stackTrace.h common/type/convert.h crypto/cipher.h crypto/cipherBlock.h crypto/crypto.h $(CC) $(CFLAGS) -c crypto/cipherBlock.c -o crypto/cipherBlock.o crypto/crypto.o: crypto/crypto.c common/debug.h common/error.auto.h common/error.h common/log.h common/logLevel.h common/stackTrace.h common/type/convert.h crypto/crypto.h @@ -302,9 +301,6 @@ crypto/crypto.o: crypto/crypto.c common/debug.h common/error.auto.h common/error crypto/hash.o: crypto/hash.c common/assert.h common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/filter.intern.h common/log.h common/logLevel.h common/memContext.h common/stackTrace.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/variant.h common/type/variantList.h crypto/crypto.h crypto/hash.h $(CC) $(CFLAGS) -c crypto/hash.c -o crypto/hash.o -crypto/random.o: crypto/random.c common/debug.h common/error.auto.h common/error.h common/log.h common/logLevel.h common/stackTrace.h common/type/convert.h crypto/random.h - $(CC) $(CFLAGS) -c crypto/random.c -o crypto/random.o - info/info.o: info/info.c common/debug.h common/error.auto.h common/error.h common/ini.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/write.h common/log.h common/logLevel.h common/memContext.h common/stackTrace.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h crypto/hash.h info/info.h storage/fileRead.h storage/fileWrite.h storage/helper.h storage/info.h storage/storage.h version.h $(CC) $(CFLAGS) -c info/info.c -o info/info.o @@ -320,7 +316,7 @@ main.o: main.c command/archive/get/get.h command/archive/push/push.h command/com perl/config.o: perl/config.c common/debug.h common/error.auto.h common/error.h common/lock.h common/log.h common/logLevel.h common/memContext.h common/stackTrace.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h config/config.auto.h config/config.h config/define.auto.h config/define.h $(CC) $(CFLAGS) -c perl/config.c -o perl/config.o -perl/exec.o: perl/exec.c ../libc/LibC.h common/debug.h common/encode.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/write.h common/lock.h common/log.h common/logLevel.h common/memContext.h common/stackTrace.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h config/config.auto.h config/config.h config/define.auto.h config/define.h config/load.h config/parse.h crypto/cipher.h crypto/cipherBlock.h crypto/hash.h crypto/random.h perl/config.h perl/embed.auto.c perl/exec.h perl/libc.auto.c postgres/pageChecksum.h storage/driver/posix/fileRead.h storage/driver/posix/fileWrite.h storage/driver/posix/storage.h storage/fileRead.h storage/fileWrite.h storage/info.h storage/storage.h storage/storage.intern.h version.h ../libc/xs/common/encode.xsh ../libc/xs/crypto/cipherBlock.xsh ../libc/xs/crypto/hash.xsh +perl/exec.o: perl/exec.c ../libc/LibC.h common/debug.h common/encode.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/write.h common/lock.h common/log.h common/logLevel.h common/memContext.h common/stackTrace.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h config/config.auto.h config/config.h config/define.auto.h config/define.h config/load.h config/parse.h crypto/cipher.h crypto/cipherBlock.h crypto/crypto.h crypto/hash.h perl/config.h perl/embed.auto.c perl/exec.h perl/libc.auto.c postgres/pageChecksum.h storage/driver/posix/fileRead.h storage/driver/posix/fileWrite.h storage/driver/posix/storage.h storage/fileRead.h storage/fileWrite.h storage/info.h storage/storage.h storage/storage.intern.h version.h ../libc/xs/common/encode.xsh ../libc/xs/crypto/cipherBlock.xsh ../libc/xs/crypto/hash.xsh $(CC) $(CFLAGS) -c perl/exec.c -o perl/exec.o postgres/interface.o: postgres/interface.c common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/write.h common/log.h common/logLevel.h common/memContext.h common/regExp.h common/stackTrace.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h postgres/interface.h postgres/interface/v083.h postgres/interface/v084.h postgres/interface/v090.h postgres/interface/v091.h postgres/interface/v092.h postgres/interface/v093.h postgres/interface/v094.h postgres/interface/v095.h postgres/interface/v096.h postgres/interface/v100.h postgres/interface/v110.h postgres/version.h storage/fileRead.h storage/fileWrite.h storage/helper.h storage/info.h storage/storage.h diff --git a/src/crypto/cipherBlock.c b/src/crypto/cipherBlock.c index 35c6f08da..ac10937ec 100644 --- a/src/crypto/cipherBlock.c +++ b/src/crypto/cipherBlock.c @@ -11,7 +11,6 @@ Block Cipher #include "common/memContext.h" #include "crypto/cipherBlock.h" #include "crypto/crypto.h" -#include "crypto/random.h" /*********************************************************************************************************************************** Header constants and sizes @@ -165,7 +164,7 @@ cipherBlockProcess(CipherBlock *this, const unsigned char *source, size_t source destinationSize += CIPHER_BLOCK_MAGIC_SIZE; // Add salt to the destination buffer - randomBytes(destination, PKCS5_SALT_LEN); + cryptoRandomBytes(destination, PKCS5_SALT_LEN); salt = destination; destination += PKCS5_SALT_LEN; destinationSize += PKCS5_SALT_LEN; diff --git a/src/crypto/crypto.c b/src/crypto/crypto.c index c685cc85f..3fae80429 100644 --- a/src/crypto/crypto.c +++ b/src/crypto/crypto.c @@ -2,8 +2,9 @@ Crypto Common ***********************************************************************************************************************************/ #include -#include #include +#include +#include #include "common/debug.h" #include "common/error.h" @@ -64,3 +65,22 @@ cryptoIsInit(void) FUNCTION_TEST_VOID(); FUNCTION_TEST_RESULT(BOOL, cryptoInitDone); } + +/*********************************************************************************************************************************** +Generate random bytes +***********************************************************************************************************************************/ +void +cryptoRandomBytes(unsigned char *buffer, size_t size) +{ + FUNCTION_DEBUG_BEGIN(logLevelTrace); + FUNCTION_DEBUG_PARAM(UCHARP, buffer); + FUNCTION_DEBUG_PARAM(SIZE, size); + + FUNCTION_DEBUG_ASSERT(buffer != NULL); + FUNCTION_DEBUG_ASSERT(size > 0); + FUNCTION_DEBUG_END(); + + RAND_bytes(buffer, (int)size); + + FUNCTION_DEBUG_RESULT_VOID(); +} diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h index a60e331da..1c2d1a7b7 100644 --- a/src/crypto/crypto.h +++ b/src/crypto/crypto.h @@ -12,5 +12,6 @@ Functions void cryptoError(bool error, const char *description); void cryptoInit(void); bool cryptoIsInit(void); +void cryptoRandomBytes(unsigned char *buffer, size_t size); #endif diff --git a/src/crypto/random.c b/src/crypto/random.c deleted file mode 100644 index 86eb02b2c..000000000 --- a/src/crypto/random.c +++ /dev/null @@ -1,28 +0,0 @@ -/*********************************************************************************************************************************** -Cipher -***********************************************************************************************************************************/ -#include - -#include "common/debug.h" -#include "common/error.h" -#include "common/log.h" -#include "crypto/random.h" - -/*********************************************************************************************************************************** -Generate random bytes -***********************************************************************************************************************************/ -void -randomBytes(unsigned char *buffer, size_t size) -{ - FUNCTION_DEBUG_BEGIN(logLevelTrace); - FUNCTION_DEBUG_PARAM(UCHARP, buffer); - FUNCTION_DEBUG_PARAM(SIZE, size); - - FUNCTION_DEBUG_ASSERT(buffer != NULL); - FUNCTION_DEBUG_ASSERT(size > 0); - FUNCTION_DEBUG_END(); - - RAND_bytes(buffer, (int)size); - - FUNCTION_DEBUG_RESULT_VOID(); -} diff --git a/src/crypto/random.h b/src/crypto/random.h deleted file mode 100644 index 4511da700..000000000 --- a/src/crypto/random.h +++ /dev/null @@ -1,12 +0,0 @@ -/*********************************************************************************************************************************** -Random Header -***********************************************************************************************************************************/ -#ifndef CRYPTO_RANDOM_H -#define CRYPTO_RANDOM_H - -/*********************************************************************************************************************************** -Functions -***********************************************************************************************************************************/ -void randomBytes(unsigned char *buffer, size_t size); - -#endif diff --git a/src/perl/embed.auto.c b/src/perl/embed.auto.c index bc890a5e4..2ce93884b 100644 --- a/src/perl/embed.auto.c +++ b/src/perl/embed.auto.c @@ -4896,7 +4896,7 @@ static const EmbeddedModule embeddedModule[] = "{name => 'iKeySizeInBytes', default => 48},\n" ");\n" "\n\n" - "my $strCipherPass = encodeToStr(ENCODE_TYPE_BASE64, randomBytes($iKeySizeInBytes));\n" + "my $strCipherPass = encodeToStr(ENCODE_TYPE_BASE64, cryptoRandomBytes($iKeySizeInBytes));\n" "\n\n" "return logDebugReturn\n" "(\n" @@ -11404,7 +11404,7 @@ static const EmbeddedModule embeddedModule[] = "\n" "random =>\n" "[\n" - "'randomBytes',\n" + "'cryptoRandomBytes',\n" "],\n" "\n" "storage =>\n" diff --git a/src/perl/libc.auto.c b/src/perl/libc.auto.c index 13c1fd797..81172d5a8 100644 --- a/src/perl/libc.auto.c +++ b/src/perl/libc.auto.c @@ -60,7 +60,7 @@ These includes are from the src directory. There is no Perl-specific code in th #include "config/define.h" #include "config/load.h" #include "config/parse.h" -#include "crypto/random.h" +#include "crypto/crypto.h" #include "perl/config.h" #include "postgres/pageChecksum.h" #include "storage/driver/posix/storage.h" @@ -396,8 +396,8 @@ XS_EUPXS(XS_pgBackRest__LibC_pageChecksumBufferTest) /* INCLUDE: Returning to 'xs/crypto/random.xs' from 'xs/postgres/pageChecksum.xs' */ -XS_EUPXS(XS_pgBackRest__LibC_randomBytes); /* prototype to pass -Wmissing-prototypes */ -XS_EUPXS(XS_pgBackRest__LibC_randomBytes) +XS_EUPXS(XS_pgBackRest__LibC_cryptoRandomBytes); /* prototype to pass -Wmissing-prototypes */ +XS_EUPXS(XS_pgBackRest__LibC_cryptoRandomBytes) { dVAR; dXSARGS; if (items != 1) @@ -409,7 +409,7 @@ XS_EUPXS(XS_pgBackRest__LibC_randomBytes) RETVAL = newSV(size); SvPOK_only(RETVAL); - randomBytes((unsigned char *)SvPV_nolen(RETVAL), size); + cryptoRandomBytes((unsigned char *)SvPV_nolen(RETVAL), size); SvCUR_set(RETVAL, size); RETVAL = sv_2mortal(RETVAL); @@ -1217,7 +1217,7 @@ XS_EXTERNAL(boot_pgBackRest__LibC) newXS_deffile("pgBackRest::LibC::pageChecksum", XS_pgBackRest__LibC_pageChecksum); newXS_deffile("pgBackRest::LibC::pageChecksumTest", XS_pgBackRest__LibC_pageChecksumTest); newXS_deffile("pgBackRest::LibC::pageChecksumBufferTest", XS_pgBackRest__LibC_pageChecksumBufferTest); - newXS_deffile("pgBackRest::LibC::randomBytes", XS_pgBackRest__LibC_randomBytes); + newXS_deffile("pgBackRest::LibC::cryptoRandomBytes", XS_pgBackRest__LibC_cryptoRandomBytes); newXS_deffile("pgBackRest::LibC::Crypto::Hash::new", XS_pgBackRest__LibC__Crypto__Hash_new); newXS_deffile("pgBackRest::LibC::Crypto::Hash::process", XS_pgBackRest__LibC__Crypto__Hash_process); newXS_deffile("pgBackRest::LibC::Crypto::Hash::result", XS_pgBackRest__LibC__Crypto__Hash_result); diff --git a/test/code-count/file-type.yaml b/test/code-count/file-type.yaml index c2602a289..026698dbd 100644 --- a/test/code-count/file-type.yaml +++ b/test/code-count/file-type.yaml @@ -967,14 +967,6 @@ src/crypto/hash.h: class: core type: c/h -src/crypto/random.c: - class: core - type: c - -src/crypto/random.h: - class: core - type: c/h - src/info/info.c: class: core type: c diff --git a/test/define.yaml b/test/define.yaml index ec828900f..8f32f0a33 100644 --- a/test/define.yaml +++ b/test/define.yaml @@ -292,18 +292,11 @@ unit: test: # ---------------------------------------------------------------------------------------------------------------------------- - name: crypto - total: 2 + total: 3 coverage: crypto/crypto: full - # ---------------------------------------------------------------------------------------------------------------------------- - - name: random - total: 1 - - coverage: - crypto/random: full - # ---------------------------------------------------------------------------------------------------------------------------- - name: hash total: 3 diff --git a/test/src/module/crypto/cryptoTest.c b/test/src/module/crypto/cryptoTest.c index ae23ec31c..4ae7a3133 100644 --- a/test/src/module/crypto/cryptoTest.c +++ b/test/src/module/crypto/cryptoTest.c @@ -35,5 +35,27 @@ testRun(void) TEST_ERROR(cryptoError(true, "no error"), CryptoError, "no error: no details available"); } + // ***************************************************************************************************************************** + if (testBegin("cryptoRandomBytes()")) + { + // ------------------------------------------------------------------------------------------------------------------------- + // Test if the buffer was overrun + size_t bufferSize = 256; + unsigned char *buffer = memNew(bufferSize + 1); + + cryptoRandomBytes(buffer, bufferSize); + TEST_RESULT_BOOL(buffer[bufferSize] == 0, true, "check that buffer did not overrun (though random byte could be 0)"); + + // ------------------------------------------------------------------------------------------------------------------------- + // Count bytes that are not zero (there shouldn't be all zeroes) + int nonZeroTotal = 0; + + for (unsigned int charIdx = 0; charIdx < bufferSize; charIdx++) + if (buffer[charIdx] != 0) // {uncovered - ok if there are no zeros} + nonZeroTotal++; + + TEST_RESULT_INT_NE(nonZeroTotal, 0, "check that there are non-zero values in the buffer"); + } + FUNCTION_HARNESS_RESULT_VOID(); } diff --git a/test/src/module/crypto/randomTest.c b/test/src/module/crypto/randomTest.c deleted file mode 100644 index d3372448d..000000000 --- a/test/src/module/crypto/randomTest.c +++ /dev/null @@ -1,36 +0,0 @@ -/*********************************************************************************************************************************** -Test Random -***********************************************************************************************************************************/ - -/*********************************************************************************************************************************** -Test Run -***********************************************************************************************************************************/ -void -testRun(void) -{ - FUNCTION_HARNESS_VOID(); - - // ***************************************************************************************************************************** - if (testBegin("randomBytes()")) - { - // ------------------------------------------------------------------------------------------------------------------------- - // Test if the buffer was overrun - size_t bufferSize = 256; - unsigned char *buffer = memNew(bufferSize + 1); - - randomBytes(buffer, bufferSize); - TEST_RESULT_BOOL(buffer[bufferSize] == 0, true, "check that buffer did not overrun (though random byte could be 0)"); - - // ------------------------------------------------------------------------------------------------------------------------- - // Count bytes that are not zero (there shouldn't be all zeroes) - int nonZeroTotal = 0; - - for (unsigned int charIdx = 0; charIdx < bufferSize; charIdx++) - if (buffer[charIdx] != 0) // {uncovered - ok if there are no zeros} - nonZeroTotal++; - - TEST_RESULT_INT_NE(nonZeroTotal, 0, "check that there are non-zero values in the buffer"); - } - - FUNCTION_HARNESS_RESULT_VOID(); -}