Merge pull request #137 from skalenetwork/bug/SKALE-2977-sgx-crash

Bug/skale 2977 sgx crash

BLSCrypto.cpp

@@ -181,6 +181,7 @@ bool sign_aes(const char *_encryptedKeyHex, const char *_hashHex, size_t _t, siz
 
     if (yStr == nullptr) {
         std::cerr << "Null yStr" << std::endl;
+        delete xStr;
         BOOST_THROW_EXCEPTION(runtime_error("Null yStr"));
     }
 
@@ -197,6 +198,9 @@ bool sign_aes(const char *_encryptedKeyHex, const char *_hashHex, size_t _t, siz
     strncpy(xStrArg, xStr->c_str(), BUF_LEN);
     strncpy(yStrArg, yStr->c_str(), BUF_LEN);
 
+    delete xStr;
+    delete yStr;
+
     size_t sz = 0;
 
     uint8_t encryptedKey[BUF_LEN];
@@ -233,9 +237,6 @@ bool sign_aes(const char *_encryptedKeyHex, const char *_hashHex, size_t _t, siz
 
     strncpy(_sig, sig.c_str(), BUF_LEN);
 
-    delete xStr;
-    delete yStr;
-
     return true;
 }
 

BLSPrivateKeyShareSGX.cpp

@@ -117,6 +117,7 @@ std::string BLSPrivateKeyShareSGX::signWithHelperSGXstr(
 
   if (yStr == nullptr) {
     std::cerr << "Null yStr" << std::endl;
+    delete xStr;
     BOOST_THROW_EXCEPTION(runtime_error("Null yStr"));
   }
 
@@ -133,6 +134,9 @@ std::string BLSPrivateKeyShareSGX::signWithHelperSGXstr(
   strncpy(xStrArg, xStr->c_str(), BUF_LEN);
   strncpy(yStrArg, yStr->c_str(), BUF_LEN);
 
+  delete xStr;
+  delete yStr;
+
   size_t sz = 0;
 
   uint8_t encryptedKey[BUF_LEN];
@@ -159,7 +163,6 @@ std::string BLSPrivateKeyShareSGX::signWithHelperSGXstr(
 
   if (errStatus != 0) {
     BOOST_THROW_EXCEPTION(runtime_error("Enclave trustedBlsSignMessage failed:" + to_string(errStatus) + ":" + errMsg ));
-    return nullptr;
   }
 
   int sigLen;
@@ -176,9 +179,6 @@ std::string BLSPrivateKeyShareSGX::signWithHelperSGXstr(
   sig.append(":");
   sig.append(hint);
 
-  delete xStr;
-  delete yStr;
-
   return sig;
 }
 

ECDSACrypto.cpp

@@ -85,7 +85,7 @@ vector <string> genECDSAKey() {
     return keys;
 }
 
-string getECDSAPubKey(const char *_encryptedKeyHex) {
+string getECDSAPubKey(const std::string& _encryptedKeyHex) {
     vector<char> errMsg(BUF_LEN, 0);
     vector<char> pubKeyX(BUF_LEN, 0);
     vector<char> pubKeyY(BUF_LEN, 0);
@@ -94,7 +94,7 @@ string getECDSAPubKey(const char *_encryptedKeyHex) {
     int errStatus = 0;
     uint64_t enc_len = 0;
 
-    if (!hex2carray(_encryptedKeyHex, &enc_len, encrPrKey.data())) {
+    if (!hex2carray(_encryptedKeyHex.c_str(), &enc_len, encrPrKey.data())) {
         throw SGXException(INVALID_HEX, "Invalid encryptedKeyHex");
     }
 
@@ -122,47 +122,49 @@ string getECDSAPubKey(const char *_encryptedKeyHex) {
 
 bool verifyECDSASig(string& pubKeyStr, const char *hashHex, const char *signatureR,
         const char *signatureS, int base) {
-    bool result = false;
-
-    signature sig = signature_init();
-
     auto x = pubKeyStr.substr(0, 64);
     auto y = pubKeyStr.substr(64, 128);
-    domain_parameters curve = domain_parameters_init();
-    domain_parameters_load_curve(curve, secp256k1);
-    point publicKey = point_init();
 
     mpz_t msgMpz;
     mpz_init(msgMpz);
     if (mpz_set_str(msgMpz, hashHex, 16) == -1) {
         spdlog::error("invalid message hash {}", hashHex);
-        goto clean;
+        mpz_clear(msgMpz);
+        return false;
     }
 
+    signature sig = signature_init();
     if (signature_set_str(sig, signatureR, signatureS, base) != 0) {
         spdlog::error("Failed to set str signature");
-        goto clean;
+        mpz_clear(msgMpz);
+        signature_free(sig);
+        return false;
     }
 
+    domain_parameters curve = domain_parameters_init();
+    domain_parameters_load_curve(curve, secp256k1);
+
+    point publicKey = point_init();
+
     point_set_hex(publicKey, x.c_str(), y.c_str());
     if (!signature_verify(msgMpz, sig, publicKey, curve)) {
         spdlog::error("ECDSA sig not verified");
-        goto clean;
+        mpz_clear(msgMpz);
+        signature_free(sig);
+        domain_parameters_clear(curve);
+        point_clear(publicKey);
+        return false;
     }
 
-    result = true;
-
-    clean:
-
     mpz_clear(msgMpz);
+    signature_free(sig);
     domain_parameters_clear(curve);
     point_clear(publicKey);
-    signature_free(sig);
 
-    return result;
+    return true;
 }
 
-vector <string> ecdsaSignHash(const char *encryptedKeyHex, const char *hashHex, int base) {
+vector <string> ecdsaSignHash(const std::string& encryptedKeyHex, const char *hashHex, int base) {
     vector <string> signatureVector(3);
 
     vector<char> errMsg(1024, 0);
@@ -175,21 +177,22 @@ vector <string> ecdsaSignHash(const char *encryptedKeyHex, const char *hashHex,
 
     string pubKeyStr = "";
 
-    if (!hex2carray(encryptedKeyHex, &decLen, encryptedKey.data())) {
+    if (!hex2carray(encryptedKeyHex.c_str(), &decLen, encryptedKey.data())) {
         throw SGXException(INVALID_HEX, "Invalid encryptedKeyHex");
     }
 
     status = trustedEcdsaSignAES(eid, &errStatus,
-            errMsg.data(), encryptedKey.data(), decLen, (unsigned char *) hashHex,
+            errMsg.data(), encryptedKey.data(), decLen, hashHex,
                                  signatureR.data(),
                                  signatureS.data(), &signatureV, base);
 
     if (errStatus != 0) {
+        spdlog::error("failed to sign {}", errStatus);
         throw SGXException(666, errMsg.data());
     }
 
     if (status != SGX_SUCCESS) {
-        spdlog::error("failed to sign {}", status);
+        spdlog::error("failed to sign in enclave {}", status);
         throw SGXException(666, "failed to sign");
     }
 
@@ -213,6 +216,7 @@ vector <string> ecdsaSignHash(const char *encryptedKeyHex, const char *hashHex,
     if (i % 1000 == 0) {
 
         if (!verifyECDSASig(pubKeyStr, hashHex, signatureR.data(), signatureS.data(), base)) {
+            spdlog::error("failed to verify ecdsa signature");
             throw SGXException(667, "ECDSA did not verify");
         }
     }

ECDSACrypto.h

@@ -31,9 +31,9 @@ using namespace std;
 
 vector<string> genECDSAKey();
 
-string getECDSAPubKey(const char* _encryptedKeyHex);
+string getECDSAPubKey(const std::string& _encryptedKeyHex);
 
-vector<string> ecdsaSignHash(const char* encryptedKeyHex, const char* hashHex, int base);
+vector<string> ecdsaSignHash(const std::string& encryptedKeyHex, const char* hashHex, int base);
 
 
 #endif //SGXD_ECDSACRYPTO_H

ECDSAImpl.c

@@ -2,4 +2,4 @@
 #include "secure_enclave/DomainParameters.c"
 #include "secure_enclave/NumberTheory.c"
 #include "secure_enclave/Signature.c"
-#include "secure_enclave/Curves.c"
+#include "secure_enclave/Curves.c"

LevelDB.cpp

@@ -21,13 +21,11 @@
     @date 2019
 */
 
-
 #include <stdexcept>
 #include <memory>
 #include <string>
 #include <iostream>
 
-
 #include "leveldb/db.h"
 
 #include "sgxwallet_common.h"
@@ -53,9 +51,9 @@ std::shared_ptr<string> LevelDB::readString(const string &_key) {
         throw SGXException(NULL_DATABASE, "Null db");
     }
 
-    spdlog::debug("key to read from db: {}",_key );
+    spdlog::debug("key to read from db: {}", _key);
 
-    auto status = db->Get(readOptions, _key, &*result);
+    auto status = db->Get(readOptions, _key, result.get());
 
     throwExceptionOnError(status);
 
@@ -73,7 +71,7 @@ void LevelDB::writeString(const string &_key, const string &_value) {
 
     throwExceptionOnError(status);
 
-    spdlog::debug("written key: {}",_key );
+    spdlog::debug("written key: {}", _key);
 }
 
 
@@ -101,7 +99,7 @@ void LevelDB::deleteTempNEK(const string &_key) {
 
     throwExceptionOnError(status);
 
-    std::cerr << "key deleted " << _key << std::endl;
+    spdlog::debug("key deleted: {}", _key);
 }
 
 void LevelDB::deleteKey(const string &_key) {
@@ -111,7 +109,7 @@ void LevelDB::deleteKey(const string &_key) {
 
     throwExceptionOnError(status);
 
-    spdlog::debug("key deleted: {}",_key );
+    spdlog::debug("key deleted: {}", _key);
 }
 
 

Makefile.am

@@ -54,8 +54,7 @@ AM_CPPFLAGS += -DSGXWALLET_VERSION="$(WALLET_VERSION)" -Wall -DSKALE_SGX=1 -DBIN
 ## Additional targets to remove with 'make clean'. You must list
 ## any edger8r generated files here.
 
-CLEANFILES = $(COMMON_ENCLAVE_SRC) secure_enclave.edl \
-	secure_enclave.signed.so
+CLEANFILES = $(COMMON_ENCLAVE_SRC) secure_enclave.edl secure_enclave.signed.so
 
 
 ## The build target
@@ -67,7 +66,7 @@ bin_PROGRAMS =  sgxwallet testw cert_util
 ## have to be explicitly listed.
 
 COMMON_SRC = InvalidStateException.cpp  Exception.cpp InvalidArgumentException.cpp Log.cpp  \
-             SGXWalletServer.cpp SGXRegistrationServer.cpp CSRManagerServer.cpp RPCException.cpp  BLSCrypto.cpp \
+             SGXWalletServer.cpp SGXRegistrationServer.cpp CSRManagerServer.cpp BLSCrypto.cpp \
              ECDSACrypto.cpp \
              DKGCrypto.cpp ServerInit.cpp BLSPrivateKeyShareSGX.cpp LevelDB.cpp ServerDataChecker.cpp SEKManager.cpp \
              third_party/intel/sgx_stub.c third_party/intel/sgx_detect_linux.c third_party/intel/create_enclave.c third_party/intel/oc_alloc.c \
@@ -113,7 +112,7 @@ nodist_testw_SOURCES=${nodist_sgxwallet_SOURCES}
 EXTRA_testw_DEPENDENCIES=${EXTRA_sgxwallet_DEPENDENCIES}
 testw_LDADD= ${sgxwallet_LDADD}
 
-cert_util_SOURCES= InvalidStateException.cpp  Exception.cpp InvalidArgumentException.cpp Log.cpp cert_util.cpp stubclient.cpp RPCException.cpp LevelDB.cpp SGXRegistrationServer.cpp CSRManagerServer.cpp
+cert_util_SOURCES= InvalidStateException.cpp  Exception.cpp InvalidArgumentException.cpp Log.cpp cert_util.cpp stubclient.cpp LevelDB.cpp SGXRegistrationServer.cpp CSRManagerServer.cpp
 cert_util_LDADD=-LlibBLS/deps/deps_inst/x86_or_x64/lib -Lleveldb/build -LlibBLS/build \
                    -LlibBLS/build/libff/libff \
                    -l:libbls.a -l:libleveldb.a \

SGXException.h

@@ -24,7 +24,6 @@
 #ifndef SGXD_RPCEXCEPTION_H
 #define SGXD_RPCEXCEPTION_H
 
-
 #include <string>
 #include <exception>
 
@@ -39,5 +38,4 @@ class SGXException : public std::exception {
 
 };
 
-
 #endif //SGXD_RPCEXCEPTION_H