From 5768c369538680636b6a6803132b5d9e5b24f8f2 Mon Sep 17 00:00:00 2001 From: Patrick Schurig Date: Sat, 17 Oct 2026 14:44:19 +0200 Subject: [PATCH] =?UTF-8?q?fix(optimiseur):=20token=20d'au=20moins=2016=20?= =?UTF-8?q?caract=C3=A8res=20(correctifs=20brief=204,=20point=204)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Question 4 de T7 : le masquage à la réception remplace TOUTE occurrence du token dans ce que l'optimiseur renvoie ; un token court, présent par hasard dans un identifiant de charge, corromprait le plan reçu (unknown-load). - SetOptimizerToken (writeToken) refuse un token non vide de moins de 16 caractères (points de code UTF-8) : EnergyErrorInvalidParameter, fault `token-refused` comme les autres refus ; rien n'est modifié ; le motif est journalisé, jamais la valeur. Vide = retrait, inchangé. - Le client relit le fichier au début de chaque tentative ; plus court que 16 caractères, il ne se connecte PAS (ni avec ce token, ni sans token : les mesures du site ne partent pas vers un optimiseur que rien n'authentifie). lastErrorCode `token-too-short`, dit une fois par série, nouvelle tentative au backoff jusqu'au remplacement du fichier. INTERFACE.md (SetOptimizerToken, lastErrorCode) et protocole §3 (E-T2-8, forme du token). Tests : 15 refusé, 16 accepté, 15 « é » refusés (caractères, pas octets), vide = retrait, fichier court sans connexion puis token conforme envoyé ; tokens des suites portés à 16 au moins. Contre-épreuves M4a (refus retiré) et M4b (fichier court envoyé) : mordent. Co-Authored-By: Claude Opus 5.5 --- docs/OPTIMIZER_PROTOCOL.md | 6 +- .../etm/scheduler/optimizerclient.cpp | 27 ++++++- energyplugin/etm/scheduler/optimizerclient.h | 18 ++++- .../optimizerclient/testoptimizerclient.cpp | 76 +++++++++++++++++-- .../socketscheduler/testsocketscheduler.cpp | 15 +++- 5 files changed, 130 insertions(+), 12 deletions(-) diff --git a/docs/OPTIMIZER_PROTOCOL.md b/docs/OPTIMIZER_PROTOCOL.md index b0df5c0..221ba6d 100644 --- a/docs/OPTIMIZER_PROTOCOL.md +++ b/docs/OPTIMIZER_PROTOCOL.md @@ -136,7 +136,11 @@ Réponse attendue : - `planning` : `true` si l'optimiseur produit des plans multi-créneaux ; `false` s'il ne répond qu'au présent (plan à un créneau). Informationnel. - `token` : si configuré côté plugin et absent/incorrect côté serveur → connexion fermée. - Recommandé pour tout endpoint `tcp://`. + Recommandé pour tout endpoint `tcp://`. **Forme** (E-T2-8) : chaîne de **16 caractères au + moins** et de 4 096 octets au plus, imprimable, sans blanc ni caractère de contrôle (UTF-8 + admis). Le plugin refuse d'en poser un plus court, et ne se connecte pas avec un fichier de + token plus court (ni sans token) : il masque toute occurrence du token dans ce que l'optimiseur + renvoie, et un token court corromprait un identifiant qui le contiendrait. - `capabilities` (1.1, optionnel) : affiché en mode installateur. **S'il est présent**, le plugin rejette toute action dont le `kind` n'y est pas annoncé. - Pas de `locale` : la locale nymea est par connexion app (`AGENTS.md` invariant 12) ; la diff --git a/energyplugin/etm/scheduler/optimizerclient.cpp b/energyplugin/etm/scheduler/optimizerclient.cpp index 0aab4d0..12834c3 100644 --- a/energyplugin/etm/scheduler/optimizerclient.cpp +++ b/energyplugin/etm/scheduler/optimizerclient.cpp @@ -172,6 +172,8 @@ bool OptimizerClient::writeToken(const QString &path, const QByteArray &token, Q // survivrait pas à l'aller-retour, et le handshake échouerait sans raison visible. if (token.size() > kMaxTokenBytes) return fail(QStringLiteral("token refusé : plus de %1 octets").arg(kMaxTokenBytes)); + if (tokenLength(token) < kMinTokenChars) + return fail(QStringLiteral("token refusé : moins de %1 caractères").arg(kMinTokenChars)); for (const char c : token) { if (static_cast(c) <= 0x20 || c == 0x7f) return fail(QStringLiteral("token refusé : blanc ou caractère de contrôle")); @@ -198,6 +200,11 @@ bool OptimizerClient::writeToken(const QString &path, const QByteArray &token, Q return true; } +int OptimizerClient::tokenLength(const QByteArray &token) +{ + return int(QString::fromUtf8(token).toUcs4().size()); +} + void OptimizerClient::start() { if (m_state != Stopped) @@ -267,6 +274,20 @@ void OptimizerClient::connectNow() m_tokenSent = false; m_handshakeDeadline = m_clock->nowMs() + kHandshakeTimeoutMs; setState(Connecting); + + // Relu à CHAQUE tentative (décision Q2) : un token remplacé vaut dès la suivante. Un fichier + // trop court (posé à la main : la RPC le refuse) n'est pas envoyé, et l'on ne se connecte PAS + // sans lui : l'installateur a voulu une connexion authentifiée, et les mesures du site ne + // partent pas vers un optimiseur que rien n'authentifie. Dit une fois par série (failConnection), + // jamais la valeur ni sa longueur. + m_tokenTentative = readToken(); + if (!m_tokenTentative.isEmpty() && tokenLength(m_tokenTentative) < kMinTokenChars) { + m_tokenTentative.clear(); + failConnection(QStringLiteral("token-too-short"), + QStringLiteral("fichier de token de moins de %1 caractères — aucune connexion " + "tant qu'il n'est pas remplacé (SetOptimizerToken)").arg(kMinTokenChars)); + return; + } rearm(); if (m_endpoint.kind == OptimizerEndpoint::Unix) { @@ -300,8 +321,9 @@ void OptimizerClient::onConnected() {QStringLiteral("protocolVersion"), QLatin1String(kProtocolVersion)}, {QStringLiteral("engine"), QStringLiteral("etm-powersync-energy-plugin-etm/") + m_engineVersion}, }; - // Relu à CHAQUE connexion (décision Q2) : un token remplacé vaut dès la tentative suivante. - const QByteArray token = readToken(); + // Lu au début de cette tentative (connectNow) : un seul accès au fichier par tentative. + const QByteArray token = m_tokenTentative; + m_tokenTentative.clear(); m_tokenSent = !token.isEmpty(); m_tokenConnexion = QString::fromUtf8(token); if (m_tokenSent) @@ -553,6 +575,7 @@ void OptimizerClient::failConnection(const QString &code, const QString &detail) void OptimizerClient::dropSocket() { + m_tokenTentative.clear(); QIODevice *socket = m_socket; if (!socket) return; diff --git a/energyplugin/etm/scheduler/optimizerclient.h b/energyplugin/etm/scheduler/optimizerclient.h index bd30294..3628b02 100644 --- a/energyplugin/etm/scheduler/optimizerclient.h +++ b/energyplugin/etm/scheduler/optimizerclient.h @@ -155,6 +155,19 @@ public: /*! \brief Nom du fichier de token, sous \c NymeaSettings::settingsPath() (décision Q2). */ static constexpr const char *kTokenFileName = "energy-optimizer.token"; + /*! + * \brief Longueur minimale d'un token non vide, en caractères. + * + * Le masquage à la réception (\c OptimizerSecret) remplace TOUTE occurrence du token dans ce + * que l'optimiseur renvoie : un token court, présent par hasard dans un identifiant de charge + * ou un code, corromprait le plan reçu (\c unknown-load → repli). 16 caractères rendent la + * rencontre fortuite négligeable. + */ + static constexpr int kMinTokenChars = 16; + + /*! \brief \return Nombre de caractères (points de code) de \p token, lu en UTF-8. */ + static int tokenLength(const QByteArray &token); + /*! * \brief Construit un client ARRÊTÉ : rien n'est ouvert avant \c start(). * \param endpoint Adresse de l'optimiseur ; invalide → \c start() refuse. @@ -181,7 +194,9 @@ public: * aux droits plus larges est ainsi resserré à 0600. * * \param path Fichier cible (\c defaultTokenPath() en production). Jamais \c energy.conf. - * \param token Secret ; vide = retrait du fichier (plus de token configuré). + * \param token Secret ; vide = retrait du fichier (plus de token configuré). Refusé s'il + * compte moins de \c kMinTokenChars caractères, plus de 4 096 octets, un blanc ou un + * caractère de contrôle — et rien n'est alors modifié. * \param[out] error Motif d'échec, en français, sans le secret. * \return Vrai si le fichier est posé (ou retiré) avec succès. * \note Pas de RPC dans ce lot : c'est la fonction qu'elle appellera. @@ -297,6 +312,7 @@ private: quint64 m_pendingId = 0; bool m_tokenSent = false; // le handshake en cours porte un token (pour le diagnostic) QString m_tokenConnexion; // le token envoyé sur CETTE connexion : ce qu'un écho recopierait + QByteArray m_tokenTentative; // relu au début de la tentative, envoyé à son handshake bool m_echoTokenSignale = false; qint64 m_handshakeDeadline = -1; // -1 = pas d'échéance diff --git a/tests/auto/optimizerclient/testoptimizerclient.cpp b/tests/auto/optimizerclient/testoptimizerclient.cpp index 6512df8..2db6918 100644 --- a/tests/auto/optimizerclient/testoptimizerclient.cpp +++ b/tests/auto/optimizerclient/testoptimizerclient.cpp @@ -109,6 +109,7 @@ private slots: void testBadLinesKeepConnection(); void testWriteToken(); void testTokenNeverInAnyLog(); + void testAShortTokenFileOpensNoConnection(); private: // Démarre le client et mène le handshake jusqu'à Ready. @@ -350,7 +351,7 @@ void TestOptimizerClient::testInvalidHandshakeIsRefused() void TestOptimizerClient::testWrongTokenThenFixedToken() { const QString tokenPath = m_dir.filePath("energy-optimizer.token"); - QVERIFY(OptimizerClient::writeToken(tokenPath, "jeton-faux-7f3a")); + QVERIFY(OptimizerClient::writeToken(tokenPath, "jeton-faux-7f3a9")); FakeOptimizer server(FakeOptimizer::Tcp, m_dir.path()); QVERIFY(server.listen()); @@ -361,7 +362,7 @@ void TestOptimizerClient::testWrongTokenThenFixedToken() client.start(); QJsonObject hs; QVERIFY(server.waitMessage(&hs)); - QCOMPARE(hs.value("params").toObject().value("token").toString(), QStringLiteral("jeton-faux-7f3a")); + QCOMPARE(hs.value("params").toObject().value("token").toString(), QStringLiteral("jeton-faux-7f3a9")); server.abortClient(); QVERIFY(waitFor([&] { return client.state() == OptimizerClient::Waiting; })); QCOMPARE(client.lastErrorCode(), QStringLiteral("closed-during-handshake")); @@ -668,22 +669,22 @@ void TestOptimizerClient::testWriteToken() // Nouveau fichier : 0600 malgré un umask permissif. QString error; - QVERIFY2(OptimizerClient::writeToken(path, "secret-un", &error), qPrintable(error)); + QVERIFY2(OptimizerClient::writeToken(path, "secret-un-16-car", &error), qPrintable(error)); QCOMPARE(::stat(QFile::encodeName(path).constData(), &st), 0); QCOMPARE(int(st.st_mode & 0777), 0600); QFile f(path); QVERIFY(f.open(QIODevice::ReadOnly)); - QCOMPARE(f.readAll(), QByteArray("secret-un")); + QCOMPARE(f.readAll(), QByteArray("secret-un-16-car")); f.close(); // Fichier préexistant en 0644 (posé à la main) : remplacé ET resserré. QVERIFY(QFile::setPermissions(path, QFileDevice::ReadOwner | QFileDevice::WriteOwner | QFileDevice::ReadGroup | QFileDevice::ReadOther)); - QVERIFY(OptimizerClient::writeToken(path, "secret-deux")); + QVERIFY(OptimizerClient::writeToken(path, "secret-deux-1234")); QCOMPARE(::stat(QFile::encodeName(path).constData(), &st), 0); QCOMPARE(int(st.st_mode & 0777), 0600); QVERIFY(f.open(QIODevice::ReadOnly)); - QCOMPARE(f.readAll(), QByteArray("secret-deux")); + QCOMPARE(f.readAll(), QByteArray("secret-deux-1234")); f.close(); // Refus : blanc ou contrôle (le trimmed() de la relecture ne les rendrait pas), trop long. @@ -691,13 +692,25 @@ void TestOptimizerClient::testWriteToken() QVERIFY(!error.contains("avec")); QVERIFY(!OptimizerClient::writeToken(path, " bord")); QVERIFY(!OptimizerClient::writeToken(path, QByteArray(4097, 'a'))); + // Trop court : moins de 16 caractères (le masquage remplace toute occurrence du token). Comptés + // en caractères : 15 « é » font 30 octets et restent refusés. + QVERIFY(!OptimizerClient::writeToken(path, "quinze-car-1234", &error)); + QVERIFY2(error.contains("moins de 16 caractères"), qUtf8Printable(error)); + QVERIFY(!error.contains("quinze")); + QVERIFY(!OptimizerClient::writeToken(path, QString(15, QChar(0xE9)).toUtf8())); QVERIFY(f.open(QIODevice::ReadOnly)); - QCOMPARE(f.readAll(), QByteArray("secret-deux")); // intact + QCOMPARE(f.readAll(), QByteArray("secret-deux-1234")); // intact f.close(); // Seul ce fichier est écrit à côté : aucun temporaire laissé, rien d'autre (energy.conf). QCOMPARE(QDir(m_dir.filePath("tok")).entryList(QDir::Files | QDir::Hidden), QStringList{"energy-optimizer.token"}); + // 16 caractères : accepté, exactement à la borne. + QVERIFY2(OptimizerClient::writeToken(path, "seize-car-123456", &error), qPrintable(error)); + QVERIFY(f.open(QIODevice::ReadOnly)); + QCOMPARE(f.readAll(), QByteArray("seize-car-123456")); + f.close(); + // Vide : retrait (plus de token configuré) ; retirer un absent n'est pas une erreur. QVERIFY(OptimizerClient::writeToken(path, QByteArray())); QVERIFY(!QFile::exists(path)); @@ -768,5 +781,54 @@ void TestOptimizerClient::testTokenNeverInAnyLog() QVERIFY2(!line.contains(secret) && !line.contains("Zq81"), qPrintable("token dans le journal : " + line)); } +void TestOptimizerClient::testAShortTokenFileOpensNoConnection() +{ + // Un fichier de token de moins de 16 caractères (posé à la main : la RPC le refuse) : le client + // ne se connecte PAS — ni avec ce token, ni sans token. Dit une fois, sans la valeur. Relu à + // chaque tentative : un token conforme posé ensuite sert à la suivante. + const QString tokenPath = m_dir.filePath("energy-optimizer.token"); + const QByteArray court = "court-4f2a-9c1e"; // 15 caractères + { + QFile f(tokenPath); + QVERIFY(f.open(QIODevice::WriteOnly | QIODevice::Truncate)); + QVERIFY(f.setPermissions(QFileDevice::ReadOwner | QFileDevice::WriteOwner)); + QCOMPARE(f.write(court + '\n'), qint64(court.size() + 1)); + } + FakeOptimizer server(FakeOptimizer::Unix, m_dir.path()); + QVERIFY(server.listen()); + SimClock clock; + OptimizerClient client(OptimizerEndpoint::parse(server.endpoint()), tokenPath, "9.9", &clock); + int attempts = 0; + connect(&client, &OptimizerClient::stateChanged, this, [&](OptimizerClient::State s) { + if (s == OptimizerClient::Connecting) + ++attempts; + }); + + client.start(); + settle(); + QCOMPARE(client.state(), OptimizerClient::Waiting); + QCOMPARE(client.lastErrorCode(), QStringLiteral("token-too-short")); + QCOMPARE(server.connections, 0); + // Les tentatives suivantes relisent le fichier, échouent pareil, et ne le redisent pas. + clock.advance(1000); + settle(); + clock.advance(2000); + settle(); + QCOMPARE(attempts, 3); + QCOMPARE(server.connections, 0); + QCOMPARE(countLog("(token-too-short) :"), 1); + QCOMPARE(countLog("moins de 16 caractères"), 1); + for (const QString &line : std::as_const(s_log)) + QVERIFY2(!line.contains(QString::fromUtf8(court)), qUtf8Printable(line)); + + // Remplacé par un token conforme : la tentative suivante se connecte et l'envoie. + QVERIFY(OptimizerClient::writeToken(tokenPath, "seize-car-123456")); + clock.advance(4000); + QJsonObject hs; + QVERIFY(server.waitMessage(&hs)); + QCOMPARE(hs.value("params").toObject().value("token").toString(), QStringLiteral("seize-car-123456")); + QCOMPARE(server.connections, 1); +} + QTEST_GUILESS_MAIN(TestOptimizerClient) #include "testoptimizerclient.moc" diff --git a/tests/auto/socketscheduler/testsocketscheduler.cpp b/tests/auto/socketscheduler/testsocketscheduler.cpp index 54bc29e..84c3d71 100644 --- a/tests/auto/socketscheduler/testsocketscheduler.cpp +++ b/tests/auto/socketscheduler/testsocketscheduler.cpp @@ -1320,11 +1320,22 @@ void TestSocketScheduler::testTokenIsWriteOnlyAndNeverLeaks() // Refus : pas une chaîne, un blanc — rien ne change, le secret reste. r = injectAndWait("NymeaEnergy.SetOptimizerToken", {{"token", 31415926}}).toMap().value("params").toMap(); QCOMPARE(r.value("fault").toString(), QString("token-refused")); - r = rpc("SetOptimizerToken", {{"token", "avec blanc"}}); + r = rpc("SetOptimizerToken", {{"token", "avec un blanc dedans"}}); + QCOMPARE(r.value("fault").toString(), QString("token-refused")); + // Trop court (15 caractères) : refusé de même, par code, sans rien changer. + r = rpc("SetOptimizerToken", {{"token", "quinze-car-1234"}}); + QCOMPARE(r.value("energyError").toString(), QString("EnergyErrorInvalidParameter")); QCOMPARE(r.value("fault").toString(), QString("token-refused")); QVERIFY(f.open(QIODevice::ReadOnly)); QCOMPARE(QString::fromUtf8(f.readAll()).trimmed(), secret); f.close(); + QVERIFY(j.texte().contains("moins de 16 caractères")); + // 16 caractères : accepté. + QCOMPARE(rpc("SetOptimizerToken", {{"token", "seize-car-123456"}}).value("energyError").toString(), + QString("EnergyErrorNoError")); + QVERIFY(f.open(QIODevice::ReadOnly)); + QCOMPARE(QString::fromUtf8(f.readAll()).trimmed(), QString("seize-car-123456")); + f.close(); // Retrait : fichier supprimé, tokenConfigured faux. QCOMPARE(rpc("SetOptimizerToken", {{"token", ""}}).value("energyError").toString(), QString("EnergyErrorNoError")); @@ -1349,6 +1360,8 @@ void TestSocketScheduler::testTokenIsWriteOnlyAndNeverLeaks() }; QVERIFY2(horsTrafic(secret).isEmpty(), qUtf8Printable(horsTrafic(secret).join('\n'))); QVERIFY2(horsTrafic("31415926").isEmpty(), qUtf8Printable(horsTrafic("31415926").join('\n'))); + for (const QString &x : {QStringLiteral("quinze-car-1234"), QStringLiteral("seize-car-123456")}) + QVERIFY2(horsTrafic(x).isEmpty(), qUtf8Printable(horsTrafic(x).join('\n'))); QVERIFY2(!j.lignes(secret).isEmpty(), "la trace JsonRpcTraffic de nymea ne recopie plus les données entrantes : retirer la mise " "en garde d'INTERFACE.md (SetOptimizerToken)");