From 105a208968bac60a2aafc85f64615ffc18458036 Mon Sep 17 00:00:00 2001 From: Osmium Sorcerer Date: Wed, 23 Sep 2026 23:17:50 +0000 Subject: Rewrite authentication flow to handle new keys Previously, running /auth command would immediately send off request with the provided username, and all keyring actions will only happen once the client receives the challenge. This isn't possible now that we have both software X25519 and hardware P-256 keys because we must signal which key type we're using in advance. Rather than deferring everything until the challenge, perform the key setup immediately. Parse it, load it into the auth state, and send the full request. When the challenge is received, all that's left is to unlock the already resolved key. In case user cancels the prompt, open key selection dialog again. Make auth state machine stricter, so it's more consistent and easier to reason about, and only expose small interface. In particular, it provides key switching without restarting the auth flow. The state itself is only initialized once at program start, fully static, and avoids superflous allocations and deletions which could result in invalid states. Unify auth-related structures under it that were previously separate, such as saved hostname-username pairs. Send signals about request and response being ready instead of directly using the network functions, which makes it especially better fit in context of two asynchronous dialogs. To properly encode hardware or software key type in the intitial request without creating a separate packet, an extension to subprotocol 2 was introduced in a format-violating manner: an extension bit set means a hardware key is used, and thus we'll expect an ephemeral P-256 key as the challenge. The entire credential message format will be completely rewritten in version 3, this is a prototype implementation. --- src/aoapplication.cpp | 7 ++ src/aoapplication.h | 5 +- src/auth_flow.cpp | 276 +++++++++++++++++++++++++++++++++-------------- src/auth_flow.h | 58 ++++++---- src/courtroom.cpp | 3 +- src/ext_distribution.cpp | 12 +-- src/ext_packet.cpp | 29 ++++- src/ext_packet.h | 10 +- src/main.cpp | 2 +- 9 files changed, 283 insertions(+), 119 deletions(-) (limited to 'src') diff --git a/src/aoapplication.cpp b/src/aoapplication.cpp index 490c60e..b3af79b 100644 --- a/src/aoapplication.cpp +++ b/src/aoapplication.cpp @@ -33,6 +33,13 @@ AOApplication::AOApplication(QObject *parent) mus_decoder_config.pCustomBackendUserData = nullptr; mus_decoder_config.customBackendCount = 2; mus_decoder_config.ppCustomBackendVTables = mus_decoders; + + connect(&this->auth, &AuthFlow::request_ready, this, [this](AuthRequest req){ + send_ex_message(serializeAuthRequest(req)); + }); + connect(&this->auth, &AuthFlow::response_ready, this, [this](AuthResponse resp){ + send_ex_message(serializeAuthResponse(resp)); + }); } AOApplication::~AOApplication() diff --git a/src/aoapplication.h b/src/aoapplication.h index a2c88ca..5d84015 100644 --- a/src/aoapplication.h +++ b/src/aoapplication.h @@ -6,7 +6,7 @@ #include "discord_rich_presence.h" #include "ext_packet.h" #include "keyring.h" -#include "saved_auth.h" +#include "auth_flow.h" #include "serverdata.h" #include "widgets/aooptionsdialog.h" @@ -354,8 +354,7 @@ public: ma_decoder_config mus_decoder_config; KeyringModel keyring_model; - SavedAuth saved_auth; - QString ex_auth_username; + AuthFlow auth; private: QVector server_list; diff --git a/src/auth_flow.cpp b/src/auth_flow.cpp index 07544e8..8d164de 100644 --- a/src/auth_flow.cpp +++ b/src/auth_flow.cpp @@ -3,31 +3,55 @@ #include #include #include +#include +#include +#include #include "auth_flow.h" -#include "keyring.h" #include "file_functions.h" +void AuthFlow::initialize(KeyringModel *model) +{ + m_model = model; + m_saved_auth.load(); +} + // This function is supposed to open the authentication dialog with various // fields like method selection and fields to enter password or select a key, -// but for now, it'll simply submit a public key auth request. Hostname -// parameter is unused. -void start_auth_flow(AOApplication *ao_app, QString username) +// but for now, it'll simply submit a public key auth request. +void AuthFlow::start(QString user, QString host) { - ao_app->ex_auth_username = username; - AuthRequest req; - req.username = username; - req.method = AuthMethod::certificate; - ao_app->send_ex_message(serializeAuthRequest(req)); + if (m_phase == AuthPhase::requested) + { + reset(); + } + + m_username = user; + m_hostname = host; + m_key_dlg = new KeySelectDialog(m_model, m_username, m_hostname); + m_key_dlg->setWindowModality(Qt::ApplicationModal); + connect(m_key_dlg, &KeySelectDialog::key_selected, this, &AuthFlow::on_key_selected); + connect(m_key_dlg, &QDialog::rejected, this, &AuthFlow::reset); + + auto saved_key_id = m_saved_auth.lookup(host.toUtf8(), user.toUtf8()); + if (!saved_key_id.isEmpty()) + { + m_mode = FlowMode::Saved; + on_key_selected(acquire_keyring_key(saved_key_id)); + } + else + { + m_mode = FlowMode::Default; + m_key_dlg->show(); + } } -KeySelectDialog::KeySelectDialog(KeyringModel *model, QStringView username, QWidget *parent) - : QDialog(parent) +KeySelectDialog::KeySelectDialog(KeyringModel *model, QStringView username, QStringView hostname, QWidget *parent) + : QDialog(parent) , m_model(model) { - this->setAttribute(Qt::WA_DeleteOnClose); - this->setWindowTitle(QString("Select key for %1").arg(username)); + this->setWindowTitle(QString("Select key for %1@%2").arg(username, hostname)); auto view = new QTableView(this); view->setModel(m_model); view->horizontalHeader()->setSectionResizeMode(QHeaderView::Stretch); @@ -46,10 +70,9 @@ KeySelectDialog::KeySelectDialog(KeyringModel *model, QStringView username, QWid return; } - QByteArray key_id = m_model->data(rows.first(), KeyringModel::KeyIDRole).toByteArray(); - QString key_name = m_model->data(rows.first()).toString(); + QByteArray selected_key_id = m_model->data(rows.first(), KeyringModel::KeyIDRole).toByteArray(); - emit key_selected(key_id, key_name); + emit key_selected(acquire_keyring_key(selected_key_id)); }); connect(view->selectionModel(), &QItemSelectionModel::selectionChanged, this, [view, buttons](const QItemSelection &, const QItemSelection &) { bool selected = !view->selectionModel()->selectedRows().isEmpty(); @@ -57,15 +80,28 @@ KeySelectDialog::KeySelectDialog(KeyringModel *model, QStringView username, QWid }); } -KeyPassphraseDialog::KeyPassphraseDialog(QStringView key_name, QWidget *parent) - : QDialog(parent) +KeyActivationDialog::KeyActivationDialog(Mode mode, QStringView key_name, QWidget *parent) + : QDialog(parent) { - this->setWindowTitle(QString("Enter passphrase for key %1").arg(key_name)); + this->setWindowTitle(QString("Unlock key %1").arg(key_name)); QVBoxLayout *pw_layout = new QVBoxLayout(this); - pw_layout->addWidget(new QLabel(QStringLiteral("Passphrase:"), this)); - m_pw_line = new QLineEdit(this); - m_pw_line->setEchoMode(QLineEdit::Password); - pw_layout->addWidget(m_pw_line); + + QString mode_prompt; + switch (mode) { + case Mode::passphrase: + mode_prompt = QStringLiteral("Passphrase:"); + break; + case Mode::pin: + mode_prompt = QStringLiteral("PIN:"); + break; + default: + break; + } + + pw_layout->addWidget(new QLabel(mode_prompt, this)); + m_line = new QLineEdit(this); + m_line->setEchoMode(QLineEdit::Password); + pw_layout->addWidget(m_line); QDialogButtonBox *pw_buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, this); pw_layout->addWidget(pw_buttons); m_err_lbl = new QLabel(this); @@ -73,87 +109,163 @@ KeyPassphraseDialog::KeyPassphraseDialog(QStringView key_name, QWidget *parent) pw_layout->addWidget(m_err_lbl); connect(pw_buttons, &QDialogButtonBox::rejected, this, &QDialog::reject); connect(pw_buttons, &QDialogButtonBox::accepted, this, [this] { - emit passphrase_submitted(m_pw_line->text().toUtf8()); - m_pw_line->clear(); - // zero the buffer + emit input_submitted(m_line->text().toUtf8()); + m_line->clear(); }); } -void KeyPassphraseDialog::display_error(ResponseResult error) const +QString auth_error_to_string(ResponseResult error) { - QString error_text; switch (error) { case ResponseResult::unsupported_version: - error_text = QStringLiteral("Unsupported key version."); - break; + return QStringLiteral("Unsupported key version."); case ResponseResult::decryption_failed: - error_text = QStringLiteral("Wrong passphrase (or corrupted key)."); - break; + return QStringLiteral("Wrong passphrase (or corrupted key)."); + case ResponseResult::hardware_fault: + return QStringLiteral("Hardware rejected this operation. It might be due to corrupted key, invalid server challenge, or a TPM fault."); + case ResponseResult::invalid_pin: + return QStringLiteral("Invalid PIN."); + case ResponseResult::hardware_lockout: + return QStringLiteral("Your hardware is locked out after too many failed PIN attempts. Try again later."); + case ResponseResult::incompatible_arguments: + return QStringLiteral("Server authentication incompatible with your key."); default: - error_text = QString("Error unlocking key (code %1). Catastrophic failure.").arg((int)error); - break; + return QString("Error unlocking key (code %1). Catastrophic failure.").arg((int)error); } - m_err_lbl->setText(error_text); +} + +void KeyActivationDialog::display_error(ResponseResult error) const +{ + m_err_lbl->setText(auth_error_to_string(error)); m_err_lbl->setVisible(true); } -AuthFlow::AuthFlow(AOApplication *ao_app, const AuthChallenge &challenge, QWidget *parent) - : QObject(parent) - , m_ao_app(ao_app) - , m_challenge(challenge) +void AuthFlow::on_key_selected(KeyringKey key) { - auto saved_key = ao_app->saved_auth.lookup(ao_app->m_serverdata.m_server_hostname.toUtf8(), ao_app->ex_auth_username.toUtf8()); - if (!saved_key.isEmpty()) + m_key = key; + // Choosing the key for the first request. + if (m_phase == AuthPhase::none) { - m_mode = FlowMode::Saved; - m_key_dlg = nullptr; - on_key_selected(saved_key, QString("%1@%2 (saved)").arg(ao_app->ex_auth_username, ao_app->m_serverdata.m_server_hostname)); + AuthRequest req = { + .username = m_username, + .method = AuthMethod::certificate, + .variant = m_key.type == KeyringStorageType::hardware ? CertVariant::hardware : CertVariant::software, + }; + + m_phase = AuthPhase::requested; + + emit request_ready(req); } - else + // Already authenticating; choosing a different key for the same request. + // This can be erroneous if a server challenge is issued for a different type + // of key (software X25519 vs hardware P-256), creating a mismatch. + else if (m_phase == AuthPhase::requested) { - m_mode = FlowMode::Default; - m_key_dlg = new KeySelectDialog(&ao_app->keyring_model, ao_app->ex_auth_username, parent); - connect(m_key_dlg, &KeySelectDialog::key_selected, this, &AuthFlow::on_key_selected); - m_key_dlg->open(); + respond(m_challenge); } } -void AuthFlow::on_key_selected(QByteArrayView key_id, QStringView key_name) +void AuthFlow::authenticate(QByteArrayView input) { - m_pwd_dlg = new KeyPassphraseDialog(key_name, m_key_dlg); - connect(m_pwd_dlg, &KeyPassphraseDialog::passphrase_submitted, this, [this, key_id](QByteArrayView passphrase) { - AuthResponse response; - ResponseResult result = unlock_and_auth(key_id, passphrase, m_challenge.challenge, m_ao_app->ex_auth_username.toUtf8(), response); - if (result == ResponseResult::success) - { - m_ao_app->send_ex_message(serializeAuthResponse(response)); - if (m_mode == FlowMode::Default) - { - m_key_dlg->accept(); - m_ao_app->saved_auth.insert(m_ao_app->m_serverdata.m_server_hostname.toUtf8(), m_ao_app->ex_auth_username.toUtf8(), key_id); - } - else - { - m_pwd_dlg->accept(); - } - deleteLater(); - } - else + AuthResponse response; + ResponseResult result = unlock_and_auth(m_key, input, m_challenge.challenge, m_username.toUtf8(), response); + if (result == ResponseResult::success) + { + emit response_ready(response); + if (m_mode == FlowMode::Default) { - m_pwd_dlg->display_error(result); + m_saved_auth.insert(m_hostname.toUtf8(), m_username.toUtf8(), m_key.id); } - }); - connect(m_pwd_dlg, &QDialog::rejected, this, [this] { - if (m_mode == FlowMode::Saved) - { - m_ao_app->saved_auth.remove(m_ao_app->m_serverdata.m_server_hostname.toUtf8(), m_ao_app->ex_auth_username.toUtf8()); - m_mode = FlowMode::Default; + reset(); + } + // The error behavior depends on whether we need the application-level key + // unlock dialog (for TSS2) or not (for Windows CNG, which uses its own). + else if (m_input_dlg) + { + m_input_dlg->display_error(result); + } + else + { + QMessageBox::warning(nullptr, "Error", auth_error_to_string(result)); + reject_and_select_new_key(); + } +} - m_key_dlg = new KeySelectDialog(&m_ao_app->keyring_model, m_ao_app->ex_auth_username); - connect(m_key_dlg, &KeySelectDialog::key_selected, this, &AuthFlow::on_key_selected); - m_key_dlg->open(); - } - }); - m_pwd_dlg->open(); +void AuthFlow::reject_and_select_new_key() +{ + if (m_mode == FlowMode::Saved) + { + m_saved_auth.remove(m_hostname.toUtf8(), m_username.toUtf8()); + m_mode = FlowMode::Default; + } + + if (m_input_dlg) + { + m_input_dlg->deleteLater(); + } + + // The key selection dialog remains in the memory while the authentication + // attempt is performed. It's only cleared on finalization of the entire flow. + m_key_dlg->show(); +} + +void AuthFlow::call_input_dialog(KeyActivationDialog::Mode mode) +{ + m_input_dlg = new KeyActivationDialog(mode, m_key.name); + m_input_dlg->setWindowModality(Qt::ApplicationModal); + + connect(m_input_dlg, &KeyActivationDialog::input_submitted, this, &AuthFlow::authenticate); + connect(m_input_dlg, &QDialog::rejected, this, &AuthFlow::reject_and_select_new_key); + m_key_dlg->hide(); + m_input_dlg->show(); +} + +bool AuthFlow::awaiting_challenge() const +{ + return m_phase == AuthPhase::requested; +} + +void AuthFlow::respond(const AuthChallenge &challenge) +{ + if (m_phase != AuthPhase::requested) + { + return; + } + + m_challenge = challenge; + + switch (m_key.type) { + case KeyringStorageType::software: + call_input_dialog(KeyActivationDialog::Mode::passphrase); + break; + case KeyringStorageType::hardware: +#ifdef SOF_AO_HARDWARE_KEY_REQUIRES_PIN_UI + call_input_dialog(KeyActivationDialog::Mode::pin); +#else + m_key_dlg->hide(); + authenticate(QByteArray()); +#endif + break; + default: + QMessageBox::warning(nullptr, "Error", QStringLiteral("Unable to use this key.")); + break; + } +} + +void AuthFlow::reset() +{ + if (m_key_dlg) + { + m_key_dlg->deleteLater(); + m_key_dlg = nullptr; + } + + if (m_input_dlg) + { + m_input_dlg->deleteLater(); + m_input_dlg = nullptr; + } + + m_phase = AuthPhase::none; } diff --git a/src/auth_flow.h b/src/auth_flow.h index 942e1aa..4a89673 100644 --- a/src/auth_flow.h +++ b/src/auth_flow.h @@ -1,51 +1,57 @@ #pragma once #include +#include -#include "aoapplication.h" #include "ext_packet.h" #include "keyring.h" +#include "saved_auth.h" class KeySelectDialog : public QDialog { Q_OBJECT public: - explicit KeySelectDialog(KeyringModel *model, QStringView username, QWidget *parent = nullptr); + explicit KeySelectDialog(KeyringModel *model, QStringView username, QStringView hostname, QWidget *parent = nullptr); private: KeyringModel *m_model; signals: - void key_selected(QByteArrayView key_id, QStringView key_name); + void key_selected(KeyringKey key); }; -class KeyPassphraseDialog : public QDialog +class KeyActivationDialog : public QDialog { Q_OBJECT public: - explicit KeyPassphraseDialog(QStringView key_name, QWidget *parent = nullptr); + enum class Mode { + none, + passphrase, + pin, + }; + explicit KeyActivationDialog(Mode mode, QStringView key_name, QWidget *parent = nullptr); void display_error(ResponseResult error) const; private: QLabel *m_err_lbl; - QLineEdit *m_pw_line; + QLineEdit *m_line; signals: - void passphrase_submitted(QByteArrayView passphrase); + void input_submitted(QByteArrayView input); }; -// The only reason this inherits QObject is so I can call deleteLater() -// when the authentication finishes. Auth flow involves coordinating two -// asynchronous dialogs emitting signals, so I'd rather let Qt's event loop -// clean everything up to be safe. +// The only reason this inherits QObject is signals. class AuthFlow : public QObject { Q_OBJECT public: - explicit AuthFlow(AOApplication *ao_app, const AuthChallenge &challenge, QWidget *parent = nullptr); + void initialize(KeyringModel *model); + void start(QString user, QString host); + bool awaiting_challenge(void) const; + void respond(const AuthChallenge &challenge); private: enum class FlowMode @@ -54,13 +60,29 @@ private: Saved }; + enum class AuthPhase { + none, + requested, + }; + + QString m_username; + QString m_hostname; FlowMode m_mode; - AOApplication *m_ao_app; AuthChallenge m_challenge; - KeySelectDialog *m_key_dlg; - KeyPassphraseDialog *m_pwd_dlg; + KeySelectDialog *m_key_dlg = nullptr; + KeyActivationDialog *m_input_dlg = nullptr; + AuthPhase m_phase = AuthPhase::none; + SavedAuth m_saved_auth; + KeyringKey m_key; + KeyringModel *m_model; - void on_key_selected(QByteArrayView key_id, QStringView key_name); -}; + void on_key_selected(KeyringKey key); + void call_input_dialog(KeyActivationDialog::Mode mode); + void authenticate(QByteArrayView input); + void reject_and_select_new_key(void); + void reset(void); -void start_auth_flow(AOApplication *ao_app, QString username); +signals: + void request_ready(AuthRequest req); + void response_ready(AuthResponse response); +}; diff --git a/src/courtroom.cpp b/src/courtroom.cpp index a2dca24..d73bba9 100644 --- a/src/courtroom.cpp +++ b/src/courtroom.cpp @@ -5125,9 +5125,10 @@ void Courtroom::on_ooc_return_pressed() if (command.size() != 2) { append_server_chatmessage("CLIENT", "Usage: /auth ", "1"); + ui_ooc_chat_message->clear(); return; } - start_auth_flow(ao_app, command[1]); + ao_app->auth.start(command[1], ao_app->m_serverdata.m_server_hostname); ui_ooc_chat_message->clear(); return; } diff --git a/src/ext_distribution.cpp b/src/ext_distribution.cpp index 42f4156..81039bb 100644 --- a/src/ext_distribution.cpp +++ b/src/ext_distribution.cpp @@ -1,6 +1,3 @@ -// Copyright 2026 Osmium Sorcerer -// SPDX-License-Identifier: MIT - #include "ext_packet.h" #include "aoapplication.h" @@ -11,11 +8,10 @@ #include #include -static void handleAuthChallenge(QByteArrayView body, AOApplication *ao) +static void handleAuthChallenge(QByteArrayView body, AuthFlow *auth) { - if (ao->ex_auth_username.isEmpty()) + if (!auth->awaiting_challenge()) { - // We're not authenticating, ignore. return; } @@ -26,7 +22,7 @@ static void handleAuthChallenge(QByteArrayView body, AOApplication *ao) return; } - new AuthFlow(ao, challenge); + auth->respond(challenge); } void AOApplication::ex_message_received(QByteArrayView message) @@ -36,7 +32,7 @@ void AOApplication::ex_message_received(QByteArrayView message) switch (type) { case ExMsgType::auth_challenge: - handleAuthChallenge(body, this); + handleAuthChallenge(body, &this->auth); break; default: qWarning() << "Unknown message type:" << (int)type; diff --git a/src/ext_packet.cpp b/src/ext_packet.cpp index 76ac492..06cce91 100644 --- a/src/ext_packet.cpp +++ b/src/ext_packet.cpp @@ -17,11 +17,16 @@ QByteArray serializeIdent(const Ident &m) QByteArray serializeAuthRequest(const AuthRequest &m) { + quint32 extended_method = (quint32)m.method; + if (m.variant == CertVariant::hardware) + { + extended_method = 4; + } QByteArray msg; const QByteArray username = m.username.toUtf8(); uint8_t method[sizeof(quint32) + 1]; uint8_t ulen[sizeof(quint32) + 1]; - size_t method_n = vli32_encode(method, (quint32)m.method); + size_t method_n = vli32_encode(method, extended_method); size_t ulen_n = vli32_encode(ulen, username.size()); msg.reserve(2 + ulen_n + username.size() + method_n); msg.append((char)ExMsgType::auth_request); @@ -44,10 +49,26 @@ QByteArray serializeAuthResponse(const AuthResponse &m) bool parseAuthChallenge(QByteArrayView in, AuthChallenge &out) { - if (in.size() < 1 + 32) + if (in.size() < 1) { return false; } - out.challenge = QByteArray(in.constData() + 1, 32); - return true; + switch ((uchar)in.at(0)) { + case 0x00: + if (in.size() < 32 + 1) + { + return false; + } + out.challenge = QByteArray(in.constData() + 1, 32); + return true; + case 0x80: + if (in.size() < 64 + 1) + { + return false; + } + out.challenge = QByteArray(in.constData() + 1, 64); + return true; + default: + return false; + } } diff --git a/src/ext_packet.h b/src/ext_packet.h index c6c77c4..2697217 100644 --- a/src/ext_packet.h +++ b/src/ext_packet.h @@ -28,12 +28,18 @@ struct Ident quint8 version; }; +enum class CertVariant +{ + software, + hardware, +}; + struct AuthRequest { QString username; AuthMethod method; - // Will be replaced if passwords are implemented. - QByteArray credentials; + // Added as an extension. To be reimplemented in version 3. + CertVariant variant; }; struct AuthChallenge diff --git a/src/main.cpp b/src/main.cpp index 7b42925..f717b35 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -90,7 +90,7 @@ int main(int argc, char *argv[]) return EXIT_FAILURE; } main_app.keyring_model.load_keys(); - main_app.saved_auth.load(); + main_app.auth.initialize(&main_app.keyring_model); main_app.construct_lobby(); main_app.net_manager->get_server_list(); -- cgit