From 4c9b8c77943b58e610910adb33d98def179357d7 Mon Sep 17 00:00:00 2001 From: Weslly Date: Wed, 3 May 2017 20:54:19 -0300 Subject: [PATCH] Review fixes --- src/CMakeLists.txt | 2 ++ src/core/Entry.cpp | 2 +- src/gui/TotpDialog.cpp | 2 +- src/totp/base32.cpp | 68 ++++++++++++++++++++++++++++++++++++++++++ src/totp/base32.h | 34 +++++++++++++++++++++ src/totp/totp.cpp | 57 +++-------------------------------- src/totp/totp.h | 1 - tests/TestTotp.cpp | 11 +++---- tests/TestTotp.h | 2 +- 9 files changed, 117 insertions(+), 62 deletions(-) create mode 100644 src/totp/base32.cpp create mode 100644 src/totp/base32.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index d50d27b69..750b6481c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -131,6 +131,8 @@ set(keepassx_SOURCES streams/qtiocompressor.cpp streams/StoreDataStream.cpp streams/SymmetricCipherStream.cpp + totp/base32.h + totp/base32.cpp totp/totp.h totp/totp.cpp ) diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index e86a7092f..418e2a81e 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -329,7 +329,7 @@ QString Entry::totpSeed() const if (m_attributes->hasKey("otp")) { secret = m_attributes->value("otp"); - } else { + } else if (m_attributes->hasKey("TOTP Seed")) { secret = m_attributes->value("TOTP Seed"); } diff --git a/src/gui/TotpDialog.cpp b/src/gui/TotpDialog.cpp index 2eb1c9e2c..521ddbafe 100644 --- a/src/gui/TotpDialog.cpp +++ b/src/gui/TotpDialog.cpp @@ -41,7 +41,7 @@ TotpDialog::TotpDialog(DatabaseWidget* parent, Entry* entry) uCounter = resetCounter(); updateProgressBar(); - QTimer *timer = new QTimer(this); + QTimer* timer = new QTimer(this); connect(timer, SIGNAL(timeout()), this, SLOT(updateProgressBar())); connect(timer, SIGNAL(timeout()), this, SLOT(updateSeconds())); timer->start(m_step * 10); diff --git a/src/totp/base32.cpp b/src/totp/base32.cpp new file mode 100644 index 000000000..07526aa02 --- /dev/null +++ b/src/totp/base32.cpp @@ -0,0 +1,68 @@ +// Base32 implementation +// +// Copyright 2010 Google Inc. +// Author: Markus Gutschke +// Source: https://github.com/google/google-authenticator-libpam/blob/master/src/base32.c +// Modifications copyright (C) 2017 KeePassXC team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "base32.h" + +Base32::Base32() +{ +} + +QByteArray Base32::base32_decode(const QByteArray encoded) +{ + QByteArray result; + + int buffer = 0; + int bitsLeft = 0; + + for (char ch : encoded) { + if (ch == 0 || ch == ' ' || ch == '\t' || ch == '\r' || ch == '\n' || ch == '-' || ch == '=') { + continue; + } + + buffer <<= 5; + + // Deal with commonly mistyped characters + if (ch == '0') { + ch = 'O'; + } else if (ch == '1') { + ch = 'L'; + } else if (ch == '8') { + ch = 'B'; + } + + // Look up one base32 digit + if ((ch >= 'A' && ch <= 'Z') || (ch >= 'a' && ch <= 'z')) { + ch = (ch & 0x1F) - 1; + } else if (ch >= '2' && ch <= '7') { + ch -= '2' - 26; + } else { + return QByteArray(); + } + + buffer |= ch; + bitsLeft += 5; + + if (bitsLeft >= 8) { + result.append(static_cast (buffer >> (bitsLeft - 8))); + bitsLeft -= 8; + } + } + + return result; +} \ No newline at end of file diff --git a/src/totp/base32.h b/src/totp/base32.h new file mode 100644 index 000000000..3f1965b2f --- /dev/null +++ b/src/totp/base32.h @@ -0,0 +1,34 @@ +// Base32 implementation +// +// Copyright 2010 Google Inc. +// Author: Markus Gutschke +// Source: https://github.com/google/google-authenticator-libpam/blob/master/src/base32.h +// Modifications copyright (C) 2017 KeePassXC team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef BASE32_H +#define BASE32_H + +#include +#include + +class Base32 +{ +public: + Base32(); + static QByteArray base32_decode(const QByteArray encoded); +}; + + +#endif //BASE32_H diff --git a/src/totp/totp.cpp b/src/totp/totp.cpp index 45e98d38d..f85c76f06 100644 --- a/src/totp/totp.cpp +++ b/src/totp/totp.cpp @@ -16,6 +16,7 @@ */ #include "totp.h" +#include "base32.h" #include #include #include @@ -91,62 +92,12 @@ QString QTotp::parseOtpString(QString key, quint8 &digits, quint8 &step) return seed; } - -QByteArray QTotp::base32_decode(const QByteArray encoded) -{ - // Base32 implementation - // Copyright 2010 Google Inc. - // Author: Markus Gutschke - // Licensed under the Apache License, Version 2.0 - - QByteArray result; - - int buffer = 0; - int bitsLeft = 0; - - for (char ch : encoded) { - if (ch == 0 || ch == ' ' || ch == '\t' || ch == '\r' || ch == '\n' || ch == '-' || ch == '=') { - continue; - } - - buffer <<= 5; - - // Deal with commonly mistyped characters - if (ch == '0') { - ch = 'O'; - } else if (ch == '1') { - ch = 'L'; - } else if (ch == '8') { - ch = 'B'; - } - - // Look up one base32 digit - if ((ch >= 'A' && ch <= 'Z') || (ch >= 'a' && ch <= 'z')) { - ch = (ch & 0x1F) - 1; - } else if (ch >= '2' && ch <= '7') { - ch -= '2' - 26; - } else { - return QByteArray(); - } - - buffer |= ch; - bitsLeft += 5; - - if (bitsLeft >= 8) { - result.append(static_cast (buffer >> (bitsLeft - 8))); - bitsLeft -= 8; - } - } - - return result; -} - - -QString QTotp::generateTotp(const QByteArray key, quint64 time, const quint8 numDigits = defaultDigits, const quint8 step = defaultStep) +QString QTotp::generateTotp(const QByteArray key, quint64 time, + const quint8 numDigits = defaultDigits, const quint8 step = defaultStep) { quint64 current = qToBigEndian(time / step); - QByteArray secret = QTotp::base32_decode(key); + QByteArray secret = Base32::base32_decode(key); if (secret.isEmpty()) { return "Invalid TOTP secret key"; } diff --git a/src/totp/totp.h b/src/totp/totp.h index 8d7e86744..260babc22 100644 --- a/src/totp/totp.h +++ b/src/totp/totp.h @@ -25,7 +25,6 @@ class QTotp public: QTotp(); static QString parseOtpString(QString rawSecret, quint8 &digits, quint8 &step); - static QByteArray base32_decode(const QByteArray encoded); static QString generateTotp(const QByteArray key, quint64 time, const quint8 numDigits, const quint8 step); static const quint8 defaultStep; static const quint8 defaultDigits; diff --git a/tests/TestTotp.cpp b/tests/TestTotp.cpp index c1fe5943c..e5da3c642 100644 --- a/tests/TestTotp.cpp +++ b/tests/TestTotp.cpp @@ -25,6 +25,7 @@ #include "crypto/Crypto.h" #include "totp/totp.h" +#include "totp/base32.h" QTEST_GUILESS_MAIN(TestTotp) @@ -34,7 +35,7 @@ void TestTotp::initTestCase() } -void TestTotp::testSecret() +void TestTotp::testParseSecret() { quint8 digits = 0; quint8 step = 0; @@ -61,19 +62,19 @@ void TestTotp::testSecret() void TestTotp::testBase32() { QByteArray key = QString("JBSW Y3DP EB3W 64TM MQXC 4LQA").toLatin1(); - QByteArray secret = QTotp::base32_decode(key); + QByteArray secret = Base32::base32_decode(key); QCOMPARE(QString::fromLatin1(secret), QString("Hello world...")); key = QString("gezdgnbvgy3tqojqgezdgnbvgy3tqojq").toLatin1(); - secret = QTotp::base32_decode(key); + secret = Base32::base32_decode(key); QCOMPARE(QString::fromLatin1(secret), QString("12345678901234567890")); key = QString("ORSXG5A=").toLatin1(); - secret = QTotp::base32_decode(key); + secret = Base32::base32_decode(key); QCOMPARE(QString::fromLatin1(secret), QString("test")); key = QString("MZXW6YTBOI======").toLatin1(); - secret = QTotp::base32_decode(key); + secret = Base32::base32_decode(key); QCOMPARE(QString::fromLatin1(secret), QString("foobar")); } diff --git a/tests/TestTotp.h b/tests/TestTotp.h index 7bfa68055..9871aaf27 100644 --- a/tests/TestTotp.h +++ b/tests/TestTotp.h @@ -28,7 +28,7 @@ class TestTotp : public QObject private slots: void initTestCase(); - void testSecret(); + void testParseSecret(); void testBase32(); void testTotpCode(); };