From 85f652290b48a2178fef65678889313d838c0f22 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Thu, 5 Oct 2017 18:04:41 -0400 Subject: [PATCH 01/17] Add signtool function (Windows Only) to release-tool script --- release-tool | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/release-tool b/release-tool index a1d23b405..10c6a14c3 100755 --- a/release-tool +++ b/release-tool @@ -119,9 +119,11 @@ EOF Sign previously compiled release packages Options: - -f, --files Files to sign (required) - -g, --gpg-key GPG key used to sign the files (default: '${GPG_KEY}') - -h, --help Show this help + -f, --files Files to sign (required) + -g, --gpg-key GPG key used to sign the files (default: '${GPG_KEY}') + --signtool Specify the signtool executable (default: 'signtool') + --signtool-key Provide a key to be used with signtool (for Windows EXE) + -h, --help Show this help EOF fi } @@ -546,10 +548,10 @@ build() { checkWorkingTreeClean OUTPUT_DIR="$(realpath "$OUTPUT_DIR")" - + logInfo "Checking out release tag '${TAG_NAME}'..." git checkout "$TAG_NAME" - + logInfo "Creating output directory..." mkdir -p "$OUTPUT_DIR" @@ -663,6 +665,8 @@ build() { # ----------------------------------------------------------------------- sign() { SIGN_FILES=() + SIGNTOOL="signtool" + SIGNTOOL_KEY="" while [ $# -ge 1 ]; do local arg="$1" @@ -676,6 +680,14 @@ sign() { -g|--gpg-key) GPG_KEY="$2" shift ;; + + --signtool) + SIGNTOOL="$2" + shift ;; + + --signtool-key) + SIGNTOOL_KEY="$2" + shift ;; -h|--help) printUsage "sign" @@ -694,13 +706,30 @@ sign() { printUsage "sign" exit 1 fi + + if [[ -n "$SIGNTOOL_KEY" && ! -f "$SIGNTOOL_KEY" ]]; then + exitError "Signtool Key was not found!" + elif [[ -f "$SIGNTOOL_KEY" && ! -x $(command -v "${SIGNTOOL}") ]]; then + exitError "signtool program not found on PATH!" + fi for f in "${SIGN_FILES[@]}"; do if [ ! -f "$f" ]; then exitError "File '${f}' does not exist!" fi + + if [[ -n "$SIGNTOOL_KEY" && ${f: -4} == '.exe' ]]; then + logInfo "Signing file '${f}' using signtool...\n" + read -s -p "Signtool Key Password: " password + echo + "${SIGNTOOL}" sign -f "${SIGNTOOL_KEY}" -p ${password} -v -t http://timestamp.comodoca.com/authenticode ${f} + + if [ 0 -ne $? ]; then + exitError "Signing failed!" + fi + fi - logInfo "Signing file '${f}'..." + logInfo "Signing file '${f}' using release key..." gpg --output "${f}.sig" --armor --local-user "$GPG_KEY" --detach-sig "$f" if [ 0 -ne $? ]; then From 19eb6a8a60afed2da5733a2eef47de80009f824f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20E=2E=20Garc=C3=ADa?= Date: Sat, 14 Oct 2017 22:54:20 -0600 Subject: [PATCH 02/17] Add new Base32 implementation --- COPYING | 9 -- LICENSE.APACHE-2.0 | 201 ------------------------------ src/CMakeLists.txt | 5 +- src/core/Base32.cpp | 285 +++++++++++++++++++++++++++++++++++++++++++ src/core/Base32.h | 42 +++++++ src/core/Optional.h | 86 +++++++++++++ src/totp/base32.cpp | 68 ----------- src/totp/base32.h | 34 ------ src/totp/totp.cpp | 57 ++++++--- src/totp/totp.h | 11 +- tests/CMakeLists.txt | 3 + tests/TestBase32.cpp | 278 +++++++++++++++++++++++++++++++++++++++++ tests/TestBase32.h | 37 ++++++ tests/TestTotp.cpp | 31 +---- tests/TestTotp.h | 1 - 15 files changed, 789 insertions(+), 359 deletions(-) delete mode 100644 LICENSE.APACHE-2.0 create mode 100644 src/core/Base32.cpp create mode 100644 src/core/Base32.h create mode 100644 src/core/Optional.h delete mode 100644 src/totp/base32.cpp delete mode 100644 src/totp/base32.h create mode 100644 tests/TestBase32.cpp create mode 100644 tests/TestBase32.h diff --git a/COPYING b/COPYING index 481aaf726..403e4564b 100644 --- a/COPYING +++ b/COPYING @@ -214,10 +214,6 @@ Files: share/icons/database/C65_W.png Copyright: none License: public-domain -Files: src/crypto/salsa20/* -Copyright: none -License: public-domain - Files: src/streams/qtiocompressor.* src/streams/QtIOCompressor tests/modeltest.* @@ -241,8 +237,3 @@ Files: src/gui/KMessageWidget.h Copyright: 2011 Aurélien Gâteau 2014 Dominik Haumann License: LGPL-2.1 - -Files: src/totp/base32.cpp - src/totp/base32.h -Copyright: 2010 Google Inc. -License: Apache 2.0 \ No newline at end of file diff --git a/LICENSE.APACHE-2.0 b/LICENSE.APACHE-2.0 deleted file mode 100644 index 9c8f3ea08..000000000 --- a/LICENSE.APACHE-2.0 +++ /dev/null @@ -1,201 +0,0 @@ - Apache License - Version 2.0, January 2004 - http://www.apache.org/licenses/ - - TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION - - 1. Definitions. - - "License" shall mean the terms and conditions for use, reproduction, - and distribution as defined by Sections 1 through 9 of this document. - - "Licensor" shall mean the copyright owner or entity authorized by - the copyright owner that is granting the License. - - "Legal Entity" shall mean the union of the acting entity and all - other entities that control, are controlled by, or are under common - control with that entity. For the purposes of this definition, - "control" means (i) the power, direct or indirect, to cause the - direction or management of such entity, whether by contract or - otherwise, or (ii) ownership of fifty percent (50%) or more of the - outstanding shares, or (iii) beneficial ownership of such entity. - - "You" (or "Your") shall mean an individual or Legal Entity - exercising permissions granted by this License. - - "Source" form shall mean the preferred form for making modifications, - including but not limited to software source code, documentation - source, and configuration files. - - "Object" form shall mean any form resulting from mechanical - transformation or translation of a Source form, including but - not limited to compiled object code, generated documentation, - and conversions to other media types. - - "Work" shall mean the work of authorship, whether in Source or - Object form, made available under the License, as indicated by a - copyright notice that is included in or attached to the work - (an example is provided in the Appendix below). - - "Derivative Works" shall mean any work, whether in Source or Object - form, that is based on (or derived from) the Work and for which the - editorial revisions, annotations, elaborations, or other modifications - represent, as a whole, an original work of authorship. For the purposes - of this License, Derivative Works shall not include works that remain - separable from, or merely link (or bind by name) to the interfaces of, - the Work and Derivative Works thereof. - - "Contribution" shall mean any work of authorship, including - the original version of the Work and any modifications or additions - to that Work or Derivative Works thereof, that is intentionally - submitted to Licensor for inclusion in the Work by the copyright owner - or by an individual or Legal Entity authorized to submit on behalf of - the copyright owner. For the purposes of this definition, "submitted" - means any form of electronic, verbal, or written communication sent - to the Licensor or its representatives, including but not limited to - communication on electronic mailing lists, source code control systems, - and issue tracking systems that are managed by, or on behalf of, the - Licensor for the purpose of discussing and improving the Work, but - excluding communication that is conspicuously marked or otherwise - designated in writing by the copyright owner as "Not a Contribution." - - "Contributor" shall mean Licensor and any individual or Legal Entity - on behalf of whom a Contribution has been received by Licensor and - subsequently incorporated within the Work. - - 2. Grant of Copyright License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - copyright license to reproduce, prepare Derivative Works of, - publicly display, publicly perform, sublicense, and distribute the - Work and such Derivative Works in Source or Object form. - - 3. Grant of Patent License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - (except as stated in this section) patent license to make, have made, - use, offer to sell, sell, import, and otherwise transfer the Work, - where such license applies only to those patent claims licensable - by such Contributor that are necessarily infringed by their - Contribution(s) alone or by combination of their Contribution(s) - with the Work to which such Contribution(s) was submitted. If You - institute patent litigation against any entity (including a - cross-claim or counterclaim in a lawsuit) alleging that the Work - or a Contribution incorporated within the Work constitutes direct - or contributory patent infringement, then any patent licenses - granted to You under this License for that Work shall terminate - as of the date such litigation is filed. - - 4. Redistribution. You may reproduce and distribute copies of the - Work or Derivative Works thereof in any medium, with or without - modifications, and in Source or Object form, provided that You - meet the following conditions: - - (a) You must give any other recipients of the Work or - Derivative Works a copy of this License; and - - (b) You must cause any modified files to carry prominent notices - stating that You changed the files; and - - (c) You must retain, in the Source form of any Derivative Works - that You distribute, all copyright, patent, trademark, and - attribution notices from the Source form of the Work, - excluding those notices that do not pertain to any part of - the Derivative Works; and - - (d) If the Work includes a "NOTICE" text file as part of its - distribution, then any Derivative Works that You distribute must - include a readable copy of the attribution notices contained - within such NOTICE file, excluding those notices that do not - pertain to any part of the Derivative Works, in at least one - of the following places: within a NOTICE text file distributed - as part of the Derivative Works; within the Source form or - documentation, if provided along with the Derivative Works; or, - within a display generated by the Derivative Works, if and - wherever such third-party notices normally appear. The contents - of the NOTICE file are for informational purposes only and - do not modify the License. You may add Your own attribution - notices within Derivative Works that You distribute, alongside - or as an addendum to the NOTICE text from the Work, provided - that such additional attribution notices cannot be construed - as modifying the License. - - You may add Your own copyright statement to Your modifications and - may provide additional or different license terms and conditions - for use, reproduction, or distribution of Your modifications, or - for any such Derivative Works as a whole, provided Your use, - reproduction, and distribution of the Work otherwise complies with - the conditions stated in this License. - - 5. Submission of Contributions. Unless You explicitly state otherwise, - any Contribution intentionally submitted for inclusion in the Work - by You to the Licensor shall be under the terms and conditions of - this License, without any additional terms or conditions. - Notwithstanding the above, nothing herein shall supersede or modify - the terms of any separate license agreement you may have executed - with Licensor regarding such Contributions. - - 6. Trademarks. This License does not grant permission to use the trade - names, trademarks, service marks, or product names of the Licensor, - except as required for reasonable and customary use in describing the - origin of the Work and reproducing the content of the NOTICE file. - - 7. Disclaimer of Warranty. Unless required by applicable law or - agreed to in writing, Licensor provides the Work (and each - Contributor provides its Contributions) on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or - implied, including, without limitation, any warranties or conditions - of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A - PARTICULAR PURPOSE. You are solely responsible for determining the - appropriateness of using or redistributing the Work and assume any - risks associated with Your exercise of permissions under this License. - - 8. Limitation of Liability. In no event and under no legal theory, - whether in tort (including negligence), contract, or otherwise, - unless required by applicable law (such as deliberate and grossly - negligent acts) or agreed to in writing, shall any Contributor be - liable to You for damages, including any direct, indirect, special, - incidental, or consequential damages of any character arising as a - result of this License or out of the use or inability to use the - Work (including but not limited to damages for loss of goodwill, - work stoppage, computer failure or malfunction, or any and all - other commercial damages or losses), even if such Contributor - has been advised of the possibility of such damages. - - 9. Accepting Warranty or Additional Liability. While redistributing - the Work or Derivative Works thereof, You may choose to offer, - and charge a fee for, acceptance of support, warranty, indemnity, - or other liability obligations and/or rights consistent with this - License. However, in accepting such obligations, You may act only - on Your own behalf and on Your sole responsibility, not on behalf - of any other Contributor, and only if You agree to indemnify, - defend, and hold each Contributor harmless for any liability - incurred by, or claims asserted against, such Contributor by reason - of your accepting any such warranty or additional liability. - - END OF TERMS AND CONDITIONS - - APPENDIX: How to apply the Apache License to your work. - - To apply the Apache License to your work, attach the following - boilerplate notice, with the fields enclosed by brackets "{}" - replaced with your own identifying information. (Don't include - the brackets!) The text should be enclosed in the appropriate - comment syntax for the file format. We also recommend that a - file or class name and description of purpose be included on the - same "printed page" as the copyright notice for easier - identification within third-party archives. - - Copyright {yyyy} {name of copyright owner} - - 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. \ No newline at end of file diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 791685576..28170b162 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -59,6 +59,9 @@ set(keepassx_SOURCES core/Tools.cpp core/Translator.cpp core/Uuid.cpp + core/Base32.h + core/Base32.cpp + core/Optional.h cli/PasswordInput.cpp cli/PasswordInput.h crypto/Crypto.cpp @@ -138,8 +141,6 @@ 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/Base32.cpp b/src/core/Base32.cpp new file mode 100644 index 000000000..8b2faf629 --- /dev/null +++ b/src/core/Base32.cpp @@ -0,0 +1,285 @@ +/* + * Copyright (C) 2017 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +/* Conforms to RFC 4648. For details, see: https://tools.ietf.org/html/rfc4648 + * Use the functions Base32::addPadding/1, Base32::removePadding/1 or + * Base32::sanitizeInput/1 to fix input or output for a particular + * applications (e.g. to use with Google Authenticator). + */ + +#include "Base32.h" + +constexpr quint64 MASK_40BIT = quint64(0xF8) << 32; +constexpr quint64 MASK_35BIT = quint64(0x7C0000000); +constexpr quint64 MASK_25BIT = quint64(0x1F00000); +constexpr quint64 MASK_20BIT = quint64(0xF8000); +constexpr quint64 MASK_10BIT = quint64(0x3E0); + +constexpr char alphabet[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567"; +constexpr quint8 ALPH_POS_2 = 26; + +constexpr quint8 ASCII_2 = static_cast('2'); +constexpr quint8 ASCII_7 = static_cast('7'); +constexpr quint8 ASCII_A = static_cast('A'); +constexpr quint8 ASCII_Z = static_cast('Z'); +constexpr quint8 ASCII_a = static_cast('a'); +constexpr quint8 ASCII_z = static_cast('z'); +constexpr quint8 ASCII_EQ = static_cast('='); + +Optional Base32::decode(const QByteArray& encodedData) +{ + if (encodedData.size() <= 0) { + return Optional(""); + } + + if (encodedData.size() % 8 != 0) { + return Optional(); + } + + int nPads = 0; + for (int i = -1; i > -7; --i) { + if ('=' == encodedData[encodedData.size() + i]) + ++nPads; + } + + int specialOffset; + int nSpecialBytes; + + switch (nPads) { // in {0, 1, 3, 4, 6} + case 1: + nSpecialBytes = 4; + specialOffset = 3; + break; + case 3: + nSpecialBytes = 3; + specialOffset = 1; + break; + case 4: + nSpecialBytes = 2; + specialOffset = 4; + break; + case 6: + nSpecialBytes = 1; + specialOffset = 2; + break; + default: + nSpecialBytes = 0; + specialOffset = 0; + } + + Q_ASSERT(encodedData.size() > 0); + const int nQuanta = encodedData.size() / 8; + const int nBytes = (nQuanta - 1) * 5 + nSpecialBytes; + + QByteArray data(nBytes, Qt::Uninitialized); + + int i = 0; + int o = 0; + + while (i < encodedData.size()) { + quint64 quantum = 0; + int nQuantumBytes = 5; + + for (int n = 0; n < 8; n++) { + quint8 ch = static_cast(encodedData[i++]); + if ((ASCII_A <= ch && ch <= ASCII_Z) || (ASCII_a <= ch && ch <= ASCII_z)) { + ch -= ASCII_A; + if (ch >= ALPH_POS_2) + ch -= ASCII_a - ASCII_A; + } else { + if (ASCII_2 <= ch && ch <= ASCII_7) { + ch -= ASCII_2; + ch += ALPH_POS_2; + } else { + if (ASCII_EQ == ch) { + if (i == encodedData.size()) { + // finished with special quantum + quantum >>= specialOffset; + nQuantumBytes = nSpecialBytes; + } + continue; + } else { + // illegal character + return Optional(); + } + } + } + + quantum <<= 5; + quantum |= ch; + } + + const int offset = (nQuantumBytes - 1) * 8; + quint64 mask = quint64(0xFF) << offset; + for (int n = offset; n >= 0; n -= 8) { + char c = static_cast((quantum & mask) >> n); + data[o++] = c; + mask >>= 8; + } + } + + return Optional(data); +} + +QByteArray Base32::encode(const QByteArray& data) +{ + if (data.size() < 1) { + return QByteArray(); + } + + const int nBits = data.size() * 8; + const int rBits = nBits % 40; // in {0, 8, 16, 24, 32} + const int nQuanta = nBits / 40 + (rBits > 0 ? 1 : 0); + QByteArray encodedData(nQuanta * 8, Qt::Uninitialized); + + int i = 0; + int o = 0; + int n; + quint64 mask; + quint64 quantum; + + // 40-bits of input per input group + while (i + 5 <= data.size()) { + quantum = 0; + for (n = 32; n >= 0; n -= 8) { + quantum |= (static_cast(data[i++]) << n); + } + + mask = MASK_40BIT; + int index; + for (n = 35; n >= 0; n -= 5) { + index = (quantum & mask) >> n; + encodedData[o++] = alphabet[index]; + mask >>= 5; + } + } + + // < 40-bits of input at final input group + if (i < data.size()) { + Q_ASSERT(rBits > 0); + quantum = 0; + for (n = rBits - 8; n >= 0; n -= 8) + quantum |= static_cast(data[i++]) << n; + + switch (rBits) { + case 8: // expand to 10 bits + quantum <<= 2; + mask = MASK_10BIT; + n = 5; + break; + case 16: // expand to 20 bits + quantum <<= 4; + mask = MASK_20BIT; + n = 15; + break; + case 24: // expand to 25 bits + quantum <<= 1; + mask = MASK_25BIT; + n = 20; + break; + default: // expand to 35 bits + Q_ASSERT(rBits == 32); + quantum <<= 3; + mask = MASK_35BIT; + n = 30; + } + + while (n >= 0) { + int index = (quantum & mask) >> n; + encodedData[o++] = alphabet[index]; + mask >>= 5; + n -= 5; + } + + // add pad characters + while (o < encodedData.size()) + encodedData[o++] = '='; + } + + Q_ASSERT(encodedData.size() == o); + return encodedData; +} + +QByteArray Base32::addPadding(const QByteArray& encodedData) +{ + if (encodedData.size() <= 0 || encodedData.size() % 8 == 0) { + return encodedData; + } + + const int rBytes = encodedData.size() % 8; + // rBytes must be a member of {2, 4, 5, 7} + if (1 == rBytes || 3 == rBytes || 6 == rBytes) { + return encodedData; + } + + QByteArray newEncodedData(encodedData); + for (int nPads = 8 - rBytes; nPads > 0; --nPads) { + newEncodedData.append('='); + } + + return newEncodedData; +} + +QByteArray Base32::removePadding(const QByteArray& encodedData) +{ + if (encodedData.size() <= 0 || encodedData.size() % 8 != 0) { + return encodedData; // return same bad input + } + + int nPads = 0; + for (int i = -1; i > -7; --i) { + if ('=' == encodedData[encodedData.size() + i]) { + ++nPads; + } + } + + QByteArray newEncodedData(encodedData); + newEncodedData.remove(encodedData.size() - nPads, nPads); + newEncodedData.resize(encodedData.size() - nPads); + + return newEncodedData; +} + +QByteArray Base32::sanitizeInput(const QByteArray& encodedData) +{ + if (encodedData.size() <= 0) { + return encodedData; + } + + QByteArray newEncodedData(encodedData.size(), Qt::Uninitialized); + int i = 0; + for (auto ch : encodedData) { + switch (ch) { + case '0': + newEncodedData[i++] = 'O'; + break; + case '1': + newEncodedData[i++] = 'L'; + break; + case '8': + newEncodedData[i++] = 'B'; + break; + default: + if (('A' <= ch && ch <= 'Z') || ('a' <= ch && ch <= 'z') || ('2' <= ch && ch <= '7')) { + newEncodedData[i++] = ch; + } + } + } + newEncodedData.resize(i); + + return addPadding(newEncodedData); +} diff --git a/src/core/Base32.h b/src/core/Base32.h new file mode 100644 index 000000000..048828974 --- /dev/null +++ b/src/core/Base32.h @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2017 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +/* Conforms to RFC 4648. For details, see: https://tools.ietf.org/html/rfc4648 + * Use the functions Base32::addPadding/1, Base32::removePadding/1 or + * Base32::sanitizeInput/1 to fix input or output for a particular + * applications (e.g. to use with Google Authenticator). + */ + +#ifndef BASE32_H +#define BASE32_H + +#include "Optional.h" +#include +#include + +class Base32 +{ +public: + Base32() = default; + Q_REQUIRED_RESULT static Optional decode(const QByteArray&); + Q_REQUIRED_RESULT static QByteArray encode(const QByteArray&); + Q_REQUIRED_RESULT static QByteArray addPadding(const QByteArray&); + Q_REQUIRED_RESULT static QByteArray removePadding(const QByteArray&); + Q_REQUIRED_RESULT static QByteArray sanitizeInput(const QByteArray&); +}; + +#endif // BASE32_H diff --git a/src/core/Optional.h b/src/core/Optional.h new file mode 100644 index 000000000..0721daa3d --- /dev/null +++ b/src/core/Optional.h @@ -0,0 +1,86 @@ +/* + * Copyright (C) 2017 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef OPTIONAL_H +#define OPTIONAL_H + +/* + * This utility class is for providing basic support for an option type. + * It can be replaced by std::optional (C++17) or + * std::experimental::optional (C++11) when they become fully supported + * by all the main compiler toolchains. + */ + +template class Optional +{ +public: + // None + Optional() + : m_hasValue(false) + , m_value(){}; + + // Some T + Optional(const T& value) + : m_hasValue(true) + , m_value(value){}; + + // Copy + Optional(const Optional& other) + : m_hasValue(other.m_hasValue) + , m_value(other.m_value){}; + + const Optional& operator=(const Optional& other) + { + m_hasValue = other.m_hasValue; + m_value = other.m_value; + return *this; + } + + bool operator==(const Optional& other) const + { + if (m_hasValue) + return other.m_hasValue && m_value == other.m_value; + else + return !other.m_hasValue; + } + + bool operator!=(const Optional& other) const + { + return !(*this == other); + } + + bool hasValue() const + { + return m_hasValue; + } + + T valueOr(const T& other) const + { + return m_hasValue ? m_value : other; + } + + Optional static makeOptional(const T& value) + { + return Optional(value); + } + +private: + bool m_hasValue; + T m_value; +}; + +#endif // OPTIONAL_H diff --git a/src/totp/base32.cpp b/src/totp/base32.cpp deleted file mode 100644 index 4c81cb491..000000000 --- a/src/totp/base32.cpp +++ /dev/null @@ -1,68 +0,0 @@ -// Base32 implementation -// Source: https://github.com/google/google-authenticator-libpam/blob/master/src/base32.c -// -// Copyright 2010 Google Inc. -// Author: Markus Gutschke -// Modifications Copyright 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 deleted file mode 100644 index 75343fa43..000000000 --- a/src/totp/base32.h +++ /dev/null @@ -1,34 +0,0 @@ -// Base32 implementation -// Source: https://github.com/google/google-authenticator-libpam/blob/master/src/base32.h -// -// Copyright 2010 Google Inc. -// Author: Markus Gutschke -// Modifications Copyright 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 51af0e086..170ac2da7 100644 --- a/src/totp/totp.cpp +++ b/src/totp/totp.cpp @@ -17,16 +17,15 @@ */ #include "totp.h" -#include "base32.h" -#include -#include -#include -#include +#include "core/Base32.h" #include +#include #include +#include #include #include - +#include +#include const quint8 QTotp::defaultStep = 30; const quint8 QTotp::defaultDigits = 6; @@ -35,7 +34,7 @@ QTotp::QTotp() { } -QString QTotp::parseOtpString(QString key, quint8 &digits, quint8 &step) +QString QTotp::parseOtpString(QString key, quint8& digits, quint8& step) { QUrl url(key); @@ -58,7 +57,6 @@ QString QTotp::parseOtpString(QString key, quint8 &digits, quint8 &step) step = q_step; } - } else { // Compatibility with "KeeOtp" plugin string format QRegExp rx("key=(.+)", Qt::CaseInsensitive, QRegExp::RegExp); @@ -93,30 +91,53 @@ QString QTotp::parseOtpString(QString key, quint8 &digits, quint8 &step) return seed; } -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 = Base32::base32_decode(key); - if (secret.isEmpty()) { + Optional secret = Base32::decode(Base32::sanitizeInput(key)); + if (!secret.hasValue()) { return "Invalid TOTP secret key"; } QMessageAuthenticationCode code(QCryptographicHash::Sha1); - code.setKey(secret); + code.setKey(secret.valueOr("")); code.addData(QByteArray(reinterpret_cast(¤t), sizeof(current))); QByteArray hmac = code.result(); int offset = (hmac[hmac.length() - 1] & 0xf); - int binary = - ((hmac[offset] & 0x7f) << 24) - | ((hmac[offset + 1] & 0xff) << 16) - | ((hmac[offset + 2] & 0xff) << 8) - | (hmac[offset + 3] & 0xff); + int binary = ((hmac[offset] & 0x7f) << 24) | ((hmac[offset + 1] & 0xff) << 16) | ((hmac[offset + 2] & 0xff) << 8) | + (hmac[offset + 3] & 0xff); quint32 digitsPower = pow(10, numDigits); quint64 password = binary % digitsPower; return QString("%1").arg(password, numDigits, 10, QChar('0')); } + +// See: https://github.com/google/google-authenticator/wiki/Key-Uri-Format +QUrl QTotp::generateOtpString(const QString& secret, + const QString& type, + const QString& issuer, + const QString& username, + const QString& algorithm, + const quint8& digits, + const quint8& step) +{ + QUrl keyUri; + keyUri.setScheme("otpauth"); + keyUri.setHost(type); + keyUri.setPath(QString("/%1:%2").arg(issuer).arg(username)); + QUrlQuery parameters; + parameters.addQueryItem("secret", secret); + parameters.addQueryItem("issuer", issuer); + parameters.addQueryItem("algorithm", algorithm); + parameters.addQueryItem("digits", QString::number(digits)); + parameters.addQueryItem("period", QString::number(step)); + keyUri.setQuery(parameters); + + return keyUri; +} diff --git a/src/totp/totp.h b/src/totp/totp.h index 642b4f9a3..d5d8aa679 100644 --- a/src/totp/totp.h +++ b/src/totp/totp.h @@ -21,12 +21,21 @@ #include +class QUrl; + class QTotp { public: QTotp(); - static QString parseOtpString(QString rawSecret, quint8 &digits, quint8 &step); + static QString parseOtpString(QString rawSecret, quint8& digits, quint8& step); static QString generateTotp(const QByteArray key, quint64 time, const quint8 numDigits, const quint8 step); + static QUrl generateOtpString(const QString& secret, + const QString& type, + const QString& issuer, + const QString& username, + const QString& algorithm, + const quint8& digits, + const quint8& step); static const quint8 defaultStep; static const quint8 defaultDigits; }; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2a420270a..3f003f01c 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -161,6 +161,9 @@ add_unit_test(NAME testentry SOURCES TestEntry.cpp add_unit_test(NAME testtotp SOURCES TestTotp.cpp LIBS ${TEST_LIBRARIES}) +add_unit_test(NAME testbase32 SOURCES TestBase32.cpp + LIBS ${TEST_LIBRARIES}) + add_unit_test(NAME testcsvparser SOURCES TestCsvParser.cpp LIBS ${TEST_LIBRARIES}) diff --git a/tests/TestBase32.cpp b/tests/TestBase32.cpp new file mode 100644 index 000000000..3d829d96b --- /dev/null +++ b/tests/TestBase32.cpp @@ -0,0 +1,278 @@ +/* + * Copyright (C) 2017 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "TestBase32.h" +#include "core/Base32.h" +#include + +QTEST_GUILESS_MAIN(TestBase32) + +void TestBase32::testDecode() +{ + // 3 quanta, all upper case + padding + QByteArray encodedData = "JBSWY3DPEB3W64TMMQXC4LQ="; + auto data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("Hello world...")); + + // 4 quanta, all upper case + encodedData = "GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ"; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("12345678901234567890")); + + // 4 quanta, all lower case + encodedData = "gezdgnbvgy3tqojqgezdgnbvgy3tqojq"; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("12345678901234567890")); + + // 4 quanta, mixed upper and lower case + encodedData = "Gezdgnbvgy3tQojqgezdGnbvgy3tQojQ"; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("12345678901234567890")); + + // 1 pad characters + encodedData = "ORSXG5A="; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("test")); + + // 3 pad characters + encodedData = "L5PV6==="; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("___")); + + // 4 pad characters + encodedData = "MZXW6IDCMFZA===="; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("foo bar")); + + // six pad characters + encodedData = "MZXW6YTBOI======"; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("foobar")); + + encodedData = "IA======"; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("@")); + + // error: illegal character + encodedData = "1MZXW6YTBOI====="; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("ERROR")); + + // error: missing pad character + encodedData = "MZXW6YTBOI====="; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("ERROR")); + + // RFC 4648 test vectors + encodedData = ""; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("")); + + encodedData = "MY======"; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("f")); + + encodedData = "MZXQ===="; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("fo")); + + encodedData = "MZXW6==="; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("foo")); + + encodedData = "MZXW6YQ="; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("foob")); + + encodedData = "MZXW6YTB"; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("fooba")); + + encodedData = "MZXW6YTBOI======"; + data = Base32::decode(encodedData); + QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("foobar")); +} + +void TestBase32::testEncode() +{ + QByteArray data = "Hello world..."; + QByteArray encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("JBSWY3DPEB3W64TMMQXC4LQ=")); + + data = "12345678901234567890"; + encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ")); + + data = "012345678901234567890"; + encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("GAYTEMZUGU3DOOBZGAYTEMZUGU3DOOBZGA======")); + + data = "test"; + encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("ORSXG5A=")); + + data = "___"; + encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("L5PV6===")); + + data = "foo bar"; + encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("MZXW6IDCMFZA====")); + + data = "@"; + encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("IA======")); + + // RFC 4648 test vectors + data = ""; + encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("")); + + data = "f"; + encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("MY======")); + + data = "fo"; + encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("MZXQ====")); + + data = "foo"; + encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("MZXW6===")); + + data = "foob"; + encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("MZXW6YQ=")); + + data = "fooba"; + encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("MZXW6YTB")); + + data = "foobar"; + encodedData = Base32::encode(data); + QCOMPARE(encodedData, QByteArray("MZXW6YTBOI======")); +} + +void TestBase32::testAddPadding() +{ + // Empty. Invalid, returns input. + QByteArray data = ""; + QByteArray paddedData = Base32::addPadding(data); + QCOMPARE(paddedData, data); + + // One byte of encoded data. Invalid, returns input. + data = "B"; + paddedData = Base32::addPadding(data); + QCOMPARE(paddedData, data); + + // Two bytes of encoded data. + data = "BB"; + paddedData = Base32::addPadding(data); + QCOMPARE(paddedData, QByteArray("BB======")); + + // Three bytes of encoded data. Invalid, returns input. + data = "BBB"; + paddedData = Base32::addPadding(data); + QCOMPARE(paddedData, data); + + // Four bytes of encoded data. + data = "BBBB"; + paddedData = Base32::addPadding(data); + QCOMPARE(paddedData, QByteArray("BBBB====")); + + // Five bytes of encoded data. + data = "BBBBB"; + paddedData = Base32::addPadding(data); + QCOMPARE(paddedData, QByteArray("BBBBB===")); + + // Six bytes of encoded data. Invalid, returns input. + data = "BBBBBB"; + paddedData = Base32::addPadding(data); + QCOMPARE(paddedData, data); + + // Seven bytes of encoded data. + data = "BBBBBBB"; + paddedData = Base32::addPadding(data); + QCOMPARE(paddedData, QByteArray("BBBBBBB=")); + + // Eight bytes of encoded data. Valid, but returns same as input. + data = "BBBBBBBB"; + paddedData = Base32::addPadding(data); + QCOMPARE(paddedData, data); + + // More than eight bytes (8+5). + data = "AAAAAAAABBBBB"; + paddedData = Base32::addPadding(data); + QCOMPARE(paddedData, QByteArray("AAAAAAAABBBBB===")); +} + +void TestBase32::testRemovePadding() +{ + QByteArray data = ""; + QByteArray unpaddedData = Base32::removePadding(data); + QCOMPARE(unpaddedData, data); + + data = "AAAAAAAABB======"; + unpaddedData = Base32::removePadding(data); + QCOMPARE(unpaddedData, QByteArray("AAAAAAAABB")); + + data = "BBBB===="; + unpaddedData = Base32::removePadding(data); + QCOMPARE(unpaddedData, QByteArray("BBBB")); + + data = "AAAAAAAABBBBB==="; + unpaddedData = Base32::removePadding(data); + QCOMPARE(unpaddedData, QByteArray("AAAAAAAABBBBB")); + + data = "BBBBBBB="; + unpaddedData = Base32::removePadding(data); + QCOMPARE(unpaddedData, QByteArray("BBBBBBB")); + + // Invalid: 7 bytes of data. Returns same as input. + data = "IIIIIII"; + unpaddedData = Base32::removePadding(data); + QCOMPARE(unpaddedData, data); + + // Invalid: more padding than necessary. Returns same as input. + data = "AAAAAAAABBBB====="; + unpaddedData = Base32::removePadding(data); + QCOMPARE(unpaddedData, data); +} + +void TestBase32::testSanitizeInput() +{ + // sanitize input (white space + missing padding) + QByteArray encodedData = "JBSW Y3DP EB3W 64TM MQXC 4LQA"; + auto data = Base32::decode(Base32::sanitizeInput(encodedData)); + QCOMPARE(QString::fromLatin1(data.valueOr("ERRROR")), QString("Hello world...")); + + // sanitize input (typo + missing padding) + encodedData = "J8SWY3DPE83W64TMMQXC4LQA"; + data = Base32::decode(Base32::sanitizeInput(encodedData)); + QCOMPARE(QString::fromLatin1(data.valueOr("ERRROR")), QString("Hello world...")); + + // sanitize input (other illegal characters) + encodedData = "J8SWY3D[PE83W64TMMQ]XC!4LQA"; + data = Base32::decode(Base32::sanitizeInput(encodedData)); + QCOMPARE(QString::fromLatin1(data.valueOr("ERRROR")), QString("Hello world...")); + + // sanitize input (NUL character) + encodedData = "J8SWY3DPE83W64TMMQXC4LQA"; + encodedData.insert(3, '\0'); + data = Base32::decode(Base32::sanitizeInput(encodedData)); + QCOMPARE(QString::fromLatin1(data.valueOr("ERRROR")), QString("Hello world...")); +} diff --git a/tests/TestBase32.h b/tests/TestBase32.h new file mode 100644 index 000000000..cf7cf092c --- /dev/null +++ b/tests/TestBase32.h @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2017 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef KEEPASSX_TESTBASE32_H +#define KEEPASSX_TESTBASE32_H + +#include + +class Base32; + +class TestBase32 : public QObject +{ + Q_OBJECT + +private slots: + void testEncode(); + void testDecode(); + void testAddPadding(); + void testRemovePadding(); + void testSanitizeInput(); +}; + +#endif // KEEPASSX_TESTBASE32_H diff --git a/tests/TestTotp.cpp b/tests/TestTotp.cpp index e22c2567e..48ff88144 100644 --- a/tests/TestTotp.cpp +++ b/tests/TestTotp.cpp @@ -18,15 +18,14 @@ #include "TestTotp.h" -#include -#include #include -#include +#include #include +#include +#include #include "crypto/Crypto.h" #include "totp/totp.h" -#include "totp/base32.h" QTEST_GUILESS_MAIN(TestTotp) @@ -35,12 +34,13 @@ void TestTotp::initTestCase() QVERIFY(Crypto::init()); } - void TestTotp::testParseSecret() { quint8 digits = 0; quint8 step = 0; - QString secret = "otpauth://totp/ACME%20Co:john@example.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=SHA1&digits=6&period=30"; + QString secret = "otpauth://totp/" + "ACME%20Co:john@example.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=" + "SHA1&digits=6&period=30"; QCOMPARE(QTotp::parseOtpString(secret, digits, step), QString("HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ")); QCOMPARE(digits, quint8(6)); QCOMPARE(step, quint8(30)); @@ -60,25 +60,6 @@ void TestTotp::testParseSecret() QCOMPARE(step, quint8(30)); } -void TestTotp::testBase32() -{ - QByteArray key = QString("JBSW Y3DP EB3W 64TM MQXC 4LQA").toLatin1(); - QByteArray secret = Base32::base32_decode(key); - QCOMPARE(QString::fromLatin1(secret), QString("Hello world...")); - - key = QString("gezdgnbvgy3tqojqgezdgnbvgy3tqojq").toLatin1(); - secret = Base32::base32_decode(key); - QCOMPARE(QString::fromLatin1(secret), QString("12345678901234567890")); - - key = QString("ORSXG5A=").toLatin1(); - secret = Base32::base32_decode(key); - QCOMPARE(QString::fromLatin1(secret), QString("test")); - - key = QString("MZXW6YTBOI======").toLatin1(); - secret = Base32::base32_decode(key); - QCOMPARE(QString::fromLatin1(secret), QString("foobar")); -} - void TestTotp::testTotpCode() { // Test vectors from RFC 6238 diff --git a/tests/TestTotp.h b/tests/TestTotp.h index d197294dd..785a9f522 100644 --- a/tests/TestTotp.h +++ b/tests/TestTotp.h @@ -30,7 +30,6 @@ class TestTotp : public QObject private slots: void initTestCase(); void testParseSecret(); - void testBase32(); void testTotpCode(); }; From 95b0ad15ef5f1f33d9b27f8eec47fa2fee81f1be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20E=2E=20Garc=C3=ADa?= Date: Thu, 19 Oct 2017 13:52:23 -0600 Subject: [PATCH 03/17] Fix code style --- src/core/Base32.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/Base32.cpp b/src/core/Base32.cpp index 8b2faf629..84dca5ec3 100644 --- a/src/core/Base32.cpp +++ b/src/core/Base32.cpp @@ -206,8 +206,9 @@ QByteArray Base32::encode(const QByteArray& data) } // add pad characters - while (o < encodedData.size()) + while (o < encodedData.size()) { encodedData[o++] = '='; + } } Q_ASSERT(encodedData.size() == o); From d21ae0f94ab5af5900ade0776d52a6e83ae614e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20E=2E=20Garc=C3=ADa?= Date: Fri, 20 Oct 2017 14:54:31 -0600 Subject: [PATCH 04/17] Fix for loop's variable increment style --- src/core/Base32.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/Base32.cpp b/src/core/Base32.cpp index 84dca5ec3..5bd20901d 100644 --- a/src/core/Base32.cpp +++ b/src/core/Base32.cpp @@ -94,7 +94,7 @@ Optional Base32::decode(const QByteArray& encodedData) quint64 quantum = 0; int nQuantumBytes = 5; - for (int n = 0; n < 8; n++) { + for (int n = 0; n < 8; ++n) { quint8 ch = static_cast(encodedData[i++]); if ((ASCII_A <= ch && ch <= ASCII_Z) || (ASCII_a <= ch && ch <= ASCII_z)) { ch -= ASCII_A; From 905e104ba24c8322832c660764c779b915dd65bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20E=2E=20Garc=C3=ADa?= Date: Fri, 20 Oct 2017 15:23:21 -0600 Subject: [PATCH 05/17] Revert formatting change --- src/totp/totp.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/totp/totp.cpp b/src/totp/totp.cpp index 170ac2da7..28d808e3a 100644 --- a/src/totp/totp.cpp +++ b/src/totp/totp.cpp @@ -109,8 +109,14 @@ QString QTotp::generateTotp(const QByteArray key, QByteArray hmac = code.result(); int offset = (hmac[hmac.length() - 1] & 0xf); - int binary = ((hmac[offset] & 0x7f) << 24) | ((hmac[offset + 1] & 0xff) << 16) | ((hmac[offset + 2] & 0xff) << 8) | - (hmac[offset + 3] & 0xff); + + // clang-format off + int binary = + ((hmac[offset] & 0x7f) << 24) + | ((hmac[offset + 1] & 0xff) << 16) + | ((hmac[offset + 2] & 0xff) << 8) + | (hmac[offset + 3] & 0xff); + // clang-format on quint32 digitsPower = pow(10, numDigits); From 24f560aaa2c0a4574697bdc9b7ec41de9815c6de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20E=2E=20Garc=C3=ADa?= Date: Fri, 20 Oct 2017 16:55:02 -0600 Subject: [PATCH 06/17] Replace Optional with QVariant --- src/CMakeLists.txt | 1 - src/core/Base32.cpp | 10 +++--- src/core/Base32.h | 4 +-- src/core/Optional.h | 86 -------------------------------------------- src/totp/totp.cpp | 7 ++-- tests/TestBase32.cpp | 66 ++++++++++++++++++++++------------ 6 files changed, 54 insertions(+), 120 deletions(-) delete mode 100644 src/core/Optional.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 28170b162..5255186ea 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -61,7 +61,6 @@ set(keepassx_SOURCES core/Uuid.cpp core/Base32.h core/Base32.cpp - core/Optional.h cli/PasswordInput.cpp cli/PasswordInput.h crypto/Crypto.cpp diff --git a/src/core/Base32.cpp b/src/core/Base32.cpp index 5bd20901d..6fdecccd8 100644 --- a/src/core/Base32.cpp +++ b/src/core/Base32.cpp @@ -40,14 +40,14 @@ constexpr quint8 ASCII_a = static_cast('a'); constexpr quint8 ASCII_z = static_cast('z'); constexpr quint8 ASCII_EQ = static_cast('='); -Optional Base32::decode(const QByteArray& encodedData) +QVariant Base32::decode(const QByteArray& encodedData) { if (encodedData.size() <= 0) { - return Optional(""); + return QVariant::fromValue(QByteArray("")); } if (encodedData.size() % 8 != 0) { - return Optional(); + return QVariant(); } int nPads = 0; @@ -114,7 +114,7 @@ Optional Base32::decode(const QByteArray& encodedData) continue; } else { // illegal character - return Optional(); + return QVariant(); } } } @@ -132,7 +132,7 @@ Optional Base32::decode(const QByteArray& encodedData) } } - return Optional(data); + return QVariant::fromValue(data); } QByteArray Base32::encode(const QByteArray& data) diff --git a/src/core/Base32.h b/src/core/Base32.h index 048828974..204368c52 100644 --- a/src/core/Base32.h +++ b/src/core/Base32.h @@ -24,15 +24,15 @@ #ifndef BASE32_H #define BASE32_H -#include "Optional.h" #include +#include #include class Base32 { public: Base32() = default; - Q_REQUIRED_RESULT static Optional decode(const QByteArray&); + Q_REQUIRED_RESULT static QVariant decode(const QByteArray&); Q_REQUIRED_RESULT static QByteArray encode(const QByteArray&); Q_REQUIRED_RESULT static QByteArray addPadding(const QByteArray&); Q_REQUIRED_RESULT static QByteArray removePadding(const QByteArray&); diff --git a/src/core/Optional.h b/src/core/Optional.h deleted file mode 100644 index 0721daa3d..000000000 --- a/src/core/Optional.h +++ /dev/null @@ -1,86 +0,0 @@ -/* - * Copyright (C) 2017 KeePassXC Team - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 2 or (at your option) - * version 3 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -#ifndef OPTIONAL_H -#define OPTIONAL_H - -/* - * This utility class is for providing basic support for an option type. - * It can be replaced by std::optional (C++17) or - * std::experimental::optional (C++11) when they become fully supported - * by all the main compiler toolchains. - */ - -template class Optional -{ -public: - // None - Optional() - : m_hasValue(false) - , m_value(){}; - - // Some T - Optional(const T& value) - : m_hasValue(true) - , m_value(value){}; - - // Copy - Optional(const Optional& other) - : m_hasValue(other.m_hasValue) - , m_value(other.m_value){}; - - const Optional& operator=(const Optional& other) - { - m_hasValue = other.m_hasValue; - m_value = other.m_value; - return *this; - } - - bool operator==(const Optional& other) const - { - if (m_hasValue) - return other.m_hasValue && m_value == other.m_value; - else - return !other.m_hasValue; - } - - bool operator!=(const Optional& other) const - { - return !(*this == other); - } - - bool hasValue() const - { - return m_hasValue; - } - - T valueOr(const T& other) const - { - return m_hasValue ? m_value : other; - } - - Optional static makeOptional(const T& value) - { - return Optional(value); - } - -private: - bool m_hasValue; - T m_value; -}; - -#endif // OPTIONAL_H diff --git a/src/totp/totp.cpp b/src/totp/totp.cpp index 28d808e3a..7a584def2 100644 --- a/src/totp/totp.cpp +++ b/src/totp/totp.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -98,13 +99,13 @@ QString QTotp::generateTotp(const QByteArray key, { quint64 current = qToBigEndian(time / step); - Optional secret = Base32::decode(Base32::sanitizeInput(key)); - if (!secret.hasValue()) { + QVariant secret = Base32::decode(Base32::sanitizeInput(key)); + if (secret.isNull()) { return "Invalid TOTP secret key"; } QMessageAuthenticationCode code(QCryptographicHash::Sha1); - code.setKey(secret.valueOr("")); + code.setKey(secret.toByteArray()); code.addData(QByteArray(reinterpret_cast(¤t), sizeof(current))); QByteArray hmac = code.result(); diff --git a/tests/TestBase32.cpp b/tests/TestBase32.cpp index 3d829d96b..615b356fc 100644 --- a/tests/TestBase32.cpp +++ b/tests/TestBase32.cpp @@ -25,86 +25,102 @@ void TestBase32::testDecode() { // 3 quanta, all upper case + padding QByteArray encodedData = "JBSWY3DPEB3W64TMMQXC4LQ="; - auto data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("Hello world...")); + QVariant data = Base32::decode(encodedData); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("Hello world...")); // 4 quanta, all upper case encodedData = "GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ"; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("12345678901234567890")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("12345678901234567890")); // 4 quanta, all lower case encodedData = "gezdgnbvgy3tqojqgezdgnbvgy3tqojq"; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("12345678901234567890")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("12345678901234567890")); // 4 quanta, mixed upper and lower case encodedData = "Gezdgnbvgy3tQojqgezdGnbvgy3tQojQ"; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("12345678901234567890")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("12345678901234567890")); // 1 pad characters encodedData = "ORSXG5A="; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("test")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("test")); // 3 pad characters encodedData = "L5PV6==="; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("___")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("___")); // 4 pad characters encodedData = "MZXW6IDCMFZA===="; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("foo bar")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("foo bar")); // six pad characters encodedData = "MZXW6YTBOI======"; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("foobar")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("foobar")); encodedData = "IA======"; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("@")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("@")); // error: illegal character encodedData = "1MZXW6YTBOI====="; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("ERROR")); + QVERIFY(data.isNull()); // error: missing pad character encodedData = "MZXW6YTBOI====="; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("ERROR")); + QVERIFY(data.isNull()); // RFC 4648 test vectors encodedData = ""; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("")); encodedData = "MY======"; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("f")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("f")); encodedData = "MZXQ===="; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("fo")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("fo")); encodedData = "MZXW6==="; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("foo")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("foo")); encodedData = "MZXW6YQ="; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("foob")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("foob")); encodedData = "MZXW6YTB"; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("fooba")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("fooba")); encodedData = "MZXW6YTBOI======"; data = Base32::decode(encodedData); - QCOMPARE(QString::fromLatin1(data.valueOr("ERROR")), QString("foobar")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("foobar")); } void TestBase32::testEncode() @@ -258,21 +274,25 @@ void TestBase32::testSanitizeInput() // sanitize input (white space + missing padding) QByteArray encodedData = "JBSW Y3DP EB3W 64TM MQXC 4LQA"; auto data = Base32::decode(Base32::sanitizeInput(encodedData)); - QCOMPARE(QString::fromLatin1(data.valueOr("ERRROR")), QString("Hello world...")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("Hello world...")); // sanitize input (typo + missing padding) encodedData = "J8SWY3DPE83W64TMMQXC4LQA"; data = Base32::decode(Base32::sanitizeInput(encodedData)); - QCOMPARE(QString::fromLatin1(data.valueOr("ERRROR")), QString("Hello world...")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("Hello world...")); // sanitize input (other illegal characters) encodedData = "J8SWY3D[PE83W64TMMQ]XC!4LQA"; data = Base32::decode(Base32::sanitizeInput(encodedData)); - QCOMPARE(QString::fromLatin1(data.valueOr("ERRROR")), QString("Hello world...")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("Hello world...")); // sanitize input (NUL character) encodedData = "J8SWY3DPE83W64TMMQXC4LQA"; encodedData.insert(3, '\0'); data = Base32::decode(Base32::sanitizeInput(encodedData)); - QCOMPARE(QString::fromLatin1(data.valueOr("ERRROR")), QString("Hello world...")); + QVERIFY(!data.isNull()); + QCOMPARE(data.toString(), QString("Hello world...")); } From c731f8e5c05b8546a5aa35e3b0b8f027dc1f4009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20E=2E=20Garc=C3=ADa?= Date: Fri, 20 Oct 2017 18:05:33 -0600 Subject: [PATCH 07/17] Fix bug in Base32::decode and add bounds check The bug that was fixed, was affecting how the number of bytes of decoded data was calculated and thus, even though it didn't truncate the result, it was causing the array to be resized unnecessarily. --- src/core/Base32.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/core/Base32.cpp b/src/core/Base32.cpp index 6fdecccd8..b76fd0293 100644 --- a/src/core/Base32.cpp +++ b/src/core/Base32.cpp @@ -83,7 +83,7 @@ QVariant Base32::decode(const QByteArray& encodedData) Q_ASSERT(encodedData.size() > 0); const int nQuanta = encodedData.size() / 8; - const int nBytes = (nQuanta - 1) * 5 + nSpecialBytes; + const int nBytes = nSpecialBytes > 0 ? (nQuanta - 1) * 5 + nSpecialBytes : nQuanta * 5; QByteArray data(nBytes, Qt::Uninitialized); @@ -126,12 +126,17 @@ QVariant Base32::decode(const QByteArray& encodedData) const int offset = (nQuantumBytes - 1) * 8; quint64 mask = quint64(0xFF) << offset; for (int n = offset; n >= 0; n -= 8) { - char c = static_cast((quantum & mask) >> n); - data[o++] = c; - mask >>= 8; + if (o < nBytes) { + data[o++] = static_cast((quantum & mask) >> n); + mask >>= 8; + } else { + break; + } } } + Q_ASSERT(nBytes == o); + return QVariant::fromValue(data); } @@ -144,6 +149,7 @@ QByteArray Base32::encode(const QByteArray& data) const int nBits = data.size() * 8; const int rBits = nBits % 40; // in {0, 8, 16, 24, 32} const int nQuanta = nBits / 40 + (rBits > 0 ? 1 : 0); + const int nBytes = nQuanta * 8; QByteArray encodedData(nQuanta * 8, Qt::Uninitialized); int i = 0; @@ -211,7 +217,8 @@ QByteArray Base32::encode(const QByteArray& data) } } - Q_ASSERT(encodedData.size() == o); + Q_ASSERT(data.size() == i); + Q_ASSERT(nBytes == o); return encodedData; } From f1d99dd0ede60bfe16438caaf7323219a36fa015 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20E=2E=20Garc=C3=ADa?= Date: Fri, 20 Oct 2017 18:31:17 -0600 Subject: [PATCH 08/17] Improve testing of Base32 --- src/core/Base32.cpp | 1 + tests/TestBase32.cpp | 64 +++++++++++++++++++++++++++++++++----------- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/src/core/Base32.cpp b/src/core/Base32.cpp index b76fd0293..cfd7f04c5 100644 --- a/src/core/Base32.cpp +++ b/src/core/Base32.cpp @@ -135,6 +135,7 @@ QVariant Base32::decode(const QByteArray& encodedData) } } + Q_ASSERT(encodedData.size() == i); Q_ASSERT(nBytes == o); return QVariant::fromValue(data); diff --git a/tests/TestBase32.cpp b/tests/TestBase32.cpp index 615b356fc..1d30e7817 100644 --- a/tests/TestBase32.cpp +++ b/tests/TestBase32.cpp @@ -26,55 +26,73 @@ void TestBase32::testDecode() // 3 quanta, all upper case + padding QByteArray encodedData = "JBSWY3DPEB3W64TMMQXC4LQ="; QVariant data = Base32::decode(encodedData); + QString expectedData = "Hello world..."; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("Hello world...")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); // 4 quanta, all upper case encodedData = "GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ"; data = Base32::decode(encodedData); + expectedData = "12345678901234567890"; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("12345678901234567890")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); // 4 quanta, all lower case encodedData = "gezdgnbvgy3tqojqgezdgnbvgy3tqojq"; data = Base32::decode(encodedData); + expectedData = "12345678901234567890"; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("12345678901234567890")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); // 4 quanta, mixed upper and lower case encodedData = "Gezdgnbvgy3tQojqgezdGnbvgy3tQojQ"; data = Base32::decode(encodedData); + expectedData = "12345678901234567890"; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("12345678901234567890")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); // 1 pad characters encodedData = "ORSXG5A="; data = Base32::decode(encodedData); + expectedData = "test"; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("test")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); // 3 pad characters encodedData = "L5PV6==="; data = Base32::decode(encodedData); + expectedData = "___"; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("___")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); // 4 pad characters encodedData = "MZXW6IDCMFZA===="; data = Base32::decode(encodedData); + expectedData = "foo bar"; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("foo bar")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); // six pad characters encodedData = "MZXW6YTBOI======"; data = Base32::decode(encodedData); + expectedData = "foobar"; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("foobar")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); encodedData = "IA======"; data = Base32::decode(encodedData); + expectedData = "@"; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("@")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); // error: illegal character encodedData = "1MZXW6YTBOI====="; @@ -89,38 +107,52 @@ void TestBase32::testDecode() // RFC 4648 test vectors encodedData = ""; data = Base32::decode(encodedData); + expectedData = ""; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); encodedData = "MY======"; data = Base32::decode(encodedData); + expectedData = "f"; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("f")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); encodedData = "MZXQ===="; data = Base32::decode(encodedData); + expectedData = "fo"; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("fo")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); encodedData = "MZXW6==="; data = Base32::decode(encodedData); QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("foo")); + expectedData = "foo"; + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); encodedData = "MZXW6YQ="; data = Base32::decode(encodedData); + expectedData = "foob"; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("foob")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); encodedData = "MZXW6YTB"; + expectedData = "fooba"; data = Base32::decode(encodedData); QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("fooba")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); encodedData = "MZXW6YTBOI======"; data = Base32::decode(encodedData); + expectedData = "foobar"; QVERIFY(!data.isNull()); - QCOMPARE(data.toString(), QString("foobar")); + QCOMPARE(data.toString(), expectedData); + QVERIFY(data.value().size() == expectedData.size()); } void TestBase32::testEncode() From 86cd2c09a416fbb86eba4713a8ad66b27127795c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20E=2E=20Garc=C3=ADa?= Date: Fri, 20 Oct 2017 23:50:34 -0600 Subject: [PATCH 09/17] Improve readability of code --- src/core/Base32.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/core/Base32.cpp b/src/core/Base32.cpp index cfd7f04c5..13228a37f 100644 --- a/src/core/Base32.cpp +++ b/src/core/Base32.cpp @@ -125,13 +125,9 @@ QVariant Base32::decode(const QByteArray& encodedData) const int offset = (nQuantumBytes - 1) * 8; quint64 mask = quint64(0xFF) << offset; - for (int n = offset; n >= 0; n -= 8) { - if (o < nBytes) { + for (int n = offset; n >= 0 && o < nBytes; n -= 8) { data[o++] = static_cast((quantum & mask) >> n); mask >>= 8; - } else { - break; - } } } From 0fe06b3fbb0c4758d8a09335e1d7d635b079587a Mon Sep 17 00:00:00 2001 From: Louis-Bertrand Varin Date: Thu, 5 Oct 2017 14:44:05 -0400 Subject: [PATCH 10/17] Fix double warning display for database open --- src/gui/DatabaseOpenWidget.cpp | 37 +++++++++++++++++++--------------- src/gui/DatabaseOpenWidget.h | 2 +- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index b8f8feb1d..fee0c26ee 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -162,7 +162,10 @@ void DatabaseOpenWidget::enterKey(const QString& pw, const QString& keyFile) void DatabaseOpenWidget::openDatabase() { KeePass2Reader reader; - CompositeKey masterKey = databaseKey(); + CompositeKey* masterKey = databaseKey(); + if (masterKey == nullptr) { + return; + } QFile file(m_filename); if (!file.open(QIODevice::ReadOnly)) { @@ -174,7 +177,7 @@ void DatabaseOpenWidget::openDatabase() delete m_db; } QApplication::setOverrideCursor(QCursor(Qt::WaitCursor)); - m_db = reader.readDatabase(&file, masterKey); + m_db = reader.readDatabase(&file, *masterKey); QApplication::restoreOverrideCursor(); if (m_db) { @@ -182,20 +185,21 @@ void DatabaseOpenWidget::openDatabase() m_ui->messageWidget->animatedHide(); } emit editFinished(true); - } - else { - m_ui->messageWidget->showMessage(tr("Unable to open the database.") - .append("\n").append(reader.errorString()), MessageWidget::Error); + } else { + m_ui->messageWidget->showMessage(tr("Unable to open the database.").append("\n").append(reader.errorString()), + MessageWidget::Error); m_ui->editPassword->clear(); } + + delete masterKey; } -CompositeKey DatabaseOpenWidget::databaseKey() +CompositeKey* DatabaseOpenWidget::databaseKey() { - CompositeKey masterKey; + CompositeKey* masterKey = new CompositeKey(); if (m_ui->checkPassword->isChecked()) { - masterKey.addKey(PasswordKey(m_ui->editPassword->text())); + masterKey->addKey(PasswordKey(m_ui->editPassword->text())); } QHash lastKeyFiles = config()->get("LastKeyFiles").toHash(); @@ -206,11 +210,12 @@ CompositeKey DatabaseOpenWidget::databaseKey() QString keyFilename = m_ui->comboKeyFile->currentText(); QString errorMsg; if (!key.load(keyFilename, &errorMsg)) { - m_ui->messageWidget->showMessage(tr("Can't open key file").append(":\n") - .append(errorMsg), MessageWidget::Error); - return CompositeKey(); + m_ui->messageWidget->showMessage(tr("Can't open key file").append(":\n").append(errorMsg), + MessageWidget::Error); + delete masterKey; + return nullptr; } - masterKey.addKey(key); + masterKey->addKey(key); lastKeyFiles[m_filename] = keyFilename; } else { lastKeyFiles.remove(m_filename); @@ -237,9 +242,9 @@ CompositeKey DatabaseOpenWidget::databaseKey() // read blocking mode from LSB and slot index number from second LSB bool blocking = comboPayload & 1; - int slot = comboPayload >> 1; - auto key = QSharedPointer(new YkChallengeResponseKey(slot, blocking)); - masterKey.addChallengeResponseKey(key); + int slot = comboPayload >> 1; + auto key = QSharedPointer(new YkChallengeResponseKey(slot, blocking)); + masterKey->addChallengeResponseKey(key); } #endif diff --git a/src/gui/DatabaseOpenWidget.h b/src/gui/DatabaseOpenWidget.h index d4b47364d..d2a7e1fa3 100644 --- a/src/gui/DatabaseOpenWidget.h +++ b/src/gui/DatabaseOpenWidget.h @@ -52,7 +52,7 @@ signals: protected: void showEvent(QShowEvent* event) override; void hideEvent(QHideEvent* event) override; - CompositeKey databaseKey(); + CompositeKey* databaseKey(); protected slots: virtual void openDatabase(); From a1aad5d165ab5b16937ff2a15d307fc9565a1ffb Mon Sep 17 00:00:00 2001 From: Louis-Bertrand Varin Date: Sat, 14 Oct 2017 22:12:45 -0400 Subject: [PATCH 11/17] Using QScopedPointer --- src/gui/DatabaseOpenWidget.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index fee0c26ee..19e9a3f4b 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -162,8 +162,8 @@ void DatabaseOpenWidget::enterKey(const QString& pw, const QString& keyFile) void DatabaseOpenWidget::openDatabase() { KeePass2Reader reader; - CompositeKey* masterKey = databaseKey(); - if (masterKey == nullptr) { + QScopedPointer masterKey(databaseKey()); + if (masterKey.isNull()) { return; } @@ -190,8 +190,6 @@ void DatabaseOpenWidget::openDatabase() MessageWidget::Error); m_ui->editPassword->clear(); } - - delete masterKey; } CompositeKey* DatabaseOpenWidget::databaseKey() From aba2acb0620f0a34c29aaf4bac4eb790bec39782 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Sat, 21 Oct 2017 14:00:25 +0200 Subject: [PATCH 12/17] Use QSharedPointer --- src/gui/DatabaseOpenWidget.cpp | 9 ++++----- src/gui/DatabaseOpenWidget.h | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 19e9a3f4b..ee0e9de26 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -162,7 +162,7 @@ void DatabaseOpenWidget::enterKey(const QString& pw, const QString& keyFile) void DatabaseOpenWidget::openDatabase() { KeePass2Reader reader; - QScopedPointer masterKey(databaseKey()); + QSharedPointer masterKey = databaseKey(); if (masterKey.isNull()) { return; } @@ -192,9 +192,9 @@ void DatabaseOpenWidget::openDatabase() } } -CompositeKey* DatabaseOpenWidget::databaseKey() +QSharedPointer DatabaseOpenWidget::databaseKey() { - CompositeKey* masterKey = new CompositeKey(); + auto masterKey = QSharedPointer::create(); if (m_ui->checkPassword->isChecked()) { masterKey->addKey(PasswordKey(m_ui->editPassword->text())); @@ -210,8 +210,7 @@ CompositeKey* DatabaseOpenWidget::databaseKey() if (!key.load(keyFilename, &errorMsg)) { m_ui->messageWidget->showMessage(tr("Can't open key file").append(":\n").append(errorMsg), MessageWidget::Error); - delete masterKey; - return nullptr; + return QSharedPointer(); } masterKey->addKey(key); lastKeyFiles[m_filename] = keyFilename; diff --git a/src/gui/DatabaseOpenWidget.h b/src/gui/DatabaseOpenWidget.h index d2a7e1fa3..a7691a91e 100644 --- a/src/gui/DatabaseOpenWidget.h +++ b/src/gui/DatabaseOpenWidget.h @@ -52,7 +52,7 @@ signals: protected: void showEvent(QShowEvent* event) override; void hideEvent(QHideEvent* event) override; - CompositeKey* databaseKey(); + QSharedPointer databaseKey(); protected slots: virtual void openDatabase(); From a895729b9e75ea5b5353d80188c541f64bbcf5d9 Mon Sep 17 00:00:00 2001 From: Peter Nirschl Date: Sat, 21 Oct 2017 19:18:19 +0200 Subject: [PATCH 13/17] :bug: Fix result propagation in SymmetricCipherGcrypt::process The boolean 'ok' has always been set to true, regardless of possible errors. --- src/crypto/SymmetricCipherGcrypt.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/crypto/SymmetricCipherGcrypt.cpp b/src/crypto/SymmetricCipherGcrypt.cpp index cd4324065..e600a7edb 100644 --- a/src/crypto/SymmetricCipherGcrypt.cpp +++ b/src/crypto/SymmetricCipherGcrypt.cpp @@ -148,9 +148,10 @@ QByteArray SymmetricCipherGcrypt::process(const QByteArray& data, bool* ok) if (error != 0) { setErrorString(error); *ok = false; + } else { + *ok = true; } - *ok = true; return result; } From 76ac8dda547eadcc4091a7955a990f745af761ef Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Sat, 21 Oct 2017 17:22:47 +0200 Subject: [PATCH 14/17] Fix missing semicolon --- share/linux/org.keepassxc.desktop | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/share/linux/org.keepassxc.desktop b/share/linux/org.keepassxc.desktop index fd94fdcd5..d3b007bdc 100644 --- a/share/linux/org.keepassxc.desktop +++ b/share/linux/org.keepassxc.desktop @@ -12,5 +12,5 @@ Icon=keepassxc Terminal=false Type=Application Version=1.0 -Categories=Utility;Security;Qt +Categories=Utility;Security;Qt; MimeType=application/x-keepass2; From 94ea91ec6321f0fc4fb6f1fa28149eff5b1cb08b Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Sat, 21 Oct 2017 17:02:46 +0200 Subject: [PATCH 15/17] Switch from Ubuntu 14.04 to CentOS 7 --- AppImage-Recipe.sh | 5 ++- Dockerfile | 84 ++++++++++++++++++++++++++++------------------ 2 files changed, 55 insertions(+), 34 deletions(-) diff --git a/AppImage-Recipe.sh b/AppImage-Recipe.sh index 2a40458fc..a05a118eb 100755 --- a/AppImage-Recipe.sh +++ b/AppImage-Recipe.sh @@ -34,6 +34,7 @@ fi APP="$1" LOWERAPP="$(echo "$APP" | tr '[:upper:]' '[:lower:]')" VERSION="$2" +export ARCH=x86_64 mkdir -p $APP.AppDir wget -q https://github.com/probonopd/AppImages/raw/master/functions.sh -O ./functions.sh @@ -42,6 +43,8 @@ wget -q https://github.com/probonopd/AppImages/raw/master/functions.sh -O ./func LIB_DIR=./usr/lib if [ -d ./usr/lib/x86_64-linux-gnu ]; then LIB_DIR=./usr/lib/x86_64-linux-gnu +elif [ -d ./usr/lib64 ]; then + LIB_DIR=./usr/lib64 fi cd $APP.AppDir @@ -51,7 +54,7 @@ rm -R ./usr/local rmdir ./opt 2> /dev/null # bundle Qt platform plugins and themes -QXCB_PLUGIN="$(find /usr/lib -name 'libqxcb.so' 2> /dev/null)" +QXCB_PLUGIN="$(find /usr/lib* -name 'libqxcb.so' 2> /dev/null)" if [ "$QXCB_PLUGIN" == "" ]; then QXCB_PLUGIN="$(find /opt/qt*/plugins -name 'libqxcb.so' 2> /dev/null)" fi diff --git a/Dockerfile b/Dockerfile index 60ba4ff57..6f7ace34c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -14,50 +14,68 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -FROM ubuntu:14.04 +FROM centos:7 RUN set -x \ - && apt-get update \ - && apt-get install --yes software-properties-common + && curl "https://copr.fedorainfracloud.org/coprs/bugzy/keepassxc/repo/epel-7/bugzy-keepassxc-epel-7.repo" \ + > /etc/yum.repos.d/bugzy-keepassxc-epel-7.repo RUN set -x \ - && add-apt-repository ppa:george-edison55/cmake-3.x - -ENV QT_PPA=qt591 -ENV QT_VERSION=qt59 + && curl "https://copr.fedorainfracloud.org/coprs/sic/backports/repo/epel-7/sic-backports-epel-7.repo" \ + > /etc/yum.repos.d/sic-backports-epel-7.repo RUN set -x \ - && add-apt-repository --yes ppa:beineri/opt-${QT_PPA}-trusty - + && yum clean -y all \ + && yum upgrade -y +# build and runtime dependencies RUN set -x \ - && apt-get update \ - && apt-get install --yes \ - g++ \ + && yum install -y \ + make \ + automake \ + gcc-c++ \ cmake \ - libgcrypt20-dev \ - ${QT_VERSION}base \ - ${QT_VERSION}tools \ - ${QT_VERSION}x11extras \ - libxi-dev \ - libxtst-dev \ - zlib1g-dev \ - libyubikey-dev \ - libykpers-1-dev \ - xvfb \ - wget \ - file \ - fuse \ - python + libgcrypt16-devel \ + qt5-qtbase-devel \ + qt5-linguist \ + qt5-qttools \ + zlib-devel \ + qt5-qtx11extras \ + qt5-qtx11extras-devel \ + libXi-devel \ + libXtst-devel +# AppImage dependencies RUN set -x \ - && apt-get install --yes mesa-common-dev - + && yum install -y \ + wget \ + fuse-libs + +# build libyubikey +ENV YUBIKEY_VERSION=1.13 +RUN set -x && yum install -y libusb-devel +RUN set -x \ + && wget "https://developers.yubico.com/yubico-c/Releases/libyubikey-${YUBIKEY_VERSION}.tar.gz" \ + && tar xf libyubikey-${YUBIKEY_VERSION}.tar.gz \ + && cd libyubikey-${YUBIKEY_VERSION} \ + && ./configure --prefix=/usr --libdir=/usr/lib64 \ + && make \ + && make install \ + && cd .. \ + && rm -Rf libyubikey-${YUBIKEY_VERSION}* + +# build libykpers-1 +ENV YKPERS_VERSION=1.18.0 +RUN set -x \ + && wget "https://developers.yubico.com/yubikey-personalization/Releases/ykpers-${YKPERS_VERSION}.tar.gz" \ + && tar xf ykpers-${YKPERS_VERSION}.tar.gz \ + && cd ykpers-${YKPERS_VERSION} \ + && ./configure --prefix=/usr --libdir=/usr/lib64 \ + && make \ + && make install \ + && cd .. \ + && rm -Rf ykpers-${YKPERS_VERSION}* + VOLUME /keepassxc/src VOLUME /keepassxc/out WORKDIR /keepassxc - -ENV CMAKE_PREFIX_PATH=/opt/${QT_VERSION}/lib/cmake -ENV LD_LIBRARY_PATH=/opt/${QT_VERSION}/lib -RUN set -x \ - && echo /opt/${QT_VERSION}/lib > /etc/ld.so.conf.d/${QT_VERSION}.conf From 51ed7a59da5f1331277ef6d799d534d5ab3029d4 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Sat, 21 Oct 2017 20:24:39 +0200 Subject: [PATCH 16/17] Fix Exec replacement --- AppImage-Recipe.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/AppImage-Recipe.sh b/AppImage-Recipe.sh index a05a118eb..0e7d16af9 100755 --- a/AppImage-Recipe.sh +++ b/AppImage-Recipe.sh @@ -74,7 +74,6 @@ get_desktop get_icon cat << EOF > ./usr/bin/keepassxc_env #!/usr/bin/env bash -#export QT_QPA_PLATFORMTHEME=gtk2 export LD_LIBRARY_PATH="..$(dirname ${QT_PLUGIN_PATH})/lib:\${LD_LIBRARY_PATH}" export QT_PLUGIN_PATH="..${QT_PLUGIN_PATH}:\${KPXC_QT_PLUGIN_PATH}" @@ -85,7 +84,7 @@ unset XDG_DATA_DIRS exec keepassxc "\$@" EOF chmod +x ./usr/bin/keepassxc_env -sed -i 's/Exec=keepassxc/Exec=keepassxc_env/' keepassxc.desktop +sed -i 's/Exec=keepassxc/Exec=keepassxc_env/' org.keepassxc.desktop get_desktopintegration $LOWERAPP GLIBC_NEEDED=$(glibc_needed) @@ -94,5 +93,5 @@ cd .. generate_type2_appimage -mv ../out/*.AppImage .. +mv ../out/*.AppImage ../KeePassXC-${VERSION}-${ARCH}.AppImage rmdir ../out > /dev/null 2>&1 From e2c6f5010830855ff1a4b49bfe60f455951218fd Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Sat, 21 Oct 2017 20:32:41 +0200 Subject: [PATCH 17/17] Support starting keepassxc-cli via 'cli' argument --- AppImage-Recipe.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/AppImage-Recipe.sh b/AppImage-Recipe.sh index 0e7d16af9..c421aa79e 100755 --- a/AppImage-Recipe.sh +++ b/AppImage-Recipe.sh @@ -78,10 +78,15 @@ export LD_LIBRARY_PATH="..$(dirname ${QT_PLUGIN_PATH})/lib:\${LD_LIBRARY_PATH}" export QT_PLUGIN_PATH="..${QT_PLUGIN_PATH}:\${KPXC_QT_PLUGIN_PATH}" # unset XDG_DATA_DIRS to make tray icon work in Ubuntu Unity -# see https://github.com/probonopd/AppImageKit/issues/351 +# see https://github.com/AppImage/AppImageKit/issues/351 unset XDG_DATA_DIRS -exec keepassxc "\$@" +if [ "\${1}" == "cli" ]; then + shift + exec keepassxc-cli "\$@" +else + exec keepassxc "\$@" +fi EOF chmod +x ./usr/bin/keepassxc_env sed -i 's/Exec=keepassxc/Exec=keepassxc_env/' org.keepassxc.desktop