From 5a53d7cda37677f393cddcc483523eb2cbfaf88d Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Fri, 12 Dec 2014 10:15:34 +0100 Subject: [PATCH 1/7] [Qt] paymentserver: do not log NULL certificates - also add a few more comments in PaymentServer::LoadRootCAs --- src/qt/paymentserver.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index ad489de34..f05bfc059 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -143,13 +143,20 @@ void PaymentServer::LoadRootCAs(X509_STORE* _store) int nRootCerts = 0; const QDateTime currentTime = QDateTime::currentDateTime(); - foreach (const QSslCertificate& cert, certList) - { + + foreach (const QSslCertificate& cert, certList) { + // Don't log NULL certificates + if (cert.isNull()) + continue; + + // Not yet active/valid, or expired certificate if (currentTime < cert.effectiveDate() || currentTime > cert.expiryDate()) { ReportInvalidCertificate(cert); continue; } + #if QT_VERSION >= 0x050000 + // Blacklisted certificate if (cert.isBlacklisted()) { ReportInvalidCertificate(cert); continue; From 6e17a747661bc1c737b10f77d1df87d37e49fbb8 Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Fri, 12 Dec 2014 10:17:56 +0100 Subject: [PATCH 2/7] [Qt] paymentserver: better logging of invalid certs Before and after was tested in Windows: before: GUI: ReportInvalidCertificate : Payment server found an invalid certificate: ("Microsoft Authenticode(tm) Root Authority") GUI: ReportInvalidCertificate : Payment server found an invalid certificate: () GUI: ReportInvalidCertificate : Payment server found an invalid certificate: () GUI: ReportInvalidCertificate : Payment server found an invalid certificate: () after: GUI: ReportInvalidCertificate: Payment server found an invalid certificate: "01" ("Microsoft Authenticode(tm) Root Authority") () () GUI: ReportInvalidCertificate: Payment server found an invalid certificate: "01" () () ("Copyright (c) 1997 Microsoft Corp.", "Microsoft Time Stamping Service Root", "Microsoft Corporation") GUI: ReportInvalidCertificate: Payment server found an invalid certificate: "4a:19:d2:38:8c:82:59:1c:a5:5d:73:5f:15:5d:dc:a3" () () ("NO LIABILITY ACCEPTED, (c)97 VeriSign, Inc.", "VeriSign Time Stamping Service Root", "VeriSign, Inc.") GUI: ReportInvalidCertificate: Payment server found an invalid certificate: "e4:9e:fd:f3:3a:e8:0e:cf:a5:11:3e:19:a4:24:02:32" () () ("Class 3 Public Primary Certification Authority") --- src/qt/paymentserver.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index f05bfc059..1d35de6f0 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -97,7 +97,11 @@ static QList savedPaymentRequests; static void ReportInvalidCertificate(const QSslCertificate& cert) { - qDebug() << "ReportInvalidCertificate: Payment server found an invalid certificate: " << cert.subjectInfo(QSslCertificate::CommonName); +#if QT_VERSION < 0x050000 + qDebug() << QString("%1: Payment server found an invalid certificate: ").arg(__func__) << cert.serialNumber() << cert.subjectInfo(QSslCertificate::CommonName) << cert.subjectInfo(QSslCertificate::OrganizationalUnitName); +#else + qDebug() << QString("%1: Payment server found an invalid certificate: ").arg(__func__) << cert.serialNumber() << cert.subjectInfo(QSslCertificate::CommonName) << cert.subjectInfo(QSslCertificate::DistinguishedNameQualifier) << cert.subjectInfo(QSslCertificate::OrganizationalUnitName); +#endif } // From d19ae3cf665de269c5dc4e2d9ed5ebe954b88be8 Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Fri, 12 Dec 2014 10:21:07 +0100 Subject: [PATCH 3/7] [Qt] remove unused PaymentRequestPlus::getPKIType function --- src/qt/paymentrequestplus.cpp | 6 ------ src/qt/paymentrequestplus.h | 1 - 2 files changed, 7 deletions(-) diff --git a/src/qt/paymentrequestplus.cpp b/src/qt/paymentrequestplus.cpp index b69461ad9..de222fe14 100644 --- a/src/qt/paymentrequestplus.cpp +++ b/src/qt/paymentrequestplus.cpp @@ -59,12 +59,6 @@ bool PaymentRequestPlus::IsInitialized() const return paymentRequest.IsInitialized(); } -QString PaymentRequestPlus::getPKIType() const -{ - if (!IsInitialized()) return QString("none"); - return QString::fromStdString(paymentRequest.pki_type()); -} - bool PaymentRequestPlus::getMerchant(X509_STORE* certStore, QString& merchant) const { merchant.clear(); diff --git a/src/qt/paymentrequestplus.h b/src/qt/paymentrequestplus.h index 61f8a3415..99a7186b8 100644 --- a/src/qt/paymentrequestplus.h +++ b/src/qt/paymentrequestplus.h @@ -29,7 +29,6 @@ public: bool SerializeToString(std::string* output) const; bool IsInitialized() const; - QString getPKIType() const; // Returns true if merchant's identity is authenticated, and // returns human-readable merchant identity in merchant bool getMerchant(X509_STORE* certStore, QString& merchant) const; From 9b14aefee379345a051f2d4cdd92fe8cd52a658d Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Fri, 12 Dec 2014 16:14:44 +0100 Subject: [PATCH 4/7] [Qt] take care of a missing typecast in PaymentRequestPlus::getMerchant() --- src/qt/paymentrequestplus.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/qt/paymentrequestplus.cpp b/src/qt/paymentrequestplus.cpp index de222fe14..7e9729eeb 100644 --- a/src/qt/paymentrequestplus.cpp +++ b/src/qt/paymentrequestplus.cpp @@ -118,7 +118,7 @@ bool PaymentRequestPlus::getMerchant(X509_STORE* certStore, QString& merchant) c // The first cert is the signing cert, the rest are untrusted certs that chain // to a valid root authority. OpenSSL needs them separately. STACK_OF(X509) *chain = sk_X509_new_null(); - for (int i = certs.size()-1; i > 0; i--) { + for (int i = certs.size() - 1; i > 0; i--) { sk_X509_push(chain, certs[i]); } X509 *signing_cert = certs[0]; @@ -166,9 +166,8 @@ bool PaymentRequestPlus::getMerchant(X509_STORE* certStore, QString& merchant) c EVP_MD_CTX_init(&ctx); if (!EVP_VerifyInit_ex(&ctx, digestAlgorithm, NULL) || !EVP_VerifyUpdate(&ctx, data_to_verify.data(), data_to_verify.size()) || - !EVP_VerifyFinal(&ctx, (const unsigned char*)paymentRequest.signature().data(), paymentRequest.signature().size(), pubkey)) { - - throw SSLVerifyError("Bad signature, invalid PaymentRequest."); + !EVP_VerifyFinal(&ctx, (const unsigned char*)paymentRequest.signature().data(), (unsigned int)paymentRequest.signature().size(), pubkey)) { + throw SSLVerifyError("Bad signature, invalid payment request."); } // OpenSSL API for getting human printable strings from certs is baroque. From 35d15959b0b6eb58e87b351d8f58952fa8e90103 Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Mon, 12 Jan 2015 14:24:01 +0100 Subject: [PATCH 5/7] [Qt] constify first parameter of processPaymentRequest() --- src/qt/paymentserver.cpp | 2 +- src/qt/paymentserver.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index 1d35de6f0..e7c2f6a08 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -526,7 +526,7 @@ bool PaymentServer::readPaymentRequestFromFile(const QString& filename, PaymentR return request.parse(data); } -bool PaymentServer::processPaymentRequest(PaymentRequestPlus& request, SendCoinsRecipient& recipient) +bool PaymentServer::processPaymentRequest(const PaymentRequestPlus& request, SendCoinsRecipient& recipient) { if (!optionsModel) return false; diff --git a/src/qt/paymentserver.h b/src/qt/paymentserver.h index 6bf5ac2ee..32ed27983 100644 --- a/src/qt/paymentserver.h +++ b/src/qt/paymentserver.h @@ -131,7 +131,7 @@ protected: bool eventFilter(QObject *object, QEvent *event); private: - bool processPaymentRequest(PaymentRequestPlus& request, SendCoinsRecipient& recipient); + bool processPaymentRequest(const PaymentRequestPlus& request, SendCoinsRecipient& recipient); void fetchRequest(const QUrl& url); // Setup networking From 06087bda87214e3f4995a0de65633511d4158d78 Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Mon, 12 Jan 2015 14:26:52 +0100 Subject: [PATCH 6/7] [Qt] minor comment updates in PaymentServer --- src/qt/paymentserver.cpp | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index e7c2f6a08..09e9949b1 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -312,7 +312,7 @@ PaymentServer::PaymentServer(QObject* parent, bool startLocalServer) : // Install global event filter to catch QFileOpenEvents // on Mac: sent when you click bitcoin: links - // other OSes: helpful when dealing with payment request files (in the future) + // other OSes: helpful when dealing with payment request files if (parent) parent->installEventFilter(this); @@ -343,14 +343,13 @@ PaymentServer::~PaymentServer() } // -// OSX-specific way of handling bitcoin: URIs and -// PaymentRequest mime types +// OSX-specific way of handling bitcoin: URIs and PaymentRequest mime types. +// Also used by paymentservertests.cpp and when opening a payment request file +// via "Open URI..." menu entry. // bool PaymentServer::eventFilter(QObject *object, QEvent *event) { - // clicking on bitcoin: URIs creates FileOpen events on the Mac - if (event->type() == QEvent::FileOpen) - { + if (event->type() == QEvent::FileOpen) { QFileOpenEvent *fileEvent = static_cast(event); if (!fileEvent->file().isEmpty()) handleURIOrFile(fileEvent->file()); @@ -571,9 +570,9 @@ bool PaymentServer::processPaymentRequest(const PaymentRequestPlus& request, Sen addresses.append(QString::fromStdString(CBitcoinAddress(dest).ToString())); } else if (!recipient.authenticatedMerchant.isEmpty()) { - // Insecure payments to custom bitcoin addresses are not supported - // (there is no good way to tell the user where they are paying in a way - // they'd have a chance of understanding). + // Unauthenticated payment requests to custom bitcoin addresses are not supported + // (there is no good way to tell the user where they are paying in a way they'd + // have a chance of understanding). emit message(tr("Payment request rejected"), tr("Unverified payment requests to custom payment scripts are unsupported."), CClientUIInterface::MSG_ERROR); From 6171e494fcd38a4e9a2921e066c10720a74e0ddb Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Tue, 3 Feb 2015 22:44:33 +0100 Subject: [PATCH 7/7] [Qt] Use identical strings for expired payment request message - used in sendcoinsdialog.cpp and paymentserver.cpp - removes an unneded translation string --- src/qt/sendcoinsdialog.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 774667d4a..59939fa87 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -531,7 +531,7 @@ void SendCoinsDialog::processSendCoinsReturn(const WalletModel::SendCoinsReturn msgParams.first = tr("A fee higher than %1 is considered an absurdly high fee.").arg(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), 10000000)); break; case WalletModel::PaymentRequestExpired: - msgParams.first = tr("Payment request expired!"); + msgParams.first = tr("Payment request expired."); msgParams.second = CClientUIInterface::MSG_ERROR; break; // included to prevent a compiler warning.