Skip to content

Commit

Permalink
Improve OTP expierience
Browse files Browse the repository at this point in the history 
Code refactoring
Live validation of entered OTP secret key
Label showing the entered key is too long
Cleaning of user input and adding proper padding (fixes #280)
Catch all exceptions from parsing the secret key (fixes #240)
Possibly a solution for #239
Possibility of entering secret with spaces or hyphens
Added parsing by base32-crowford as a fallback

Signed-off-by: Szczepan Zalega <szczepan@nitrokey.com>
  • Loading branch information
@szszszsz
szszszsz committed Oct 21, 2017
1 parent d7795e1 commit 22d91b8
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 46 deletions.
2 changes: 1 addition & 1 deletion libnitrokey
Submodule libnitrokey updated 1 files
+5 −6 include/DeviceCommunicationExceptions.h
6 changes: 3 additions & 3 deletions src/hotpslot.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@

#include <cstdint>

static const int SECRET_LENGTH = 40;
static const int SECRET_LENGTH_BASE32 = SECRET_LENGTH / 10 * 16;
static const int SECRET_LENGTH_HEX = SECRET_LENGTH * 2;
static const unsigned int SECRET_LENGTH = 40;
static const unsigned int SECRET_LENGTH_BASE32 = SECRET_LENGTH / 10 * 16;
static const unsigned int SECRET_LENGTH_HEX = SECRET_LENGTH * 2;



Expand Down
165 changes: 129 additions & 36 deletions src/ui/mainwindow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ MainWindow::MainWindow(QWidget *parent):
ui->PWS_ButtonCreatePW->setText(QString(tr("Generate random password ")));
ui->PWS_progressBar->hide();
ui->statusBar->showMessage(tr("Nitrokey disconnected"));
ui->l_supportedLength->hide();

QTimer *timer = new QTimer(this);
connect(timer, SIGNAL(timeout()), this, SLOT(checkConnection()));
Expand Down Expand Up @@ -138,10 +139,38 @@ MainWindow::MainWindow(QWidget *parent):
this->adjustSize();
this->move(QApplication::desktop()->screen()->rect().center() - this->rect().center());


first_run();
}

#include <cppcodec/base32_crockford.hpp>
#include <cppcodec/base32_default_rfc4648.hpp>
#include <cppcodec/hex_upper.hpp>

std::vector<uint8_t> decodeBase32Secret(std::string secret){
std::vector<uint8_t> secret_raw;
std::string error;
try {
secret_raw = base32::decode(secret);
return secret_raw;
}
catch (const cppcodec::parse_error &e){
qDebug() << e.what();
error = error + "base32: " + e.what();
}
try {
auto s = QString::fromStdString(secret);
s = s.remove('=');
secret_raw = cppcodec::base32_crockford::decode(s);
return secret_raw;
}
catch (const cppcodec::parse_error &e){
qDebug() << e.what();
error = error + "; crockford: " + e.what();
}
throw cppcodec::parse_error(error);
return secret_raw;
}

void MainWindow::first_run(){
QSettings settings;
const auto first_run_key = "main/first_run";
Expand Down Expand Up @@ -307,16 +336,17 @@ void MainWindow::generateHOTPConfig(OTPSlot *slot) {
}
}

#include <cppcodec/base32_default_rfc4648.hpp>
#include <cppcodec/hex_upper.hpp>
void MainWindow::generateOTPConfig(OTPSlot *slot) const {
#include <src/GUI/ManageWindow.h>

void MainWindow::generateOTPConfig(OTPSlot *slot) {
using hex = cppcodec::hex_upper;

std::string secret_hex;
auto secretFromGUI = this->ui->secretEdit->text().toStdString();
// ui->base32RadioButton->toggle();
if(ui->base32RadioButton->isDown()){
auto secret_raw = base32::decode(secretFromGUI);
ui->base32RadioButton->toggle();
auto secretFromGUI = getOTPSecretCleaned().toLatin1().toStdString();

if(ui->base32RadioButton->isChecked()){
auto secret_raw = decodeBase32Secret(secretFromGUI);
secret_hex = hex::encode(secret_raw);
} else{
secret_hex = secretFromGUI;
Expand Down Expand Up @@ -425,10 +455,9 @@ void MainWindow::displayCurrentTotpSlotConfig(uint8_t slotNo) {
ui->muiEdit->setText(QString("%1").arg(QString::fromStdString(cardSerial), 8, '0'));
ui->intervalSpinBox->setValue(30);

//TODO readout TOTP slot data
//TODO implement reading slot data in libnitrokey
//TODO move reading to separate thread


try{
if (libada::i()->isTOTPSlotProgrammed(slotNo)) {
//FIXME use separate thread
Expand Down Expand Up @@ -505,6 +534,7 @@ void MainWindow::displayCurrentSlotConfig() {
displayCurrentTotpSlotConfig(slotNo);
}
ui->slotComboBox->setEnabled(true);
checkTextEdited();
}

void MainWindow::displayCurrentGeneralConfig() {
Expand All @@ -526,11 +556,7 @@ void MainWindow::startConfiguration() {
displayCurrentGeneralConfig();
SetupPasswordSafeConfig();

//Bring up the window
raise();
activateWindow();
showNormal();
setWindowState(Qt::WindowActive);
ManageWindow::bringToFocus(this);

QTimer::singleShot(0, this, SLOT(resizeMin()));
}
Expand Down Expand Up @@ -605,7 +631,6 @@ void MainWindow::on_writeButton_clicked() {

if (ui->nameEdit->text().isEmpty()) {
csApplet()->warningBox(tr("Please enter a slotname."));
// csApplet()->warningBox(tr("The name of the slot must not be empty."));
return;
}

Expand All @@ -619,16 +644,18 @@ void MainWindow::on_writeButton_clicked() {
if (isHOTP) { // HOTP slot
generateHOTPConfig(&otp);
} else {
generateTOTPConfig(&otp);
generateTOTPConfig(&otp);
}
}
catch(const cppcodec::parse_error){
ui->secretEdit->setText("");
csApplet()->warningBox(tr("The secret string you have entered is invalid. Please reenter it."));
catch(const cppcodec::parse_error &e){
csApplet()->warningBox(tr("The secret string you have entered is invalid. Please reenter it.")
+"\n"+tr("Details: ") + e.what());
return;
}

if (!this->ui->secretEdit->text().isEmpty() && !validate_secret(otp.secret)) {
if (!this->ui->secretEdit->text().isEmpty() && !validate_raw_secret(otp.secret)) {
csApplet()->warningBox(tr("The secret string you have entered is invalid. Please reenter it.")
+"\n"+tr("Details: ") + tr("secret is not passing validation."));
return;
}

Expand All @@ -638,9 +665,12 @@ void MainWindow::on_writeButton_clicked() {
csApplet()->messageBox(tr("Configuration successfully written."));
emit OTP_slot_write(slotNo, isHOTP);
}
catch (CommandFailedException &e){
catch (const CommandFailedException&){
csApplet()->warningBox(tr("Error writing configuration!"));
}
catch (const InvalidHexString&){
csApplet()->warningBox(tr("Provided secret hex string is invalid. Please check input and try again."));
}
}

// QApplication::setOverrideCursor(QCursor(Qt::WaitCursor));
Expand All @@ -649,14 +679,14 @@ void MainWindow::on_writeButton_clicked() {
generateAllConfigs();
}

bool MainWindow::validate_secret(const char *secret) const {
bool MainWindow::validate_raw_secret(const char *secret) const {
if(libada::i()->is_nkpro_07_rtm1() && secret[0] == 0){
csApplet()->warningBox(tr("Nitrokey Pro v0.7 does not support secrets starting from null byte. Please change the secret."));
return false;
}
//check if the secret consist only from null bytes
//(this value is reserved - it would be ignored by device)
for (int i=0; i < SECRET_LENGTH; i++){
for (unsigned int i=0; i < SECRET_LENGTH; i++){
if (secret[i] != 0){
return true;
}
Expand All @@ -673,26 +703,44 @@ void MainWindow::on_hexRadioButton_toggled(bool checked) {
if (!checked) {
return;
}
ui->secretEdit->setMaxLength(get_supported_secret_length_hex());

QString secret_input = getOTPSecretCleaned();

auto secret = ui->secretEdit->text().toLatin1().toStdString();
auto secret = secret_input.toLatin1().toStdString();
if (secret.size() != 0) {
try{
auto secret_raw = base32::decode(secret);
auto secret_raw = decodeBase32Secret(secret);
auto secret_hex = QString::fromStdString(cppcodec::hex_upper::encode(secret_raw));
ui->secretEdit->setText(secret_hex);
clipboard.copyToClipboard(secret_hex);
showNotificationLabel();
}
catch (const cppcodec::parse_error &e) {
ui->secretEdit->setText("");
csApplet()->warningBox(tr("The secret string you have entered is invalid. Please reenter it."));
csApplet()->warningBox(tr("The secret string you have entered is invalid. Please reenter it.")
+"\n"+tr("Details: ") + e.what());
}
}
}

int MainWindow::get_supported_secret_length_hex() const {
//TODO move to libada or libnitrokey
QString MainWindow::getOTPSecretCleaned(bool fix) {
auto secret_input = ui->secretEdit->text();
secret_input = secret_input.remove('-').remove(' ').trimmed();
constexpr auto base32_block_size = 8u;
int secret_length = std::min(get_supported_secret_length_base32(),
roundToNextMultiple(secret_input.length(), base32_block_size));
secret_input = secret_input.leftJustified(secret_length, '=');
if(fix)
ui->secretEdit->setText(secret_input);
return secret_input;
}

unsigned int MainWindow::roundToNextMultiple(const int number, const int multipleOf) const {
return static_cast<unsigned int>(
number + ((number % multipleOf == 0) ? 0 : multipleOf - number % multipleOf));
}

unsigned int MainWindow::get_supported_secret_length_hex() const {
auto local_secret_length = SECRET_LENGTH_HEX;
if (!libada::i()->is_secret320_supported()){
local_secret_length /= 2;
Expand All @@ -712,13 +760,14 @@ void MainWindow::on_base32RadioButton_toggled(bool checked) {
auto secret_base32 = QString::fromStdString(base32::encode(secret_raw));
ui->secretEdit->setText(secret_base32);
clipboard.copyToClipboard(secret_base32);
showNotificationLabel();
}
catch (const cppcodec::parse_error &e) {
ui->secretEdit->setText("");
csApplet()->warningBox(tr("The secret string you have entered is invalid. Please reenter it."));
csApplet()->warningBox(tr("The secret string you have entered is invalid. Please reenter it.")
+" "+tr("Details: ") + e.what());
}
}
ui->secretEdit->setMaxLength(get_supported_secret_length_base32());
}

void MainWindow::on_setToZeroButton_clicked() { ui->counterEdit->setText("0"); }
Expand Down Expand Up @@ -783,8 +832,9 @@ void MainWindow::on_writeGeneralConfigButton_clicked() {
void MainWindow::getHOTPDialog(int slot) {
try{
auto OTPcode = getNextCode(0x10 + slot);
if (OTPcode.empty()) return;
clipboard.copyToClipboard(QString::fromStdString(OTPcode));
if (OTPcode.empty()) return;
clipboard.copyToClipboard(QString::fromStdString(OTPcode));


if (libada::i()->getHOTPSlotName(slot).empty())
tray.showTrayMessage(QString(tr("HOTP slot ")).append(QString::number(slot + 1, 10)),
Expand Down Expand Up @@ -885,9 +935,25 @@ void MainWindow::on_randomSecretButton_clicked() {
ui->checkBox->setEnabled(true);
ui->checkBox->setChecked(true);
clipboard.copyToClipboard(secretArray);
showNotificationLabel();
this->checkTextEdited();
}

void MainWindow::showNotificationLabel() {
static qint64 lastTimeNotificationShown = 0;
lastTimeNotificationShown = QDateTime::currentMSecsSinceEpoch();
constexpr auto delayBeforeHiding = 6000;
constexpr auto timer_precision_off = 1000;
ui->labelNotify->show();
QTimer::singleShot(delayBeforeHiding, [this](){
if (QDateTime::currentMSecsSinceEpoch() >
lastTimeNotificationShown + delayBeforeHiding - timer_precision_off){
ui->labelNotify->hide();
}
});
}

int MainWindow::get_supported_secret_length_base32() const {
unsigned int MainWindow::get_supported_secret_length_base32() const {
auto local_secret_length = SECRET_LENGTH_BASE32;
if (!libada::i()->is_secret320_supported()){
local_secret_length /= 2;
Expand All @@ -900,11 +966,38 @@ void MainWindow::on_checkBox_toggled(bool checked) {
}

void MainWindow::checkTextEdited() {
ui->secretEdit->setText(ui->secretEdit->text().remove(" ").remove("\t"));
if (!ui->checkBox->isEnabled()) {
ui->checkBox->setEnabled(true);
ui->checkBox->setChecked(false);
}
bool valid = ui->secretEdit->text().isEmpty();
bool correct_length = true;

if (!valid){
bool base32 = ui->base32RadioButton->isChecked();
if (base32) {
auto secret = getOTPSecretCleaned(false).toStdString();
if (secret.empty()) return;
try {
correct_length = secret.length() <= get_supported_secret_length_base32();
auto secret_raw = decodeBase32Secret(secret);
valid = validate_raw_secret(reinterpret_cast<const char *>(secret_raw.data()));
valid = valid && correct_length;
}
catch (...) {}
} else { // hex
const auto l = static_cast<unsigned int>(ui->secretEdit->text().length());
correct_length = l <= get_supported_secret_length_hex();
valid = l % 2 == 0;
valid = valid && correct_length;
}
}

ui->l_supportedLength->setVisible(!correct_length);
ui->secretEdit->setStyleSheet(valid ? "background: white;" : "background: #FFDDDD;");
ui->base32RadioButton->setEnabled(valid);
ui->hexRadioButton->setEnabled(valid);
ui->writeButton->setEnabled(valid);
}

void MainWindow::SetupPasswordSafeConfig(void) {
Expand Down
14 changes: 10 additions & 4 deletions src/ui/mainwindow.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public :
DebugDialog *debug;


bool validate_secret(const char *secret) const;
bool validate_raw_secret(const char *secret) const;
void initialTimeReset();
QMutex check_connection_mutex;
QString nkpro_user_PIN;
Expand Down Expand Up @@ -178,9 +178,9 @@ private slots:


public:
void generateOTPConfig(OTPSlot *slot) const;
int get_supported_secret_length_base32() const;
int get_supported_secret_length_hex() const;
void generateOTPConfig(OTPSlot *slot);
unsigned int get_supported_secret_length_base32() const;
unsigned int get_supported_secret_length_hex() const;

std::atomic_bool long_operation_in_progress {false};
std::shared_ptr<Stick20ResponseDialog> progress_window;
Expand All @@ -201,6 +201,12 @@ private slots:

private:
bool debug_mode = false;

void showNotificationLabel();

unsigned int roundToNextMultiple(const int number, const int multipleOf) const;

QString getOTPSecretCleaned(bool fix = true);
};

class utf8FieldLengthValidator : public QValidator {
Expand Down
Loading

0 comments on commit 22d91b8

Please sign in to comment.