Skip to content

Commit

Permalink
Minimal fix for Debian bug 907135 [#36]
Browse files Browse the repository at this point in the history
Unfortunately, the changes required to implement the full solution to Debian
bug 907135 were quite large and could not be reviewed in time for Debian 10's
release date. This would have meant that Box Backup was not available at all in
Debian 10.

Therefore we have developed a workaround specifically for Debian 10 users
(this patch), which contains only the minimal changes needed to:

* reduce the security level for Box Backup to 1 (the previous default),
* overriding the system default; ensure that all newly generated certificates
* meet the new security requirements that will later be imposed.

This interim version will hopefully be replaced by a version from the master
branch that supports the SSLSecurityLevel configuration option, which we hope
to see in debian-backports as soon as possible, and we recommend that anyone
using the interim version upgrade to this master version as soon as possible.

See
https://github.com/boxbackup/boxbackup/wiki/WeakSSLCertificates#workaround-2
for more details.
  • Loading branch information
qris committed Jun 4, 2019
1 parent 2f5b556 commit 27ad0d8
Show file tree
Hide file tree
Showing 48 changed files with 1,359 additions and 18 deletions.
2 changes: 1 addition & 1 deletion bin/bbackupd/bbackupd-config.in
Expand Up @@ -169,7 +169,7 @@ if(!-f $private_key)
if(!-f $certificate_request)
{
die "Couldn't run openssl for CSR generation" unless
open(CSR,"|openssl req -new -key $private_key -sha1 -out $certificate_request");
open(CSR,"|openssl req -new -key $private_key -sha256 -out $certificate_request");
print CSR <<__E;
.
.
Expand Down
8 changes: 4 additions & 4 deletions bin/bbstored/bbstored-certs.in
Expand Up @@ -122,7 +122,7 @@ sub cmd_init_create_root

# make CSR
die "Couldn't run openssl for CSR generation" unless
open(CSR,"|openssl req -new -key $key -sha1 -out $csr");
open(CSR,"|openssl req -new -key $key -sha256 -out $csr");
print CSR <<__E;
.
.
Expand All @@ -140,7 +140,7 @@ __E
die "Certificate request wasn't created.\n" unless -f $csr;

# sign it to make a self-signed root CA key
if(system("openssl x509 -req -in $csr -sha1 -extensions v3_ca -signkey $key -out $cert -days $root_sign_period") != 0)
if(system("openssl x509 -req -in $csr -sha256 -extensions v3_ca -signkey $key -out $cert -days $root_sign_period") != 0)
{
die "Couldn't generate root certificate."
}
Expand Down Expand Up @@ -201,7 +201,7 @@ __E
my $out_cert = "$cert_dir/clients/$acc"."-cert.pem";

# sign it!
if(system("openssl x509 -req -in $csr -sha1 -extensions usr_crt -CA $cert_dir/roots/clientCA.pem -CAkey $cert_dir/keys/clientRootKey.pem -out $out_cert -days $sign_period") != 0)
if(system("openssl x509 -req -in $csr -sha256 -extensions usr_crt -CA $cert_dir/roots/clientCA.pem -CAkey $cert_dir/keys/clientRootKey.pem -out $out_cert -days $sign_period") != 0)
{
die "Signing failed"
}
Expand Down Expand Up @@ -257,7 +257,7 @@ __E
my $out_cert = "$cert_dir/servers/$common_name"."-cert.pem";

# sign it!
if(system("openssl x509 -req -in $csr -sha1 -extensions usr_crt -CA $cert_dir/roots/serverCA.pem -CAkey $cert_dir/keys/serverRootKey.pem -out $out_cert -days $sign_period") != 0)
if(system("openssl x509 -req -in $csr -sha256 -extensions usr_crt -CA $cert_dir/roots/serverCA.pem -CAkey $cert_dir/keys/serverRootKey.pem -out $out_cert -days $sign_period") != 0)
{
die "Signing failed"
}
Expand Down
19 changes: 12 additions & 7 deletions infrastructure/cmake/CMakeLists.txt
Expand Up @@ -71,13 +71,6 @@ function(move_file_if_exists source_file dest_file)
endif()
endfunction()

foreach(file_to_configure ${files_to_configure})
configure_file("${base_dir}/${file_to_configure}.in" "${base_dir}/${file_to_configure}.out" @ONLY)
replace_file_if_different(
"${base_dir}/${file_to_configure}"
"${base_dir}/${file_to_configure}.out")
endforeach()

# If BOXBACKUP_VERSION is defined when running CMake (as the AppVeyor config does), use it
# as-is, since it contains the full version number, branch, and platform (Win32/Win64):
if(BOXBACKUP_VERSION)
Expand Down Expand Up @@ -375,6 +368,7 @@ file(WRITE "${boxconfig_h_file}" "// Auto-generated by CMake. Do not edit.\n")

if(WIN32)
target_link_libraries(lib_common PUBLIC ws2_32 gdi32)
list(APPEND CMAKE_REQUIRED_LIBRARIES ws2_32 gdi32)
endif()

# On Windows we want to statically link zlib to make debugging and distribution easier,
Expand Down Expand Up @@ -430,6 +424,7 @@ else()
endif()
include_directories(${OPENSSL_INCLUDE_DIR})
target_link_libraries(lib_crypto PUBLIC ${OPENSSL_LIBRARIES})
list(APPEND CMAKE_REQUIRED_LIBRARIES ${OPENSSL_LIBRARIES})

# Link to PCRE
if (WIN32)
Expand Down Expand Up @@ -613,6 +608,9 @@ foreach(function_name ${detect_functions})
file(APPEND "${boxconfig_h_file}" "#cmakedefine HAVE_${platform_var_name}\n")
endforeach()

check_function_exists(SSL_CTX_set_security_level HAVE_SSL_CTX_SET_SECURITY_LEVEL)
file(APPEND "${boxconfig_h_file}" "#cmakedefine HAVE_SSL_CTX_SET_SECURITY_LEVEL\n")

check_symbol_exists(dirfd "dirent.h" HAVE_DECL_DIRFD)
file(APPEND "${boxconfig_h_file}" "#cmakedefine01 HAVE_DECL_DIRFD\n")

Expand Down Expand Up @@ -829,6 +827,13 @@ file(TO_NATIVE_PATH "${PERL_EXECUTABLE}" perl_executable_native)
string(REPLACE "\\" "\\\\" perl_path_escaped ${perl_executable_native})
target_compile_definitions(test_backupstorefix PRIVATE -DPERL_EXECUTABLE="${perl_path_escaped}")

foreach(file_to_configure ${files_to_configure})
configure_file("${base_dir}/${file_to_configure}.in" "${base_dir}/${file_to_configure}.out" @ONLY)
replace_file_if_different(
"${base_dir}/${file_to_configure}"
"${base_dir}/${file_to_configure}.out")
endforeach()

# Configure test timeouts:
# I've set the timeout to 4 times as long as it took to run on a particular run on Appveyor:
# https://ci.appveyor.com/project/qris/boxbackup/build/job/xm10itascygtu93j
Expand Down
3 changes: 2 additions & 1 deletion infrastructure/m4/boxbackup_tests.m4
Expand Up @@ -142,7 +142,8 @@ AC_SEARCH_LIBS(
Upgrade or read the documentation for alternatives]])
fi
])

AC_CHECK_FUNCS([SSL_CTX_set_security_level], [HAVE_SSL_CTX_SET_SECURITY_LEVEL=1])
AC_SUBST([HAVE_SSL_CTX_SET_SECURITY_LEVEL])

### Checks for header files.

Expand Down
4 changes: 4 additions & 0 deletions lib/common/BoxPortsAndFiles.h.in
Expand Up @@ -20,6 +20,10 @@
// directory within the RAIDFILE root for the backup store daemon
#define BOX_RAIDFILE_ROOT_BBSTORED "backup"

// default security level if SSLSecurityLevel is not specified: see
// https://github.com/boxbackup/boxbackup/wiki/WeakSSLCertificates
const int BOX_DEFAULT_SSL_SECURITY_LEVEL = 1;

// configuration file paths
#ifdef WIN32
// no default config file path, use these macros to call
Expand Down
2 changes: 2 additions & 0 deletions lib/common/Test.h
Expand Up @@ -23,13 +23,15 @@
#define BBACKUPQUERY "..\\..\\bin\\bbackupquery\\bbackupquery.exe"
#define BBSTOREACCOUNTS "..\\..\\bin\\bbstoreaccounts\\bbstoreaccounts.exe"
#define TEST_RETURN(actual, expected) TEST_EQUAL(expected, actual);
#define TEST_RETURN_COMMAND(actual, expected, command) TEST_EQUAL_LINE(expected, actual, command);
#else
#define BBACKUPCTL "../../bin/bbackupctl/bbackupctl"
#define BBACKUPD "../../bin/bbackupd/bbackupd"
#define BBSTORED "../../bin/bbstored/bbstored"
#define BBACKUPQUERY "../../bin/bbackupquery/bbackupquery"
#define BBSTOREACCOUNTS "../../bin/bbstoreaccounts/bbstoreaccounts"
#define TEST_RETURN(actual, expected) TEST_EQUAL((expected << 8), actual);
#define TEST_RETURN_COMMAND(actual, expected, command) TEST_EQUAL_LINE((expected << 8), actual, command);
#endif

extern int num_failures;
Expand Down
9 changes: 9 additions & 0 deletions lib/server/TLSContext.cpp
Expand Up @@ -14,6 +14,7 @@

#include "autogen_ConnectionException.h"
#include "autogen_ServerException.h"
#include "BoxPortsAndFiles.h"
#include "CryptoUtils.h"
#include "SSLLib.h"
#include "TLSContext.h"
Expand Down Expand Up @@ -84,6 +85,14 @@ void TLSContext::Initialise(bool AsServer, const char *CertificatesFile, const c
THROW_EXCEPTION(ServerException, TLSAllocationFailed)
}

#ifdef HAVE_SSL_CTX_SET_SECURITY_LEVEL
BOX_WARNING("This version of Box Backup overrides the system-wide SSLSecurityLevel for "
"backwards compatibility. Please upgrade as soon as possible. See "
"https://github.com/boxbackup/boxbackup/wiki/WeakSSLCertificates#workaround-2 "
"for details");
SSL_CTX_set_security_level(mpContext, BOX_DEFAULT_SSL_SECURITY_LEVEL);
#endif

// Setup our identity
if(::SSL_CTX_use_certificate_chain_file(mpContext, CertificatesFile) != 1)
{
Expand Down
97 changes: 97 additions & 0 deletions test/basicserver/testbasicserver.cpp
Expand Up @@ -449,6 +449,80 @@ void TestStreamReceive(TestProtocolClient &protocol, int value, bool uncertainst
TEST_THAT(count == (24273*3)); // over 64 k of data, definately
}

bool test_security_level(int cert_level)
{
int old_num_failures = num_failures;

// Context first
TLSContext context;
if(cert_level == 0)
{
context.Initialise(false /* client */,
"testfiles/clientCerts.pem",
"testfiles/clientPrivKey.pem",
"testfiles/clientTrustedCAs.pem");
}
else if(cert_level == 1)
{
context.Initialise(false /* client */,
"testfiles/seclevel2-sha1/ca/clients/1234567-cert.pem",
"testfiles/seclevel2-sha1/bbackupd/1234567-key.pem",
"testfiles/seclevel2-sha1/ca/roots/serverCA.pem");
}
else if(cert_level == 2)
{
context.Initialise(false /* client */,
"testfiles/seclevel2-sha256/ca/clients/1234567-cert.pem",
"testfiles/seclevel2-sha256/bbackupd/1234567-key.pem",
"testfiles/seclevel2-sha256/ca/roots/serverCA.pem");
}
else
{
TEST_FAIL_WITH_MESSAGE("No certificates generated for level " << cert_level);
return false;
}

SocketStreamTLS conn;
conn.Open(context, Socket::TypeINET, "localhost", 2003);

return (num_failures == old_num_failures); // no new failures -> good
}

// Test the certificates that were distributed with the Box Backup source since ancient times,
// which have only 1024-bit keys, and thus fail with "ee key too small".
bool test_ancient_certificates()
{
int old_num_failures = num_failures;

// Level -1 (allow weaker, with warning) should pass with any certificates:
TEST_THAT(test_security_level(0)); // cert_level

return (num_failures == old_num_failures); // no new failures -> good
}

// Test a set of more recent certificates, which have a longer key but are signed using the SHA1
// algorithm instead of SHA256, which fail with "ca md too weak" instead.
bool test_old_certificates()
{
int old_num_failures = num_failures;

// Level -1 (allow weaker, with warning) should pass with any certificates:
TEST_THAT(test_security_level(1)); // cert_level

return (num_failures == old_num_failures); // no new failures -> good
}


bool test_new_certificates()
{
int old_num_failures = num_failures;

// Level -1 (allow weaker, with warning) should pass with any certificates:
TEST_THAT(test_security_level(2)); // cert_level

return (num_failures == old_num_failures); // no new failures -> good
}


int test(int argc, const char *argv[])
{
Expand Down Expand Up @@ -682,6 +756,11 @@ int test(int argc, const char *argv[])
TEST_THAT(ServerIsAlive(pid));
#endif

// Try testing with different security levels, check that the behaviour is
// as documented at:
// https://github.com/boxbackup/boxbackup/wiki/WeakSSLCertificates
TEST_THAT(test_ancient_certificates());

// Kill it
TEST_THAT(KillServer(pid));
::sleep(1);
Expand All @@ -691,6 +770,24 @@ int test(int argc, const char *argv[])
TestRemoteProcessMemLeaks("test-srv3.memleaks");
#endif
}

cmd = TEST_EXECUTABLE " --test-daemon-args=";
cmd += test_args;
cmd += " srv3 testfiles/srv3-seclevel2-sha1.conf";
pid = LaunchServer(cmd, "testfiles/srv3.pid");

TEST_THAT(pid != -1 && pid != 0);
TEST_THAT(test_old_certificates());
TEST_THAT(KillServer(pid));

cmd = TEST_EXECUTABLE " --test-daemon-args=";
cmd += test_args;
cmd += " srv3 testfiles/srv3-seclevel2-sha256.conf";
pid = LaunchServer(cmd, "testfiles/srv3.pid");

TEST_THAT(pid != -1 && pid != 0);
TEST_THAT(test_new_certificates());
TEST_THAT(KillServer(pid));
}

//protocolserver:
Expand Down

0 comments on commit 27ad0d8

Please sign in to comment.