Merge pull request #5467

6171e49 [Qt] Use identical strings for expired payment request message (Philip Kaufmann)
06087bd [Qt] minor comment updates in PaymentServer (Philip Kaufmann)
35d1595 [Qt] constify first parameter of processPaymentRequest() (Philip Kaufmann)
9b14aef [Qt] take care of a missing typecast in PaymentRequestPlus::getMerchant() (Philip Kaufmann)
d19ae3c [Qt] remove unused PaymentRequestPlus::getPKIType function (Philip Kaufmann)
6e17a74 [Qt] paymentserver: better logging of invalid certs (Philip Kaufmann)
5a53d7c [Qt] paymentserver: do not log NULL certificates (Philip Kaufmann)

5 files changed, 28 insertions, 26 deletions

diff --git a/src/qt/paymentrequestplus.cpp b/src/qt/paymentrequestplus.cpp
index b69461ad9e..7e9729eeb9 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();
@@ -124,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];
@@ -172,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.
diff --git a/src/qt/paymentrequestplus.h b/src/qt/paymentrequestplus.h
index 61f8a3415d..99a7186b85 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;
diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp
index ad489de343..09e9949b10 100644
--- a/src/qt/paymentserver.cpp
+++ b/src/qt/paymentserver.cpp
@@ -97,7 +97,11 @@ static QList<QString> 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
 }
 
 //
@@ -143,13 +147,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;
@@ -301,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);
 
@@ -332,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<QFileOpenEvent*>(event);
         if (!fileEvent->file().isEmpty())
             handleURIOrFile(fileEvent->file());
@@ -515,7 +525,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;
@@ -560,9 +570,9 @@ bool PaymentServer::processPaymentRequest(PaymentRequestPlus& request, SendCoins
             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);
diff --git a/src/qt/paymentserver.h b/src/qt/paymentserver.h
index 6bf5ac2eea..32ed27983e 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
diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
index 774667d4ac..59939fa871 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.