Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
26649: libroach: make CryptoPP build with runtime AES-NI detection. r=mberhault a=mberhault

Fixes cockroachdb#26383

Set appropriate `-m` flags for cryptopp to enable AES runtime detection
checks (without those, it does not even try).

Add a `UsesAESNI()` function in CryptoPP which returns true iff:
* AES-NI runtime detection is compiled in
* AES-NI instruction is available

Add a warning to stdout (normal logging requires `vmodule=rocksdb=3`) if
encryption is requested but AES-NI is not available.

Add a test to make sure our builds always have AES-NI enabled.

Release note (core): build release binaries with runtime AES detection.

Co-authored-by: marc <[email protected]>
  • Loading branch information
craig[bot] and marc committed Jun 14, 2018
2 parents f4faa15 + 6881f30 commit b497dd0
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 2 deletions.
7 changes: 6 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -558,12 +558,17 @@ $(CGO_FLAGS_FILES): Makefile
# only rebuild the affected objects, but in practice dependencies on configure
# flags are not tracked correctly, and these stale artifacts can cause
# particularly hard-to-debug errors.
#
# Flags needed to make cryptopp to runtime detection of AES cpu instruction sets.
# pclmul and ssse3 need to be defined for the overall AES switch but are only used
# in GCM mode (not currently in use by cockroach).
AES_FLAGS := -maes -mpclmul -mssse3
$(CRYPTOPP_DIR)/Makefile: $(C_DEPS_DIR)/cryptopp-rebuild | bin/.submodules-initialized
rm -rf $(CRYPTOPP_DIR)
mkdir -p $(CRYPTOPP_DIR)
@# NOTE: If you change the CMake flags below, bump the version in
@# $(C_DEPS_DIR)/cryptopp-rebuild. See above for rationale.
cd $(CRYPTOPP_DIR) && cmake $(XCMAKE_FLAGS) $(CRYPTOPP_SRC_DIR) \
cd $(CRYPTOPP_DIR) && CFLAGS+=" $(AES_FLAGS)" && CXXFLAGS+=" $(AES_FLAGS)" cmake $(XCMAKE_FLAGS) $(CRYPTOPP_SRC_DIR) \
-DCMAKE_BUILD_TYPE=Release

$(JEMALLOC_SRC_DIR)/configure.ac: | bin/.submodules-initialized
Expand Down
1 change: 1 addition & 0 deletions build/variables.mk
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
define VALID_VARS
.DEFAULT_GOAL
ACCEPTANCETIMEOUT
AES_FLAGS
ARCHIVE
ARCHIVE_BASE
ARCHIVE_EXTRAS
Expand Down
2 changes: 1 addition & 1 deletion c-deps/cryptopp
Submodule cryptopp updated 2 files
+4 −0 aes.h
+7 −0 rijndael.cpp
1 change: 1 addition & 0 deletions c-deps/libroach/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ set(tests
encoding_test.cc
file_registry_test.cc
merge_test.cc
ccl/crypto_utils_test.cc
ccl/db_test.cc
ccl/encrypted_env_test.cc
ccl/key_manager_test.cc
Expand Down
2 changes: 2 additions & 0 deletions c-deps/libroach/ccl/crypto_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,5 @@ class AESEncryptCipher : public rocksdb_utils::BlockCipher {
rocksdb_utils::BlockCipher* NewAESEncryptCipher(const enginepbccl::SecretKey* key) {
return new AESEncryptCipher(key->key());
}

bool UsesAESNI() { return CryptoPP::UsesAESNI(); }
3 changes: 3 additions & 0 deletions c-deps/libroach/ccl/crypto_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,6 @@ std::string RandomBytes(size_t length);
// Create a new AES cipher using the passed-in key.
// Suitable for encryption only, Decrypt is not implemented.
rocksdb_utils::BlockCipher* NewAESEncryptCipher(const enginepbccl::SecretKey* key);

// Returns true if CryptoPP is using AES-NI.
bool UsesAESNI();
12 changes: 12 additions & 0 deletions c-deps/libroach/ccl/crypto_utils_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright 2017 The Cockroach Authors.
//
// Licensed as a CockroachDB Enterprise file under the Cockroach Community
// License (the "License"); you may not use this file except in compliance with
// the License. You may obtain a copy of the License at
//
// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt

#include "../testutils.h"
#include "crypto_utils.h"

TEST(CryptoUtils, HasAESNI) { EXPECT_TRUE(UsesAESNI()); }
11 changes: 11 additions & 0 deletions c-deps/libroach/ccl/db.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "../status.h"
#include "ccl/baseccl/encryption_options.pb.h"
#include "ccl/storageccl/engineccl/enginepbccl/stats.pb.h"
#include "crypto_utils.h"
#include "ctr_stream.h"
#include "key_manager.h"

Expand Down Expand Up @@ -78,6 +79,16 @@ rocksdb::Status DBOpenHook(std::shared_ptr<rocksdb::Logger> info_log, const std:
return rocksdb::Status::OK();
}

// We have encryption options. Check whether the AES instruction set is supported.
if (!UsesAESNI()) {
// Shout loudly on standard out.
std::cout << std::endl
<< "*** WARNING ***" << std::endl
<< "Encryption requested, but no AES instruction set detected" << std::endl
<< "Expect significant performance degradation!" << std::endl
<< std::endl;
}

// The Go code sets the "file_registry" storage version if we specified encryption flags,
// but let's double check anyway.
if (!db_opts.use_file_registry) {
Expand Down

0 comments on commit b497dd0

Please sign in to comment.