From 3ea4de25c712971a35dbad27d8834d75933daa08 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Sat, 31 Jan 2015 03:21:07 +0100 Subject: [PATCH 01/16] fix: Make CppCheck happy by fixing the code Used CppCheck command: cppcheck --enable=all -v --inconclusive --- src/apps/client.cpp | 6 +++--- src/apps/signer.cpp | 2 +- src/crypto/X509.cpp | 5 ++--- src/crypto/X509.h | 2 +- src/crypto/remoteSigner.cpp | 4 +--- src/crypto/sslUtil.cpp | 10 ++++------ src/crypto/sslUtil.h | 8 ++++---- src/io/bios.cpp | 12 ++++++------ src/io/opensslBIO.cpp | 3 +-- src/io/record.cpp | 2 +- src/io/record.h | 6 +++--- src/io/recordHandler.cpp | 10 ++-------- src/io/slipBio.cpp | 17 ++--------------- 13 files changed, 31 insertions(+), 56 deletions(-) diff --git a/src/apps/client.cpp b/src/apps/client.cpp index e185519..7c5bf08 100644 --- a/src/apps/client.cpp +++ b/src/apps/client.cpp @@ -78,7 +78,7 @@ int main( int argc, const char* argv[] ) { std::shared_ptr jp( new MySQLJobProvider( sqlHost, sqlUser, sqlPass, sqlDB ) ); std::shared_ptr b = openSerial( serialPath ); std::shared_ptr slip1( BIO_new( toBio() ), BIO_free ); - ( ( SlipBIO* )slip1->ptr )->setTarget( std::shared_ptr( new OpensslBIOWrapper( b ) ) ); + static_cast( slip1->ptr )->setTarget( std::shared_ptr( new OpensslBIOWrapper( b ) ) ); std::shared_ptr sign( new RemoteSigner( slip1, generateSSLContext( false ) ) ); // std::shared_ptr sign( new SimpleOpensslSigner() ); @@ -170,7 +170,7 @@ int main( int argc, const char* argv[] ) { continue; } catch( const char* c ) { log << "ERROR: " << c << std::endl; - } catch( std::string c ) { + } catch( std::string& c ) { log << "ERROR: " << c << std::endl; } @@ -178,7 +178,7 @@ int main( int argc, const char* argv[] ) { jp->failJob( job ); } catch( const char* c ) { log << "ERROR: " << c << std::endl; - } catch( std::string c ) { + } catch( std::string& c ) { log << "ERROR: " << c << std::endl; } } else if( job->task == "revoke" ) { diff --git a/src/apps/signer.cpp b/src/apps/signer.cpp index dc9e702..b607b9c 100644 --- a/src/apps/signer.cpp +++ b/src/apps/signer.cpp @@ -46,7 +46,7 @@ int main( int argc, const char* argv[] ) { std::shared_ptr conn = openSerial( serialPath ); std::shared_ptr slip1( BIO_new( toBio() ), BIO_free ); - ( ( SlipBIO* )slip1->ptr )->setTarget( std::shared_ptr( new OpensslBIOWrapper( conn ) ) ); + static_cast( slip1->ptr )->setTarget( std::shared_ptr( new OpensslBIOWrapper( conn ) ) ); DefaultRecordHandler* dh = new DefaultRecordHandler( std::shared_ptr( new SimpleOpensslSigner( ) ), slip1 ); diff --git a/src/crypto/X509.cpp b/src/crypto/X509.cpp index ba39b4b..d340a18 100644 --- a/src/crypto/X509.cpp +++ b/src/crypto/X509.cpp @@ -7,8 +7,7 @@ #include #include -X509Req::X509Req( X509_REQ* csr ) { - req = std::shared_ptr( csr, X509_REQ_free ); +X509Req::X509Req( X509_REQ* csr ) : req( csr, X509_REQ_free ) { EVP_PKEY* pkt = X509_REQ_get_pubkey( req.get() ); if( !pkt ) { @@ -48,7 +47,7 @@ int X509Req::verify() { return X509_REQ_verify( req.get(), pk.get() ); } -std::shared_ptr X509Req::getPkey() { +std::shared_ptr X509Req::getPkey() const { return pk; } diff --git a/src/crypto/X509.h b/src/crypto/X509.h index 0407999..ac0e3d3 100644 --- a/src/crypto/X509.h +++ b/src/crypto/X509.h @@ -19,7 +19,7 @@ public: static std::shared_ptr parseCSR( std::string content ); static std::shared_ptr parseSPKAC( std::string content ); int verify(); - std::shared_ptr getPkey(); + std::shared_ptr getPkey() const; }; class X509Cert { diff --git a/src/crypto/remoteSigner.cpp b/src/crypto/remoteSigner.cpp index b060f6c..b6c7c31 100644 --- a/src/crypto/remoteSigner.cpp +++ b/src/crypto/remoteSigner.cpp @@ -6,9 +6,7 @@ #include #include -RemoteSigner::RemoteSigner( std::shared_ptr target, std::shared_ptr ctx ) { - this->target = target; - this->ctx = ctx; +RemoteSigner::RemoteSigner( std::shared_ptr target, std::shared_ptr ctx ) : target( target ), ctx( ctx ) { } RemoteSigner::~RemoteSigner() { diff --git a/src/crypto/sslUtil.cpp b/src/crypto/sslUtil.cpp index d0710e0..753981c 100644 --- a/src/crypto/sslUtil.cpp +++ b/src/crypto/sslUtil.cpp @@ -17,7 +17,7 @@ std::shared_ptr ssl_lib_ref( CRYPTO_cleanup_all_ex_data(); } ); -std::shared_ptr loadX509FromFile( std::string filename ) { +std::shared_ptr loadX509FromFile( const std::string& filename ) { FILE* f = fopen( filename.c_str(), "r" ); if( !f ) { @@ -38,7 +38,7 @@ std::shared_ptr loadX509FromFile( std::string filename ) { } ); } -std::shared_ptr loadPkeyFromFile( std::string filename ) { +std::shared_ptr loadPkeyFromFile( const std::string& filename ) { FILE* f = fopen( filename.c_str(), "r" ); if( !f ) { @@ -162,7 +162,7 @@ void setupSerial( FILE* f ) { } } -std::shared_ptr openSerial( const std::string name ) { +std::shared_ptr openSerial( const std::string& name ) { FILE* f = fopen( name.c_str(), "r+" ); if( !f ) { @@ -175,9 +175,7 @@ std::shared_ptr openSerial( const std::string name ) { return b; } -CAConfig::CAConfig( std::string name ) { - this->name = name; - this->path = "ca/" + name; +CAConfig::CAConfig( const std::string& name ) : path( "ca/" + name ), name( name ) { ca = loadX509FromFile( path + "/ca.crt" ); caKey = loadPkeyFromFile( path + "/ca.key" ); ASN1_TIME* tm = X509_get_notBefore( ca ); diff --git a/src/crypto/sslUtil.h b/src/crypto/sslUtil.h index c4193fe..1327a17 100644 --- a/src/crypto/sslUtil.h +++ b/src/crypto/sslUtil.h @@ -18,7 +18,7 @@ public: std::shared_ptr ca; std::shared_ptr caKey; std::shared_ptr notBefore; - CAConfig( std::string name ); + CAConfig( const std::string& name ); bool crlNeedsResign(); }; @@ -43,10 +43,10 @@ struct Profile { extern std::shared_ptr ssl_lib_ref; -std::shared_ptr loadX509FromFile( std::string filename ); -std::shared_ptr loadPkeyFromFile( std::string filename ); +std::shared_ptr loadX509FromFile( const std::string& filename ); +std::shared_ptr loadPkeyFromFile( const std::string& filename ); std::shared_ptr generateSSLContext( bool server ); -std::shared_ptr openSerial( const std::string name ); +std::shared_ptr openSerial( const std::string& name ); std::string timeToString( std::shared_ptr time ); void extractTimes( std::shared_ptr source, std::shared_ptr cert ); diff --git a/src/io/bios.cpp b/src/io/bios.cpp index 15d318c..9a1ea57 100644 --- a/src/io/bios.cpp +++ b/src/io/bios.cpp @@ -5,27 +5,27 @@ namespace BIOWrapper { int write( BIO* b, const char* buf, int num ) { - return ( ( OpensslBIO* )b->ptr )->write( buf, num ); + return static_cast( b->ptr )->write( buf, num ); } int read( BIO* b, char* buf, int size ) { - return ( ( OpensslBIO* )b->ptr )->read( buf, size ); + return static_cast( b->ptr )->read( buf, size ); } int puts( BIO* b, const char* str ) { - return ( ( OpensslBIO* )b->ptr )->puts( str ); + return static_cast( b->ptr )->puts( str ); } int gets( BIO* b, char* str, int size ) { - return ( ( OpensslBIO* )b->ptr )->gets( str, size ); + return static_cast( b->ptr )->gets( str, size ); } long ctrl( BIO* b, int cmod, long arg1, void* arg2 ) { - return ( ( OpensslBIO* )b->ptr )->ctrl( cmod, arg1, arg2 ); + return static_cast( b->ptr )->ctrl( cmod, arg1, arg2 ); } int free( BIO* b ) { - delete( ( OpensslBIO* ) b->ptr ); + delete static_cast( b->ptr ); b->ptr = 0; return 0; } diff --git a/src/io/opensslBIO.cpp b/src/io/opensslBIO.cpp index 3562822..03122db 100644 --- a/src/io/opensslBIO.cpp +++ b/src/io/opensslBIO.cpp @@ -1,7 +1,6 @@ #include "opensslBIO.h" -OpensslBIOWrapper::OpensslBIOWrapper( std::shared_ptr b ) { - this->b = b; +OpensslBIOWrapper::OpensslBIOWrapper( std::shared_ptr b ) : b( b ) { } OpensslBIOWrapper::~OpensslBIOWrapper() { diff --git a/src/io/record.cpp b/src/io/record.cpp index 8f81d02..bd0cc1b 100644 --- a/src/io/record.cpp +++ b/src/io/record.cpp @@ -52,7 +52,7 @@ int32_t fromHexDigit( char c ) { return res; } -std::string parseCommand( RecordHeader& head, const std::string input, std::shared_ptr log ) { +std::string parseCommand( RecordHeader& head, const std::string& input, std::shared_ptr log ) { if( log ) { ( *log.get() ) << "FINE: RECORD input: " << input << std::endl; } diff --git a/src/io/record.h b/src/io/record.h index ba5a8d8..29c05ca 100644 --- a/src/io/record.h +++ b/src/io/record.h @@ -52,12 +52,12 @@ public: } template - void append( std::string& str, T val ) { + static void append( std::string& str, T val ) { str.append( ( char* ) &val, sizeof( T ) ); } template - void read( std::string::iterator& it, T& val ) { + static void read( std::string::iterator& it, T& val ) { char* data = ( char* ) &val; for( size_t i = 0; i < sizeof( T ); i++ ) { @@ -96,6 +96,6 @@ public: }; -std::string parseCommand( RecordHeader& head, const std::string input, std::shared_ptr log ); +std::string parseCommand( RecordHeader& head, const std::string& input, std::shared_ptr log ); void sendCommand( RecordHeader& head, const std::string& data, std::shared_ptr bio, std::shared_ptr log ); diff --git a/src/io/recordHandler.cpp b/src/io/recordHandler.cpp index 2ba71c2..c46c845 100644 --- a/src/io/recordHandler.cpp +++ b/src/io/recordHandler.cpp @@ -221,14 +221,8 @@ public: } }; -DefaultRecordHandler::DefaultRecordHandler( std::shared_ptr signer, std::shared_ptr bio ) : - currentSession() { - - this->signer = signer; - - ctx = generateSSLContext( true ); - - this->bio = bio; +DefaultRecordHandler::DefaultRecordHandler( std::shared_ptr signer, std::shared_ptr bio ) + : bio( bio ), ctx( generateSSLContext( true ) ), signer( signer ), currentSession() { } void DefaultRecordHandler::reset() { diff --git a/src/io/slipBio.cpp b/src/io/slipBio.cpp index 1a7ba04..fb3f6c1 100644 --- a/src/io/slipBio.cpp +++ b/src/io/slipBio.cpp @@ -37,27 +37,14 @@ std::string toHex( const char* buf, int len ) { return data; } -SlipBIO::SlipBIO() { - this->buffer = std::vector( BUFFER_SIZE ); - this->decodeTarget = 0; - this->decodePos = 0; - this->rawPos = 0; - this->failed = false; +SlipBIO::SlipBIO() : buffer( std::vector( BUFFER_SIZE ) ), decodeTarget( 0 ), decodePos( 0 ), rawPos( 0 ), failed( false ) { } void SlipBIO::setTarget( std::shared_ptr target ) { this->target = target; } -SlipBIO::SlipBIO( std::shared_ptr target ) { - this->target = target; - - this->buffer = std::vector( BUFFER_SIZE ); - this->decodeTarget = 0; - this->decodePos = 0; - this->rawPos = 0; - - this->failed = false; +SlipBIO::SlipBIO( std::shared_ptr target ) : target( target ), buffer( std::vector( BUFFER_SIZE ) ), decodeTarget( 0 ), decodePos( 0 ), rawPos( 0 ), failed( false ) { } SlipBIO::~SlipBIO() {} -- 2.39.5 From 7da0e9e3fcc0e3a08af49ae16bb82cecaf160560 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Sat, 31 Jan 2015 03:27:13 +0100 Subject: [PATCH 02/16] upd: Make testcases cppcheck-compliant --- test/src/slipBioTest.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/src/slipBioTest.cpp b/test/src/slipBioTest.cpp index d579eb3..8f967de 100644 --- a/test/src/slipBioTest.cpp +++ b/test/src/slipBioTest.cpp @@ -12,15 +12,12 @@ class OpensslBIOVector : public OpensslBIO { private: - std::vector>::iterator it, end; std::vector> input; + std::vector>::iterator it, end; public: std::vector> result = std::vector>(); - OpensslBIOVector( std::vector> data ) { - input = data; - it = input.begin(); - end = input.end(); + OpensslBIOVector( std::vector> data ) : input( data ), it( input.begin() ), end( input.end() ) { } int write( const char* buf, int num ); @@ -46,7 +43,7 @@ int OpensslBIOVector::read( char* buf, int size ) { std::copy( it->begin(), it->end(), buf ); auto result = it->size(); - it++; + ++it; return result; } -- 2.39.5 From 28e331e99235c616e8ccd27b7b5083bae47a1c1b Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Tue, 10 Feb 2015 00:53:03 +0100 Subject: [PATCH 03/16] ADD: gcov make launch --- .gitignore | 2 ++ Makefile | 15 +++++++++++++++ src/util.cpp | 2 +- test/Makefile | 6 ++++++ test/src/time.cpp | 6 ++---- 5 files changed, 26 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 56e0b1c..2bc7011 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,5 @@ obj *.csr config.txt serial +*.gcov +gcov.log diff --git a/Makefile b/Makefile index d39af3e..12bf2e4 100644 --- a/Makefile +++ b/Makefile @@ -36,6 +36,12 @@ CFLAGS+=${ADDFLAGS} -Wall -Werror -Wextra -pedantic -std=c++11 -Ilib/openssl/inc CXXFLAGS=$(CFLAGS) LDFLAGS+=${ADDFLAGS} -L/usr/lib/i386-linux-gnu/ -lssl -lcrypto -ldl -Llib/openssl +ifneq (,$(filter coverage,$(DEB_BUILD_OPTIONS))) + LDFLAGS += -lgcov + CFLAGS += -fprofile-arcs -ftest-coverage +endif + + SRC_DIR=src OBJ_DIR=obj DEP_DIR=dep @@ -55,6 +61,8 @@ all: build .PHONY: clean clean:: -rm -rf .libs + -rm -rf *.gcov + -rm -rf gcov.log -rm -rf *.a -rm -rf *.d -rm -rf *.o @@ -96,6 +104,13 @@ openssl: collissiondetect: ${MAKE} -C lib/collissiondetect +.PHONY: coverage +coverage: + find . -name "*.gcda" -exec rm {} + + ${MAKE} "DEB_BUILD_OPTIONS=coverage noopt" + find obj -name "*.gcda" -exec gcov -p {} + > gcov.log + + # -------- cassiopeia: bin/cassiopeia bin/cassiopeia-signer diff --git a/src/util.cpp b/src/util.cpp index f3d95c0..20d4660 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -78,7 +78,7 @@ std::pair parseDate( const std::string& date ) { std::size_t siz = strftime( check, 11, "%Y-%m-%d", &t ); if( siz != 10 ) { - return std::pair( false, 0 ); + return std::pair( false, 0 ); // NO-COVERAGE (by contract of strftime) } std::string checkS( check, siz ); diff --git a/test/Makefile b/test/Makefile index 114e9ee..68e794e 100644 --- a/test/Makefile +++ b/test/Makefile @@ -23,6 +23,12 @@ CFLAGS+=${ADDFLAGS} -Wall -Werror -Wextra -pedantic -std=c++11 -I../src -I../lib CXXFLAGS=$(CFLAGS) LDFLAGS+=${ADDFLAGS} -lmysqlclient -lssl -lcrypto -ldl -lboost_unit_test_framework -L../lib/openssl -L/usr/lib/i386-linux-gnu +ifneq (,$(filter coverage,$(DEB_BUILD_OPTIONS))) + LDFLAGS += -lgcov + CFLAGS += -fprofile-arcs -ftest-coverage +endif + + SRC_DIR=src OBJ_DIR=obj DEP_DIR=dep diff --git a/test/src/time.cpp b/test/src/time.cpp index ccdd1f4..b6081d0 100644 --- a/test/src/time.cpp +++ b/test/src/time.cpp @@ -8,10 +8,6 @@ BOOST_AUTO_TEST_SUITE( TestTime ) BOOST_AUTO_TEST_CASE( testParseDate ) { - if( 1 ) { - return; - } - auto r = parseDate( "2012-01-01" ); BOOST_CHECK( r.first ); BOOST_CHECK( r.second == 1325376000 ); @@ -24,6 +20,7 @@ BOOST_AUTO_TEST_CASE( testParseDate ) { BOOST_CHECK( !( parseDate( "hallo" ) ).first ); BOOST_CHECK( !( parseDate( "aaaa-aa-aa" ) ).first ); BOOST_CHECK( !( parseDate( "32-12-12" ) ).first ); + BOOST_CHECK( !( parseDate( "2000-12-01z" ) ).first ); BOOST_CHECK( !( parseDate( "2000-13-01" ) ).first ); BOOST_CHECK( !( parseDate( "2000-00-01" ) ).first ); BOOST_CHECK( !( parseDate( "2000-02-30" ) ).first ); @@ -71,6 +68,7 @@ BOOST_AUTO_TEST_CASE( testInvalidInverval ) { BOOST_CHECK( !( parseMonthInterval( base, "--2m" ).first ) ); BOOST_CHECK( !( parseMonthInterval( base, "25m" ).first ) ); // too big + BOOST_CHECK( !( parseYearInterval( base, "12k" ).first ) ); BOOST_CHECK( !( parseYearInterval( base, "12my" ).first ) ); BOOST_CHECK( !( parseYearInterval( base, "-2m2y" ).first ) ); BOOST_CHECK( !( parseYearInterval( base, "--2y" ).first ) ); -- 2.39.5 From 98913da09f32fd12c5e1bfda53f823498befe082 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Tue, 10 Feb 2015 02:15:58 +0100 Subject: [PATCH 04/16] UPD: calling 'getSignature' on unsigned CRLs violates contract (from now on) due to openssl, strange behaviour --- test/src/CRL.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/test/src/CRL.cpp b/test/src/CRL.cpp index c3c999b..f28ef1d 100644 --- a/test/src/CRL.cpp +++ b/test/src/CRL.cpp @@ -14,7 +14,6 @@ BOOST_AUTO_TEST_CASE( SeperateSignature ) { std::shared_ptr ca = CAs.at( "unassured" ); CRL c( "" ); - c.setSignature( c.getSignature() ); c.sign( ca ); std::string oldsig = c.getSignature(); BOOST_CHECK( c.verify( ca ) ); -- 2.39.5 From fb2b78dcfe1a347973f3f9d7c90025ab5c7f83a7 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Tue, 10 Feb 2015 21:32:26 +0100 Subject: [PATCH 05/16] UPD: use lcov for coverage --- .gitignore | 3 +++ Makefile | 10 ++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 2bc7011..a74ae4c 100644 --- a/.gitignore +++ b/.gitignore @@ -37,3 +37,6 @@ config.txt serial *.gcov gcov.log + +coverage +coverage.info diff --git a/Makefile b/Makefile index 12bf2e4..116ec78 100644 --- a/Makefile +++ b/Makefile @@ -106,10 +106,12 @@ collissiondetect: .PHONY: coverage coverage: - find . -name "*.gcda" -exec rm {} + - ${MAKE} "DEB_BUILD_OPTIONS=coverage noopt" - find obj -name "*.gcda" -exec gcov -p {} + > gcov.log - + find . -name "*.gcda" -exec rm {} + &&\ + rm -rf coverage &&\ + rm -rf coverage.info &&\ + ${MAKE} "DEB_BUILD_OPTIONS=coverage noopt" &&\ + lcov -c --directory obj --directory test/obj --output-file coverage.info &&\ + genhtml -p $(shell pwd) coverage.info --output-directory coverage # -------- -- 2.39.5 From a477eeab986cf4307f74d1b4601cc46a7b0173c0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Fri, 13 Feb 2015 12:51:51 +0100 Subject: [PATCH 06/16] upd: striping system lib crom coverage --- .gitignore | 1 + Makefile | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index a74ae4c..03358dd 100644 --- a/.gitignore +++ b/.gitignore @@ -40,3 +40,4 @@ gcov.log coverage coverage.info +coverage_stripped.info diff --git a/Makefile b/Makefile index 116ec78..b7fc179 100644 --- a/Makefile +++ b/Makefile @@ -108,10 +108,11 @@ collissiondetect: coverage: find . -name "*.gcda" -exec rm {} + &&\ rm -rf coverage &&\ - rm -rf coverage.info &&\ + rm -rf coverage.info coverage_stripped.info &&\ ${MAKE} "DEB_BUILD_OPTIONS=coverage noopt" &&\ lcov -c --directory obj --directory test/obj --output-file coverage.info &&\ - genhtml -p $(shell pwd) coverage.info --output-directory coverage + lcov -r coverage.info "/usr/**" -o coverage_stripped.info &&\ + genhtml -p $(shell pwd) coverage_stripped.info --output-directory coverage # -------- -- 2.39.5 From a2b12c4b2819ffb4aa5409f5122c7f356f9776ef Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Tue, 10 Feb 2015 21:53:56 +0100 Subject: [PATCH 07/16] ADD: test for mysql interface (requires config.txt) --- test/src/sql.cpp | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 test/src/sql.cpp diff --git a/test/src/sql.cpp b/test/src/sql.cpp new file mode 100644 index 0000000..0b45e63 --- /dev/null +++ b/test/src/sql.cpp @@ -0,0 +1,67 @@ +#include +#include +#include +#include + +extern std::string sqlHost, sqlUser, sqlPass, sqlDB; + +BOOST_AUTO_TEST_SUITE( TestTime ) + +BOOST_AUTO_TEST_CASE( testSQL ) { + BOOST_REQUIRE( parseConfig("config.txt") == 0 ); + std::shared_ptr jp( new MySQLJobProvider( sqlHost, sqlUser, sqlPass, sqlDB ) ); + BOOST_REQUIRE( jp->query( "TRUNCATE TABLE profiles" ).first == 0 ); + BOOST_REQUIRE( jp->query( "TRUNCATE TABLE certs" ).first == 0 ); + BOOST_REQUIRE( jp->query( "TRUNCATE TABLE certAvas" ).first == 0 ); + BOOST_REQUIRE( jp->query( "TRUNCATE TABLE subjectAlternativeNames" ).first == 0 ); + BOOST_REQUIRE( jp->query( "TRUNCATE TABLE jobs" ).first == 0 ); + BOOST_REQUIRE( jp->query( "INSERT INTO profiles SET id='1', keyname='assured', keyUsage='', extendedKeyUsage='', name='assured'" ).first == 0 ); + BOOST_REQUIRE( jp->query( "INSERT INTO jobs SET task='sign', targetId='1'" ).first == 0 ); + + std::shared_ptr job = jp->fetchJob(); + BOOST_REQUIRE( job ); + jp->failJob(job); + BOOST_REQUIRE_EQUAL( job->target, "1" ); + BOOST_REQUIRE_EQUAL( job->task, "sign" ); + job = jp->fetchJob(); + BOOST_REQUIRE( job ); + std::shared_ptr cert = jp->fetchTBSCert(job); + BOOST_REQUIRE( !cert ); + BOOST_REQUIRE( jp->query( "INSERT INTO certs SET csr_type='CSR', id='1', profile='1'" ).first == 0 ); + BOOST_REQUIRE( jp->query( "INSERT INTO subjectAlternativeNames SET certId='1', contents='example.org', type='DNS'" ).first == 0 ); + BOOST_REQUIRE( jp->query( "INSERT INTO certAvas SET certid='1', name='CN', value='example.org'" ).first == 0 ); + cert = jp->fetchTBSCert(job); + BOOST_REQUIRE( cert ); + + std::shared_ptr fcert( new SignedCertificate() ); + fcert->certificate="CERT"; + fcert->serial="1234"; + fcert->crt_name="crt.name.crt"; + fcert->ca_name="assured"; + jp->writeBack( job, fcert ); + jp->finishJob( job ); + BOOST_REQUIRE( !jp->fetchJob() ); + BOOST_REQUIRE( jp->query( "INSERT INTO jobs SET task='revoke', targetId='1'" ).first == 0 ); + job = jp->fetchJob(); + BOOST_REQUIRE_EQUAL( job->target, "1" ); + BOOST_REQUIRE_EQUAL( job->task, "revoke" ); + std::pair revocationInfo = jp->getRevocationInfo( job ); + BOOST_REQUIRE_EQUAL( revocationInfo.first, "1234"); + BOOST_REQUIRE_EQUAL( revocationInfo.second, "assured"); + jp->writeBackRevocation( job, "2000-01-01 01:01:01" ); + jp->finishJob( job ); +} + +BOOST_AUTO_TEST_CASE( testSQLDisconnected ) { + //if(1) return; + //BOOST_REQUIRE( parseConfig("config.txt") == 0 ); + std::shared_ptr jp( new MySQLJobProvider( sqlHost, sqlUser, sqlPass, sqlDB ) ); + jp->disconnect(); + jp->disconnect(); + BOOST_REQUIRE( jp->query("SELECT 1").first); + BOOST_REQUIRE_THROW( jp->escape_string("uia"), const char * ); + BOOST_REQUIRE_THROW( jp->finishJob(std::shared_ptr()), const char * ); + BOOST_REQUIRE_THROW( jp->failJob(std::shared_ptr()), const char * ); +} + +BOOST_AUTO_TEST_SUITE_END() -- 2.39.5 From 5e63f619c0ea0d16babd26d98d14489c311c5ebe Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Sat, 21 Feb 2015 01:40:03 +0100 Subject: [PATCH 08/16] Fix: coverity complaining about memory corruption --- src/io/record.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/io/record.h b/src/io/record.h index 29c05ca..27c40c1 100644 --- a/src/io/record.h +++ b/src/io/record.h @@ -57,8 +57,8 @@ public: } template - static void read( std::string::iterator& it, T& val ) { - char* data = ( char* ) &val; + static void read( std::string::iterator& it, T&& val ) { + char* data = reinterpret_cast( &val ); for( size_t i = 0; i < sizeof( T ); i++ ) { data[i] = *it; -- 2.39.5 From cbaea9cfa01351920e7c131332051dda09718ae4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Sat, 21 Feb 2015 01:48:52 +0100 Subject: [PATCH 09/16] Fix: uninitialized fields --- src/io/recordHandler.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/io/recordHandler.cpp b/src/io/recordHandler.cpp index c46c845..67214e3 100644 --- a/src/io/recordHandler.cpp +++ b/src/io/recordHandler.cpp @@ -38,7 +38,9 @@ public: std::vector serials; RecordHandlerSession( DefaultRecordHandler* parent, std::shared_ptr signer, std::shared_ptr ctx, std::shared_ptr output ) : - tbs( new TBSCertificate() ) { + sessid( 0 ), + lastCommandCount( 0 ), + tbs( new TBSCertificate() ){ this->parent = parent; this->signer = signer; time_t c_time; -- 2.39.5 From 1aee468deafb447ebda1b6034c5233f13a5058c5 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Sun, 22 Feb 2015 15:46:27 +0100 Subject: [PATCH 10/16] fix: Use a less arcane way of typecasting from char[] to T --- src/io/record.h | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/io/record.h b/src/io/record.h index 27c40c1..8857665 100644 --- a/src/io/record.h +++ b/src/io/record.h @@ -57,13 +57,21 @@ public: } template - static void read( std::string::iterator& it, T&& val ) { - char* data = reinterpret_cast( &val ); + static void read( std::string::const_iterator& it, T& val ) { + union typeConversion { + char buf[sizeof(T)]; + T value; + + typeConversion(const T& v) : value(v) {} + }; + + typeConversion data( 0 ); for( size_t i = 0; i < sizeof( T ); i++ ) { - data[i] = *it; - it++; + data.buf[i] = *it++; } + + val = data.value; } std::string packToString() { @@ -79,12 +87,12 @@ public: return res; } - void unpackFromString( std::string str ) { + void unpackFromString( const std::string& str ) { if( str.size() != RECORD_HEADER_SIZE ) { throw "Invalid string length"; } - auto it = str.begin(); + auto it = str.cbegin(); read( it, command ); read( it, flags ); read( it, sessid ); -- 2.39.5 From e4a2b7cebcc4b38f1b7181f02c6fcb694fa0af8a Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Mon, 23 Feb 2015 19:33:48 +0100 Subject: [PATCH 11/16] fix: Check return value of writeBackFile for non-empty filename --- src/apps/client.cpp | 8 +++++++- src/crypto/simpleOpensslSigner.cpp | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/apps/client.cpp b/src/apps/client.cpp index 7c5bf08..e30803e 100644 --- a/src/apps/client.cpp +++ b/src/apps/client.cpp @@ -159,8 +159,14 @@ int main( int argc, const char* argv[] ) { log << "FINE: CERTIFICATE LOG: " << res->log << std::endl; log << "FINE: CERTIFICATE:" << std::endl << res->certificate << std::endl; std::string fn = writeBackFile( job->target.c_str(), res->certificate, keyDir ); + if( fn.empty() ) { + log << "ERROR: Writeback of the certificate failed." << std::endl; + jp->failJob( job ); + continue; + } + res->crt_name = fn; - jp->writeBack( job, res ); + jp->writeBack( job, res ); //! \FIXME: Check return value log << "FINE: signing done." << std::endl; if( DAEMON ) { diff --git a/src/crypto/simpleOpensslSigner.cpp b/src/crypto/simpleOpensslSigner.cpp index dce6025..f7042a2 100644 --- a/src/crypto/simpleOpensslSigner.cpp +++ b/src/crypto/simpleOpensslSigner.cpp @@ -190,7 +190,13 @@ std::shared_ptr SimpleOpensslSigner::sign( std::shared_ptr output = c.sign( ca->caKey, cert->md ); signlog << "FINE: all went well" << std::endl; - signlog << "FINE: crt went to: " << writeBackFile( num, output->certificate, ca->path ) << std::endl; + std::string fn = writeBackFile( num, output->certificate, ca->path ); + if( fn.empty() ) { + signlog << "ERROR: failed to get filename for storage of signed certificate." << std::endl; + throw "Storage location could not be determined"; + } + + signlog << "FINE: crt went to: " << fn << std::endl; output->ca_name = ca->name; output->log = signlog.str(); return output; -- 2.39.5 From de2271154502f2fe7bc6259208bcb092d4e82ae3 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Mon, 23 Feb 2015 19:34:16 +0100 Subject: [PATCH 12/16] fix: Check the CA certificate file could be loaded --- src/crypto/sslUtil.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/crypto/sslUtil.cpp b/src/crypto/sslUtil.cpp index 753981c..5855e66 100644 --- a/src/crypto/sslUtil.cpp +++ b/src/crypto/sslUtil.cpp @@ -94,7 +94,9 @@ std::shared_ptr generateSSLContext( bool server ) { SSL_CTX_set_verify( ctx.get(), SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, verify_callback ); SSL_CTX_use_certificate_file( ctx.get(), server ? "keys/signer_server.crt" : "keys/signer_client.crt", SSL_FILETYPE_PEM ); SSL_CTX_use_PrivateKey_file( ctx.get(), server ? "keys/signer_server.key" : "keys/signer_client.key", SSL_FILETYPE_PEM ); - SSL_CTX_load_verify_locations( ctx.get(), "keys/ca.crt", 0 ); + if( 1 != SSL_CTX_load_verify_locations( ctx.get(), "keys/ca.crt", 0 ) ) { + throw "Cannot load CA store for certificate validation."; + } if( server ) { STACK_OF( X509_NAME ) *names = SSL_load_client_CA_file( "keys/env.crt" ); -- 2.39.5 From 91814cbc6f74173f40278062f8de1859c9b2e7c4 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Mon, 23 Feb 2015 19:34:58 +0100 Subject: [PATCH 13/16] fix: Properly check for success to create the necessary directories --- src/util.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/util.cpp b/src/util.cpp index 20d4660..5cf30da 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -25,10 +25,17 @@ std::string readFile( const std::string& name ) { } std::string writeBackFile( const std::string& serial, const std::string& cert, const std::string& keydir ) { + errno = 0; + std::string filename = keydir; - mkdir( filename.c_str(), 0755 ); + if( 0 != mkdir( filename.c_str(), 0755 ) ) { + return ""; + } + filename += "/crt"; - mkdir( filename.c_str(), 0755 ); + if( 0 != mkdir( filename.c_str(), 0755 ) ) { + return ""; + } std::string first; if( serial.length() < 3 ) { @@ -38,12 +45,15 @@ std::string writeBackFile( const std::string& serial, const std::string& cert, c } filename += "/" + first; - mkdir( filename.c_str(), 0755 ); + if( 0 != mkdir( filename.c_str(), 0755 ) ) { + return ""; + } filename += "/" + serial + ".crt"; writeFile( filename, cert ); return filename; } + bool isDigit( char c ) { return ( c >= '0' ) && ( c <= '9' ); } -- 2.39.5 From 87325777087f80af2065a09977ae6abf39bc50d1 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Mon, 23 Feb 2015 19:41:04 +0100 Subject: [PATCH 14/16] fix: Refine Error Management and note about further plces that need care --- src/util.cpp | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/util.cpp b/src/util.cpp index 5cf30da..ed7b42e 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -14,6 +14,8 @@ void writeFile( const std::string& name, const std::string& content ) { file.open( name ); file << content; file.close(); + + //! \FIXME: Error checking } std::string readFile( const std::string& name ) { @@ -29,12 +31,20 @@ std::string writeBackFile( const std::string& serial, const std::string& cert, c std::string filename = keydir; if( 0 != mkdir( filename.c_str(), 0755 ) ) { - return ""; + if( EEXIST != errno ) { + return ""; + } + + //! \FIXME: Check this is a directory } filename += "/crt"; if( 0 != mkdir( filename.c_str(), 0755 ) ) { - return ""; + if( EEXIST != errno ) { + return ""; + } + + //! \FIXME: Check this is a directory } std::string first; @@ -46,8 +56,13 @@ std::string writeBackFile( const std::string& serial, const std::string& cert, c filename += "/" + first; if( 0 != mkdir( filename.c_str(), 0755 ) ) { - return ""; + if( EEXIST != errno ) { + return ""; + } + + //! \FIXME: Check this is a directory } + filename += "/" + serial + ".crt"; writeFile( filename, cert ); -- 2.39.5 From db26a9bc01d5ee5f34557fa1ea00feca5be3b79f Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Mon, 23 Feb 2015 21:13:29 +0100 Subject: [PATCH 15/16] chg: Use automatic memory and resource management --- src/crypto/X509.cpp | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/crypto/X509.cpp b/src/crypto/X509.cpp index d340a18..a068957 100644 --- a/src/crypto/X509.cpp +++ b/src/crypto/X509.cpp @@ -247,15 +247,28 @@ std::shared_ptr X509Cert::sign( std::shared_ptr caK //X509_print_fp( stdout, target.get() ); std::shared_ptr mem = std::shared_ptr( BIO_new( BIO_s_mem() ), BIO_free ); + if( !mem ) { + throw "Failed to allocate memory for the signed certificate."; + } PEM_write_bio_X509( mem.get(), target.get() ); - BUF_MEM* buf; + + BUF_MEM* buf = NULL; BIO_get_mem_ptr( mem.get(), &buf ); + std::shared_ptr res = std::shared_ptr( new SignedCertificate() ); res->certificate = std::string( buf->data, buf->data + buf->length ); - BIGNUM* ser = ASN1_INTEGER_to_BN( target->cert_info->serialNumber, NULL ); - char* serStr = BN_bn2hex( ser ); - res->serial = std::string( serStr ); - OPENSSL_free( serStr ); - BN_free( ser ); + + std::shared_ptr ser( ASN1_INTEGER_to_BN( target->cert_info->serialNumber, NULL ), BN_free ); + if( !ser ) { + throw "Failed to retrieve certificate serial of signed certificate."; + } + + std::shared_ptr serStr( + BN_bn2hex( ser.get() ), + []( char *p ) { + OPENSSL_free(p); + } ); // OPENSSL_free is a macro... + res->serial = serStr ? std::string( serStr.get() ) : ""; + return res; } -- 2.39.5 From 47c4827215e38321868c1d07cd81cf1107d9508f Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Mon, 23 Feb 2015 21:33:55 +0100 Subject: [PATCH 16/16] chg: Use std::shared_ptr for resource management --- src/crypto/sslUtil.cpp | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/crypto/sslUtil.cpp b/src/crypto/sslUtil.cpp index 5855e66..c011b3c 100644 --- a/src/crypto/sslUtil.cpp +++ b/src/crypto/sslUtil.cpp @@ -18,14 +18,13 @@ std::shared_ptr ssl_lib_ref( } ); std::shared_ptr loadX509FromFile( const std::string& filename ) { - FILE* f = fopen( filename.c_str(), "r" ); + std::shared_ptr f( fopen( filename.c_str(), "r" ), fclose ); if( !f ) { return std::shared_ptr(); } - X509* key = PEM_read_X509( f, NULL, NULL, 0 ); - fclose( f ); + X509* key = PEM_read_X509( f.get(), NULL, NULL, 0 ); if( !key ) { return std::shared_ptr(); @@ -39,14 +38,13 @@ std::shared_ptr loadX509FromFile( const std::string& filename ) { } std::shared_ptr loadPkeyFromFile( const std::string& filename ) { - FILE* f = fopen( filename.c_str(), "r" ); + std::shared_ptr f( fopen( filename.c_str(), "r" ), fclose ); if( !f ) { return std::shared_ptr(); } - EVP_PKEY* key = PEM_read_PrivateKey( f, NULL, NULL, 0 ); - fclose( f ); + EVP_PKEY* key = PEM_read_PrivateKey( f.get(), NULL, NULL, 0 ); if( !key ) { return std::shared_ptr(); @@ -108,11 +106,10 @@ std::shared_ptr generateSSLContext( bool server ) { } if( !dh_param ) { - FILE* paramfile = fopen( "dh_param.pem", "r" ); + std::shared_ptr paramfile( fopen( "dh_param.pem", "r" ), fclose ); if( paramfile ) { - dh_param = std::shared_ptr( PEM_read_DHparams( paramfile, NULL, NULL, NULL ), DH_free ); - fclose( paramfile ); + dh_param = std::shared_ptr( PEM_read_DHparams( paramfile.get(), NULL, NULL, NULL ), DH_free ); } else { dh_param = std::shared_ptr( DH_new(), DH_free ); std::cout << "Generating DH params" << std::endl; @@ -126,11 +123,10 @@ std::shared_ptr generateSSLContext( bool server ) { } std::cout << std::endl; - paramfile = fopen( "dh_param.pem", "w" ); + paramfile = std::shared_ptr( fopen( "dh_param.pem", "w" ), fclose ); if( paramfile ) { - PEM_write_DHparams( paramfile, dh_param.get() ); - fclose( paramfile ); + PEM_write_DHparams( paramfile.get(), dh_param.get() ); } } } @@ -143,10 +139,10 @@ std::shared_ptr generateSSLContext( bool server ) { return ctx; } -void setupSerial( FILE* f ) { +void setupSerial( std::shared_ptr f ) { struct termios attr; - if( tcgetattr( fileno( f ), &attr ) ) { + if( tcgetattr( fileno( f.get() ), &attr ) ) { throw "failed to get attrs"; } @@ -159,13 +155,13 @@ void setupSerial( FILE* f ) { cfsetispeed( &attr, B115200 ); cfsetospeed( &attr, B115200 ); - if( tcsetattr( fileno( f ), TCSANOW, &attr ) ) { + if( tcsetattr( fileno( f.get() ), TCSANOW, &attr ) ) { throw "failed to get attrs"; } } std::shared_ptr openSerial( const std::string& name ) { - FILE* f = fopen( name.c_str(), "r+" ); + std::shared_ptr f( fopen( name.c_str(), "r+" ), fclose ); if( !f ) { std::cout << "Opening serial device failed" << std::endl; @@ -173,8 +169,11 @@ std::shared_ptr openSerial( const std::string& name ) { } setupSerial( f ); - std::shared_ptr b( BIO_new_fd( fileno( f ), 0 ), BIO_free ); - return b; + return std::shared_ptr( + BIO_new_fd( fileno( f.get() ), 0 ), + [f]( BIO* b ) { + BIO_free(b); + } ); } CAConfig::CAConfig( const std::string& name ) : path( "ca/" + name ), name( name ) { -- 2.39.5