From b30d3fcc12ad48145cbb6f0e592e2fdba4379507 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Wed, 20 Dec 2017 15:40:10 -0800 Subject: [PATCH 1/3] Change three lines of code. Fix one thing. Break another. --- libraries/entities/src/EntityItemProperties.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 678ddfcea5..22e7ce5257 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -2530,7 +2530,8 @@ bool EntityItemProperties::verifySignature(const QString& publicKey, const QByte return false; } - const unsigned char* key = reinterpret_cast(publicKey.toUtf8().constData()); + auto keyByteArray = publicKey.toUtf8(); + auto key = keyByteArray.constData(); int keyLength = publicKey.length(); BIO *bio = BIO_new_mem_buf((void*)key, keyLength); @@ -2548,14 +2549,14 @@ bool EntityItemProperties::verifySignature(const QString& publicKey, const QByte // ECSDA verification prototype: note that type is currently ignored // int ECDSA_verify(int type, const unsigned char *dgst, int dgstlen, // const unsigned char *sig, int siglen, EC_KEY *eckey); - bool answer = ECDSA_verify(0, + int answer = ECDSA_verify(0, digest, digestLength, signature, signatureLength, ec); long error = ERR_get_error(); - if (error != 0) { + if (error != 0 || answer == -1) { const char* error_str = ERR_error_string(error, NULL); qCWarning(entities) << "ERROR while verifying signature! EC error:" << error_str << "\nKey:" << publicKey << "\nutf8 Key Length:" << keyLength @@ -2569,7 +2570,7 @@ bool EntityItemProperties::verifySignature(const QString& publicKey, const QByte if (evp_key) { EVP_PKEY_free(evp_key); } - return answer; + return (answer == 1); } else { if (bio) { BIO_free(bio); From e96367340971a3c1411664d74fa75006e4055162 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Thu, 21 Dec 2017 13:48:35 -0800 Subject: [PATCH 2/3] Progress? --- interface/src/commerce/Ledger.cpp | 2 +- interface/src/commerce/Wallet.cpp | 9 +++++---- libraries/entities/src/EntityItemProperties.cpp | 8 ++++++-- libraries/entities/src/EntityTree.cpp | 7 ++++--- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/interface/src/commerce/Ledger.cpp b/interface/src/commerce/Ledger.cpp index 3257a634c7..59d274158b 100644 --- a/interface/src/commerce/Ledger.cpp +++ b/interface/src/commerce/Ledger.cpp @@ -61,7 +61,7 @@ void Ledger::send(const QString& endpoint, const QString& success, const QString void Ledger::signedSend(const QString& propertyName, const QByteArray& text, const QString& key, const QString& endpoint, const QString& success, const QString& fail, const bool controlled_failure) { auto wallet = DependencyManager::get(); - QString signature = key.isEmpty() ? "" : wallet->signWithKey(text, key); + QString signature = wallet->signWithKey(text, key); QJsonObject request; request[propertyName] = QString(text); if (!controlled_failure) { diff --git a/interface/src/commerce/Wallet.cpp b/interface/src/commerce/Wallet.cpp index 00941d6c50..04a14e2a54 100644 --- a/interface/src/commerce/Wallet.cpp +++ b/interface/src/commerce/Wallet.cpp @@ -548,13 +548,16 @@ QStringList Wallet::listPublicKeys() { // the horror of code pages and so on (changing the bytes) by just returning a base64 // encoded string representing the signature (suitable for http, etc...) QString Wallet::signWithKey(const QByteArray& text, const QString& key) { - qCInfo(commerce) << "Signing text" << text << "with key" << key; EC_KEY* ecPrivateKey = NULL; + + auto keyFilePathString = keyFilePath().toStdString(); if ((ecPrivateKey = readPrivateKey(keyFilePath().toStdString().c_str()))) { unsigned char* sig = new unsigned char[ECDSA_size(ecPrivateKey)]; unsigned int signatureBytes = 0; + qCInfo(commerce) << "Signing text" << text << "with key at" << ecPrivateKey; + QByteArray hashedPlaintext = QCryptographicHash::hash(text, QCryptographicHash::Sha256); @@ -747,12 +750,10 @@ void Wallet::handleChallengeOwnershipPacket(QSharedPointer pack } EC_KEY_free(ec); - QByteArray ba = sig.toLocal8Bit(); - const char *sigChar = ba.data(); QByteArray textByteArray; if (status > -1) { - textByteArray = QByteArray(sigChar, (int) strlen(sigChar)); + textByteArray = sig.toUtf8(); } textByteArraySize = textByteArray.size(); int certIDSize = certID.size(); diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 22e7ce5257..13ebd9ef9f 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -2557,11 +2557,15 @@ bool EntityItemProperties::verifySignature(const QString& publicKey, const QByte ec); long error = ERR_get_error(); if (error != 0 || answer == -1) { - const char* error_str = ERR_error_string(error, NULL); - qCWarning(entities) << "ERROR while verifying signature! EC error:" << error_str + qCWarning(entities) << "ERROR while verifying signature!" << "\nKey:" << publicKey << "\nutf8 Key Length:" << keyLength << "\nDigest:" << digest << "\nDigest Length:" << digestLength << "\nSignature:" << signature << "\nSignature Length:" << signatureLength; + while (error != 0) { + const char* error_str = ERR_error_string(error, NULL); + qCWarning(entities) << "EC error:" << error_str; + error = ERR_get_error(); + } } EC_KEY_free(ec); if (bio) { diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index b5765bb44b..6aaddfaa57 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1189,13 +1189,14 @@ bool EntityTree::verifyNonce(const QString& certID, const QString& nonce, Entity key = sent.second; } - QString annotatedKey = "-----BEGIN PUBLIC KEY-----\n" + key.insert(64, "\n") + "\n-----END PUBLIC KEY-----"; - bool verificationSuccess = EntityItemProperties::verifySignature(annotatedKey.toUtf8(), actualNonce.toUtf8(), nonce.toUtf8()); + QString annotatedKey = "-----BEGIN PUBLIC KEY-----\n" + key.insert(64, "\n") + "\n-----END PUBLIC KEY-----\n"; + QByteArray hashedActualNonce = QCryptographicHash::hash(QByteArray::fromBase64(actualNonce.toUtf8()), QCryptographicHash::Sha256); + bool verificationSuccess = EntityItemProperties::verifySignature(annotatedKey.toUtf8(), hashedActualNonce, QByteArray::fromBase64(nonce.toUtf8())); if (verificationSuccess) { qCDebug(entities) << "Ownership challenge for Cert ID" << certID << "succeeded."; } else { - qCDebug(entities) << "Ownership challenge for Cert ID" << certID << "failed for nonce" << actualNonce << "key" << key << "signature" << nonce; + qCDebug(entities) << "Ownership challenge for Cert ID" << certID << "failed.\nHashed actual nonce (digest):" << hashedActualNonce << "\nSent nonce (signature)" << nonce << "\nKey" << key; } return verificationSuccess; From 73eb258e794062255f31a26e8ac1118c116c8020 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Thu, 21 Dec 2017 15:01:49 -0800 Subject: [PATCH 3/3] IT'S WORKING!!! --- interface/src/commerce/Ledger.cpp | 2 +- interface/src/commerce/Wallet.cpp | 2 +- libraries/entities/src/EntityTree.cpp | 5 +++-- libraries/networking/src/udt/PacketHeaders.cpp | 2 +- libraries/networking/src/udt/PacketHeaders.h | 3 ++- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/interface/src/commerce/Ledger.cpp b/interface/src/commerce/Ledger.cpp index 59d274158b..d7d36dabf6 100644 --- a/interface/src/commerce/Ledger.cpp +++ b/interface/src/commerce/Ledger.cpp @@ -61,7 +61,7 @@ void Ledger::send(const QString& endpoint, const QString& success, const QString void Ledger::signedSend(const QString& propertyName, const QByteArray& text, const QString& key, const QString& endpoint, const QString& success, const QString& fail, const bool controlled_failure) { auto wallet = DependencyManager::get(); - QString signature = wallet->signWithKey(text, key); + QString signature = wallet->signWithKey(text, key); QJsonObject request; request[propertyName] = QString(text); if (!controlled_failure) { diff --git a/interface/src/commerce/Wallet.cpp b/interface/src/commerce/Wallet.cpp index 04a14e2a54..c3c91e82a8 100644 --- a/interface/src/commerce/Wallet.cpp +++ b/interface/src/commerce/Wallet.cpp @@ -556,7 +556,7 @@ QString Wallet::signWithKey(const QByteArray& text, const QString& key) { unsigned int signatureBytes = 0; - qCInfo(commerce) << "Signing text" << text << "with key at" << ecPrivateKey; + qCInfo(commerce) << "Hashing and signing plaintext" << text << "with key at address" << ecPrivateKey; QByteArray hashedPlaintext = QCryptographicHash::hash(text, QCryptographicHash::Sha256); diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 6aaddfaa57..8f780355db 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1190,13 +1190,14 @@ bool EntityTree::verifyNonce(const QString& certID, const QString& nonce, Entity } QString annotatedKey = "-----BEGIN PUBLIC KEY-----\n" + key.insert(64, "\n") + "\n-----END PUBLIC KEY-----\n"; - QByteArray hashedActualNonce = QCryptographicHash::hash(QByteArray::fromBase64(actualNonce.toUtf8()), QCryptographicHash::Sha256); + QByteArray hashedActualNonce = QCryptographicHash::hash(QByteArray(actualNonce.toUtf8()), QCryptographicHash::Sha256); bool verificationSuccess = EntityItemProperties::verifySignature(annotatedKey.toUtf8(), hashedActualNonce, QByteArray::fromBase64(nonce.toUtf8())); if (verificationSuccess) { qCDebug(entities) << "Ownership challenge for Cert ID" << certID << "succeeded."; } else { - qCDebug(entities) << "Ownership challenge for Cert ID" << certID << "failed.\nHashed actual nonce (digest):" << hashedActualNonce << "\nSent nonce (signature)" << nonce << "\nKey" << key; + qCDebug(entities) << "Ownership challenge for Cert ID" << certID << "failed. Actual nonce:" << actualNonce << + "\nHashed actual nonce (digest):" << hashedActualNonce << "\nSent nonce (signature)" << nonce << "\nKey" << key; } return verificationSuccess; diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index 62354da11a..0a75e8c31b 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -30,7 +30,7 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::EntityEdit: case PacketType::EntityData: case PacketType::EntityPhysics: - return static_cast(EntityVersion::StaticCertJsonVersionOne); + return static_cast(EntityVersion::OwnershipChallengeFix); case PacketType::EntityQuery: return static_cast(EntityQueryPacketVersion::ConnectionIdentifier); diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 618ac2de0c..05d40f9621 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -200,7 +200,8 @@ enum class EntityVersion : PacketVersion { StrokeColorProperty = 77, HasDynamicOwnershipTests, HazeEffect, - StaticCertJsonVersionOne + StaticCertJsonVersionOne, + OwnershipChallengeFix, }; enum class EntityScriptCallMethodVersion : PacketVersion {