From d2744ade327fe6b63bb80d6acd9c1cd7ae6e317f Mon Sep 17 00:00:00 2001 From: Sona Kurazyan Date: Thu, 23 May 2019 15:37:21 +0200 Subject: Remove QCoapClien's constructor for specifying a custom connection Change-Id: I5b51c71ab11cbce5755a69c58f34bfa5e7cd5385 Reviewed-by: Simon Hausmann --- src/coap/qcoapclient.cpp | 34 +++++++++++++++++++++--------- src/coap/qcoapclient.h | 1 - src/coap/qcoapclient_p.h | 2 ++ src/coap/qcoapprotocol_p.h | 1 + tests/auto/qcoapclient/tst_qcoapclient.cpp | 16 ++++++++------ 5 files changed, 37 insertions(+), 17 deletions(-) diff --git a/src/coap/qcoapclient.cpp b/src/coap/qcoapclient.cpp index 5469ef2..4ff4ced 100644 --- a/src/coap/qcoapclient.cpp +++ b/src/coap/qcoapclient.cpp @@ -162,16 +162,8 @@ QCoapClientPrivate::~QCoapClientPrivate() constructors. */ QCoapClient::QCoapClient(QtCoap::SecurityMode securityMode, QObject *parent) : - QCoapClient(new QCoapQUdpConnection(securityMode), parent) -{ -} - -/*! - Constructs a QCoapClient object with the given \a connection and - sets \a parent as the parent object. -*/ -QCoapClient::QCoapClient(QCoapConnection *connection, QObject *parent) : - QObject(*new QCoapClientPrivate(new QCoapProtocol, connection), parent) + QObject(*new QCoapClientPrivate(new QCoapProtocol, new QCoapQUdpConnection(securityMode)), + parent) { Q_D(QCoapClient); @@ -209,6 +201,28 @@ QCoapClient::QCoapClient(QCoapConnection *connection, QObject *parent) : this, &QCoapClient::error); } +/*! + \internal + + Sets the client's connection to \a customConnection. +*/ +void QCoapClientPrivate::setConnection(QCoapConnection *customConnection) +{ + Q_Q(QCoapClient); + + delete connection; + connection = customConnection; + + q->connect(connection, &QCoapConnection::readyRead, protocol, + [this](const QByteArray &data, const QHostAddress &sender) { + protocol->d_func()->onFrameReceived(data, sender); + }); + q->connect(connection, &QCoapConnection::error, protocol, + [this](QAbstractSocket::SocketError socketError) { + protocol->d_func()->onConnectionError(socketError); + }); +} + /*! Destroys the QCoapClient object and frees up any resources. Note that QCoapReply objects that are returned from diff --git a/src/coap/qcoapclient.h b/src/coap/qcoapclient.h index 7171b59..31bf9f2 100644 --- a/src/coap/qcoapclient.h +++ b/src/coap/qcoapclient.h @@ -55,7 +55,6 @@ class Q_COAP_EXPORT QCoapClient : public QObject public: explicit QCoapClient(QtCoap::SecurityMode securityMode = QtCoap::SecurityMode::NoSecurity, QObject *parent = nullptr); - explicit QCoapClient(QCoapConnection *connection, QObject *parent = nullptr); ~QCoapClient(); QCoapReply *get(const QCoapRequest &request); diff --git a/src/coap/qcoapclient_p.h b/src/coap/qcoapclient_p.h index 84d3a83..18c916e 100644 --- a/src/coap/qcoapclient_p.h +++ b/src/coap/qcoapclient_p.h @@ -63,6 +63,8 @@ public: QCoapResourceDiscoveryReply *sendDiscovery(const QCoapRequest &request); bool send(QCoapReply *reply); + void setConnection(QCoapConnection *customConnection); + Q_DECLARE_PUBLIC(QCoapClient) }; diff --git a/src/coap/qcoapprotocol_p.h b/src/coap/qcoapprotocol_p.h index 4bd61c8..b24363e 100644 --- a/src/coap/qcoapprotocol_p.h +++ b/src/coap/qcoapprotocol_p.h @@ -100,6 +100,7 @@ private: Q_DECLARE_PRIVATE(QCoapProtocol) friend class QCoapClient; + friend class QCoapClientPrivate; }; struct CoapExchangeData { diff --git a/tests/auto/qcoapclient/tst_qcoapclient.cpp b/tests/auto/qcoapclient/tst_qcoapclient.cpp index 6f66346..0af3bee 100644 --- a/tests/auto/qcoapclient/tst_qcoapclient.cpp +++ b/tests/auto/qcoapclient/tst_qcoapclient.cpp @@ -108,9 +108,11 @@ private: class QCoapClientForSocketErrorTests : public QCoapClient { public: - QCoapClientForSocketErrorTests() : - QCoapClient(new QCoapQUdpConnectionSocketTests) - {} + QCoapClientForSocketErrorTests() + { + QCoapClientPrivate *privateClient = static_cast(d_func()); + privateClient->setConnection(new QCoapQUdpConnectionSocketTests()); + } QCoapQUdpConnection *connection() { @@ -162,9 +164,11 @@ public: class QCoapClientForMulticastTests : public QCoapClient { public: - QCoapClientForMulticastTests() : - QCoapClient(new QCoapConnectionMulticastTests) - {} + QCoapClientForMulticastTests() + { + QCoapClientPrivate *privateClient = static_cast(d_func()); + privateClient->setConnection(new QCoapConnectionMulticastTests()); + } QCoapConnection *connection() { -- cgit v1.2.3 From 430a041c2ce117282a5ef660b65d0a81f61c00b7 Mon Sep 17 00:00:00 2001 From: Sona Kurazyan Date: Wed, 22 May 2019 16:10:39 +0200 Subject: Add a method for setting the minimum token size For security reasons it is recommended to use tokens with a length of at least 4 bytes. Added a method for setting the minimum token size and changed it to be 4 by default. Change-Id: Ib589b338df8e59ccaf24dceab6691f92d92f861c Reviewed-by: Leena Miettinen Reviewed-by: Alex Blasche --- src/coap/qcoapclient.cpp | 12 ++++++++ src/coap/qcoapclient.h | 1 + src/coap/qcoapprotocol.cpp | 24 ++++++++++++++-- src/coap/qcoapprotocol_p.h | 2 ++ tests/auto/qcoapclient/tst_qcoapclient.cpp | 45 ++++++++++++++++++++++++++++++ 5 files changed, 81 insertions(+), 3 deletions(-) diff --git a/src/coap/qcoapclient.cpp b/src/coap/qcoapclient.cpp index 4ff4ced..76fee53 100644 --- a/src/coap/qcoapclient.cpp +++ b/src/coap/qcoapclient.cpp @@ -690,4 +690,16 @@ void QCoapClient::setMaximumRetransmitCount(uint maximumRetransmitCount) Q_ARG(uint, maximumRetransmitCount)); } +/*! + Sets the minimum token size to \a tokenSize in bytes. For security reasons it is + recommended to use tokens with a length of at least 4 bytes. The default value for + this parameter is 4 bytes. +*/ +void QCoapClient::setMinumumTokenSize(int tokenSize) +{ + Q_D(QCoapClient); + QMetaObject::invokeMethod(d->protocol, "setMinumumTokenSize", Qt::QueuedConnection, + Q_ARG(int, tokenSize)); +} + QT_END_NAMESPACE diff --git a/src/coap/qcoapclient.h b/src/coap/qcoapclient.h index 31bf9f2..c273aa1 100644 --- a/src/coap/qcoapclient.h +++ b/src/coap/qcoapclient.h @@ -88,6 +88,7 @@ public: void setAckTimeout(uint ackTimeout); void setAckRandomFactor(double ackRandomFactor); void setMaximumRetransmitCount(uint maximumRetransmitCount); + void setMinumumTokenSize(int tokenSize); Q_SIGNALS: void finished(QCoapReply *reply); diff --git a/src/coap/qcoapprotocol.cpp b/src/coap/qcoapprotocol.cpp index 46f420c..33ca9d7 100644 --- a/src/coap/qcoapprotocol.cpp +++ b/src/coap/qcoapprotocol.cpp @@ -690,9 +690,7 @@ QCoapToken QCoapProtocolPrivate::generateUniqueToken() const // TODO: Store used token for the period specified by CoAP spec QCoapToken token; while (isTokenRegistered(token)) { - // TODO: Allow setting minimum token size as a security setting - quint8 length = static_cast(QtCoap::randomGenerator().bounded(1, 8)); - + quint8 length = static_cast(QtCoap::randomGenerator().bounded(minimumTokenSize, 9)); token.resize(length); quint8 *tokenData = reinterpret_cast(token.data()); for (int i = 0; i < token.size(); ++i) @@ -1151,4 +1149,24 @@ void QCoapProtocol::setMaximumServerResponseDelay(uint responseDelay) d->maximumServerResponseDelay = responseDelay; } +/*! + \internal + + Sets the minimum token size to \a tokenSize in bytes. For security reasons it is + recommended to use tokens with a length of at least 4 bytes. The default value for + this parameter is 4 bytes. +*/ +void QCoapProtocol::setMinumumTokenSize(int tokenSize) +{ + Q_D(QCoapProtocol); + + if (tokenSize > 0 && tokenSize <= 8) { + d->minimumTokenSize = tokenSize; + } else { + qCWarning(lcCoapProtocol, + "Failed to set the minimum token size," + "it should not be more than 8 bytes and cannot be 0."); + } +} + QT_END_NAMESPACE diff --git a/src/coap/qcoapprotocol_p.h b/src/coap/qcoapprotocol_p.h index b24363e..4a7c61e 100644 --- a/src/coap/qcoapprotocol_p.h +++ b/src/coap/qcoapprotocol_p.h @@ -90,6 +90,7 @@ public: Q_INVOKABLE void setMaximumRetransmitCount(uint maximumRetransmitCount); Q_INVOKABLE void setBlockSize(quint16 blockSize); Q_INVOKABLE void setMaximumServerResponseDelay(uint responseDelay); + Q_INVOKABLE void setMinumumTokenSize(int tokenSize); private: Q_INVOKABLE void sendRequest(QPointer reply, QCoapConnection *connection); @@ -161,6 +162,7 @@ public: uint maximumRetransmitCount = 4; uint ackTimeout = 2000; uint maximumServerResponseDelay = 250 * 1000; + int minimumTokenSize = 4; double ackRandomFactor = 1.5; Q_DECLARE_PUBLIC(QCoapProtocol) diff --git a/tests/auto/qcoapclient/tst_qcoapclient.cpp b/tests/auto/qcoapclient/tst_qcoapclient.cpp index 0af3bee..aa8f70f 100644 --- a/tests/auto/qcoapclient/tst_qcoapclient.cpp +++ b/tests/auto/qcoapclient/tst_qcoapclient.cpp @@ -77,6 +77,8 @@ private Q_SLOTS: void confirmableMulticast(); void multicast(); void multicast_blockwise(); + void setMinimumTokenSize_data(); + void setMinimumTokenSize(); }; #ifdef QT_BUILD_INTERNAL @@ -871,6 +873,49 @@ void tst_QCoapClient::multicast_blockwise() #endif } +void tst_QCoapClient::setMinimumTokenSize_data() +{ + QTest::addColumn("minTokenSize"); + QTest::addColumn("expectedMinSize"); + + QTest::newRow("in_range") << 6 << 6; + QTest::newRow("out_of_range_small") << 0 << 4; + QTest::newRow("out_of_range_big") << 9 << 4; +} + +void noMessageOutput(QtMsgType, const QMessageLogContext &, const QString &) {} + +void tst_QCoapClient::setMinimumTokenSize() +{ +#ifdef QT_BUILD_INTERNAL + // Don't show warning messages for the out of range values + qInstallMessageHandler(noMessageOutput); + + QFETCH(int, minTokenSize); + QFETCH(int, expectedMinSize); + + const int maxSize = 8; + + for (int i = 0; i < 20; ++i) { + QCoapClientForSocketErrorTests client; + client.setMinumumTokenSize(minTokenSize); + + // With QCoapClientForSocketErrorTests the request will fail, but it doesn't matter, + // we are interested only in the generated token. + QSignalSpy spyClientError(&client, &QCoapClient::error); + + QScopedPointer reply; + reply.reset(client.get(QCoapRequest("127.0.0.1"))); + + QTRY_COMPARE_WITH_TIMEOUT(spyClientError.count(), 1, 10); + QVERIFY(reply->request().tokenLength() >= expectedMinSize); + QVERIFY(reply->request().tokenLength() <= maxSize); + } +#else + QSKIP("Not an internal build, skipping this test"); +#endif +} + QTEST_MAIN(tst_QCoapClient) #include "tst_qcoapclient.moc" -- cgit v1.2.3 From 0c00be007c3a72928f6f8e167e8e6f2469f1a88d Mon Sep 17 00:00:00 2001 From: Sona Kurazyan Date: Fri, 24 May 2019 16:15:17 +0200 Subject: Fix a typo: setMinumumTokenSize -> setMinimumTokenSize Change-Id: Ifcef69e73ea89f30499063452920dd9abb446f7e Reviewed-by: Simon Hausmann --- src/coap/qcoapclient.cpp | 4 ++-- src/coap/qcoapclient.h | 2 +- src/coap/qcoapprotocol.cpp | 2 +- src/coap/qcoapprotocol_p.h | 2 +- tests/auto/qcoapclient/tst_qcoapclient.cpp | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coap/qcoapclient.cpp b/src/coap/qcoapclient.cpp index 76fee53..dc02464 100644 --- a/src/coap/qcoapclient.cpp +++ b/src/coap/qcoapclient.cpp @@ -695,10 +695,10 @@ void QCoapClient::setMaximumRetransmitCount(uint maximumRetransmitCount) recommended to use tokens with a length of at least 4 bytes. The default value for this parameter is 4 bytes. */ -void QCoapClient::setMinumumTokenSize(int tokenSize) +void QCoapClient::setMinimumTokenSize(int tokenSize) { Q_D(QCoapClient); - QMetaObject::invokeMethod(d->protocol, "setMinumumTokenSize", Qt::QueuedConnection, + QMetaObject::invokeMethod(d->protocol, "setMinimumTokenSize", Qt::QueuedConnection, Q_ARG(int, tokenSize)); } diff --git a/src/coap/qcoapclient.h b/src/coap/qcoapclient.h index c273aa1..a7478cf 100644 --- a/src/coap/qcoapclient.h +++ b/src/coap/qcoapclient.h @@ -88,7 +88,7 @@ public: void setAckTimeout(uint ackTimeout); void setAckRandomFactor(double ackRandomFactor); void setMaximumRetransmitCount(uint maximumRetransmitCount); - void setMinumumTokenSize(int tokenSize); + void setMinimumTokenSize(int tokenSize); Q_SIGNALS: void finished(QCoapReply *reply); diff --git a/src/coap/qcoapprotocol.cpp b/src/coap/qcoapprotocol.cpp index 33ca9d7..3a6f3ca 100644 --- a/src/coap/qcoapprotocol.cpp +++ b/src/coap/qcoapprotocol.cpp @@ -1156,7 +1156,7 @@ void QCoapProtocol::setMaximumServerResponseDelay(uint responseDelay) recommended to use tokens with a length of at least 4 bytes. The default value for this parameter is 4 bytes. */ -void QCoapProtocol::setMinumumTokenSize(int tokenSize) +void QCoapProtocol::setMinimumTokenSize(int tokenSize) { Q_D(QCoapProtocol); diff --git a/src/coap/qcoapprotocol_p.h b/src/coap/qcoapprotocol_p.h index 4a7c61e..0e3d3ed 100644 --- a/src/coap/qcoapprotocol_p.h +++ b/src/coap/qcoapprotocol_p.h @@ -90,7 +90,7 @@ public: Q_INVOKABLE void setMaximumRetransmitCount(uint maximumRetransmitCount); Q_INVOKABLE void setBlockSize(quint16 blockSize); Q_INVOKABLE void setMaximumServerResponseDelay(uint responseDelay); - Q_INVOKABLE void setMinumumTokenSize(int tokenSize); + Q_INVOKABLE void setMinimumTokenSize(int tokenSize); private: Q_INVOKABLE void sendRequest(QPointer reply, QCoapConnection *connection); diff --git a/tests/auto/qcoapclient/tst_qcoapclient.cpp b/tests/auto/qcoapclient/tst_qcoapclient.cpp index aa8f70f..20f1cab 100644 --- a/tests/auto/qcoapclient/tst_qcoapclient.cpp +++ b/tests/auto/qcoapclient/tst_qcoapclient.cpp @@ -898,7 +898,7 @@ void tst_QCoapClient::setMinimumTokenSize() for (int i = 0; i < 20; ++i) { QCoapClientForSocketErrorTests client; - client.setMinumumTokenSize(minTokenSize); + client.setMinimumTokenSize(minTokenSize); // With QCoapClientForSocketErrorTests the request will fail, but it doesn't matter, // we are interested only in the generated token. -- cgit v1.2.3