Skip to content

Commit 798f81e

Browse files
saturnericsaturneric
committed
fix(keygen): disambiguate hybrid algo selection
* Ensures algorithm combo selections stay stable when multiple variants share the same visible name by matching on stored id/type/length data * Preserves secondary algorithm choice when changing key length, resetting to none only when no longer supported * Improves hybrid subkey UI consistency and validation by handling empty/invalid selections and clearing dependent fields * Updates success/not-supported dialogs for clearer, engine-agnostic wording * Normalizes ML-DSA display name to avoid misleading variant labeling
1 parent cb673a8 commit 798f81e

5 files changed

Lines changed: 342 additions & 138 deletions

File tree

‎src/core/model/GpgKeyGenerateInfo.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ const QContainer<KeyAlgo> KeyGenerateInfo::kPrimaryKeyAlgos = {
232232

233233
const QContainer<KeyAlgo> KeyGenerateInfo::kHybridPrimaryKeyAlgo = {
234234
{"mldsa65",
235-
"ML-DSA-65",
235+
"ML-DSA",
236236
"HYBRID-SIGN",
237237
65,
238238
kSIGN | kAUTH,
@@ -246,7 +246,7 @@ const QContainer<KeyAlgo> KeyGenerateInfo::kHybridPrimaryKeyAlgo = {
246246
},
247247
}},
248248
{"mldsa87",
249-
"ML-DSA-87",
249+
"ML-DSA",
250250
"HYBRID-SIGN",
251251
87,
252252
kSIGN | kAUTH,
@@ -637,7 +637,7 @@ const QContainer<KeyAlgo> KeyGenerateInfo::kHybridSubKeyAlgos = {
637637
},
638638
}},
639639
{"mldsa65",
640-
"ML-DSA-65",
640+
"ML-DSA",
641641
"HYBRID-SIGN",
642642
65,
643643
kSIGN | kAUTH,
@@ -651,7 +651,7 @@ const QContainer<KeyAlgo> KeyGenerateInfo::kHybridSubKeyAlgos = {
651651
},
652652
}},
653653
{"mldsa87",
654-
"ML-DSA-87",
654+
"ML-DSA",
655655
"HYBRID-SIGN",
656656
87,
657657
kSIGN | kAUTH,

‎src/ui/UserInterfaceUtils.cpp‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,11 @@ CommonUtils::CommonUtils() : QWidget(nullptr) {
144144
}
145145

146146
void CommonUtils::RaiseMessageBox(QWidget *parent, GpgError err) {
147-
GpgErrorDesc desc = DescribeGpgErrCode(err);
148147
GpgErrorCode err_code = CheckGpgError2ErrCode(err);
149148

150149
if (err_code == GPG_ERR_NO_ERROR) {
151150
QMessageBox::information(parent, tr("Success"),
152-
tr("Gpg Operation succeed."));
151+
tr("Operation completed successfully."));
153152
} else {
154153
RaiseFailureMessageBox(parent, err);
155154
}
@@ -158,8 +157,8 @@ void CommonUtils::RaiseMessageBox(QWidget *parent, GpgError err) {
158157
void CommonUtils::RaiseMessageBoxNotSupported(QWidget *parent) {
159158
QMessageBox::warning(
160159
parent, tr("Operation Not Supported"),
161-
tr("The current GnuPG version is too low and does not support this "
162-
"operation. Please upgrade your GnuPG version to continue."));
160+
tr("The current OpenPGP engine does not support this operation. "
161+
"Please use a supported engine or upgrade the engine version."));
163162
}
164163

165164
void CommonUtils::RaiseFailureMessageBox(QWidget *parent, GpgError err,

‎src/ui/dialog/key_generate/KeyGenerateDialog.cpp‎

Lines changed: 112 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,31 @@ void SetGridLayoutCompact(QGridLayout* layout) {
361361

362362
#endif
363363

364+
auto ComboCurrentAlgo(
365+
QComboBox* combo,
366+
const GpgFrontend::QContainer<GpgFrontend::KeyAlgo>& algos)
367+
-> GpgFrontend::KeyAlgo {
368+
if (combo == nullptr) return GpgFrontend::KeyGenerateInfo::kNoneAlgo;
369+
370+
const auto data = combo->currentData().toMap();
371+
const auto id = data.value("id").toString();
372+
const auto type = data.value("type").toString();
373+
const auto key_length = data.value("length").toInt();
374+
375+
auto it = std::find_if(algos.cbegin(), algos.cend(),
376+
[&](const GpgFrontend::KeyAlgo& algo) {
377+
if (algo.Id() != id || algo.Type() != type)
378+
return false;
379+
380+
// Some old itemData may not contain length.
381+
if (key_length <= 0) return true;
382+
383+
return algo.KeyLength() == key_length;
384+
});
385+
386+
return it != algos.cend() ? *it : GpgFrontend::KeyGenerateInfo::kNoneAlgo;
387+
}
388+
364389
} // namespace
365390

366391
namespace GpgFrontend::UI {
@@ -1041,24 +1066,21 @@ void KeyGenerateDialog::set_signal_slot_config() {
10411066
refresh_hybrid_algo_widgets_state();
10421067
});
10431068

1044-
connect(ui_->pScndAlgoComboBox, &QComboBox::currentTextChanged, this,
1045-
[this](const QString& text) -> void {
1046-
Q_UNUSED(text)
1069+
connect(
1070+
ui_->pScndAlgoComboBox, qOverload<int>(&QComboBox::currentIndexChanged),
1071+
this, [this](int index) -> void {
1072+
if (index < 0) {
1073+
gen_key_info_->SetSubAlgo(KeyGenerateInfo::kNoneAlgo);
1074+
return;
1075+
}
10471076

1048-
const auto sub_algos = gen_key_info_->GetAlgo().SubAlgos(channel_);
1049-
const auto [name, type] =
1050-
ComboCurrentNameType(ui_->pScndAlgoComboBox);
1077+
const auto sub_algos = gen_key_info_->GetAlgo().SubAlgos(channel_);
1078+
const auto algo = ComboCurrentAlgo(ui_->pScndAlgoComboBox, sub_algos);
10511079

1052-
auto [found, algo] = GetAlgoByNameType(name, type, sub_algos);
1053-
if (found) {
1054-
gen_key_info_->SetSubAlgo(algo);
1055-
slot_set_easy_key_algo_2_custom();
1056-
} else {
1057-
gen_key_info_->SetSubAlgo(KeyGenerateInfo::kNoneAlgo);
1058-
}
1059-
1060-
refresh_primary_hybrid_algo_widgets_state();
1061-
});
1080+
gen_key_info_->SetSubAlgo(algo);
1081+
slot_set_easy_key_algo_2_custom();
1082+
refresh_primary_hybrid_algo_widgets_state();
1083+
});
10621084

10631085
connect(ui_->easyProfileComboBox,
10641086
qOverload<int>(&QComboBox::currentIndexChanged), this,
@@ -1096,23 +1118,56 @@ void KeyGenerateDialog::set_signal_slot_config() {
10961118
auto [found, algo] = GetAlgoByNameTypeAndKeyLength(
10971119
name, type, text.toInt(), supported_primary_key_algos_);
10981120

1099-
if (found) {
1100-
gen_key_info_->SetAlgo(algo);
1101-
slot_set_easy_key_algo_2_custom();
1121+
if (!found) return;
1122+
1123+
const auto old_sub_algo = gen_key_info_->SubAlgo();
1124+
1125+
gen_key_info_->SetAlgo(algo);
1126+
1127+
const auto sub_algos = gen_key_info_->GetAlgo().SubAlgos(channel_);
1128+
if (old_sub_algo != KeyGenerateInfo::kNoneAlgo &&
1129+
std::any_of(sub_algos.cbegin(), sub_algos.cend(),
1130+
[&](const KeyAlgo& sub_algo) {
1131+
return sub_algo == old_sub_algo;
1132+
})) {
1133+
gen_key_info_->SetSubAlgo(old_sub_algo);
1134+
} else {
1135+
gen_key_info_->SetSubAlgo(KeyGenerateInfo::kNoneAlgo);
11021136
}
1137+
1138+
slot_set_easy_key_algo_2_custom();
1139+
refresh_primary_hybrid_algo_widgets_state();
11031140
});
11041141

11051142
connect(ui_->sKeyLengthComboBox, &QComboBox::currentTextChanged, this,
11061143
[this](const QString& text) -> void {
1144+
if (gen_subkey_info_ == nullptr) return;
1145+
11071146
const auto [name, type] = ComboCurrentNameType(ui_->sAlgoComboBox);
11081147

11091148
auto [found, algo] = GetAlgoByNameTypeAndKeyLength(
11101149
name, type, text.toInt(), supported_subkey_algos_);
11111150

1112-
if (found) {
1113-
gen_subkey_info_->SetAlgo(algo);
1114-
slot_set_easy_key_algo_2_custom();
1151+
if (!found) return;
1152+
1153+
const auto old_sub_algo = gen_subkey_info_->SubAlgo();
1154+
1155+
gen_subkey_info_->SetAlgo(algo);
1156+
1157+
const auto sub_algos =
1158+
gen_subkey_info_->GetAlgo().SubAlgos(channel_);
1159+
if (old_sub_algo != KeyGenerateInfo::kNoneAlgo &&
1160+
std::any_of(sub_algos.cbegin(), sub_algos.cend(),
1161+
[&](const KeyAlgo& sub_algo) {
1162+
return sub_algo == old_sub_algo;
1163+
})) {
1164+
gen_subkey_info_->SetSubAlgo(old_sub_algo);
1165+
} else {
1166+
gen_subkey_info_->SetSubAlgo(KeyGenerateInfo::kNoneAlgo);
11151167
}
1168+
1169+
slot_set_easy_key_algo_2_custom();
1170+
refresh_widgets_state();
11161171
});
11171172

11181173
connect(ui_->pScndKeyLengthComboBox, &QComboBox::currentTextChanged, this,
@@ -1124,10 +1179,14 @@ void KeyGenerateDialog::set_signal_slot_config() {
11241179
auto [found, algo] = GetAlgoByNameTypeAndKeyLength(
11251180
name, type, text.toInt(), sub_algos);
11261181

1127-
if (found) {
1128-
gen_key_info_->SetSubAlgo(algo);
1129-
slot_set_easy_key_algo_2_custom();
1130-
}
1182+
if (!found) return;
1183+
1184+
gen_key_info_->SetSubAlgo(algo);
1185+
slot_set_easy_key_algo_2_custom();
1186+
1187+
// Keep combo display consistent, especially when multiple variants
1188+
// share the same visible name.
1189+
refresh_primary_hybrid_algo_widgets_state();
11311190
});
11321191

11331192
connect(this, &KeyGenerateDialog::SignalKeyGenerated,
@@ -1144,21 +1203,20 @@ void KeyGenerateDialog::set_signal_slot_config() {
11441203
&KeyGenerateDialog::slot_reset_easy_profile_config_to_default);
11451204

11461205
connect(
1147-
ui_->scndAlgoComboBox, &QComboBox::currentTextChanged, this,
1148-
[=](const QString& text) -> void {
1149-
Q_UNUSED(text)
1150-
1206+
ui_->scndAlgoComboBox, qOverload<int>(&QComboBox::currentIndexChanged),
1207+
this, [this](int index) -> void {
11511208
if (gen_subkey_info_ == nullptr) return;
11521209

1153-
const auto sub_algos = gen_subkey_info_->GetAlgo().SubAlgos(channel_);
1154-
const auto [name, type] = ComboCurrentNameType(ui_->scndAlgoComboBox);
1155-
1156-
auto [found, algo] = GetAlgoByNameType(name, type, sub_algos);
1157-
if (found) {
1158-
gen_subkey_info_->SetSubAlgo(algo);
1159-
slot_set_easy_key_algo_2_custom();
1210+
if (index < 0) {
1211+
gen_subkey_info_->SetSubAlgo(KeyGenerateInfo::kNoneAlgo);
1212+
return;
11601213
}
11611214

1215+
const auto sub_algos = gen_subkey_info_->GetAlgo().SubAlgos(channel_);
1216+
const auto algo = ComboCurrentAlgo(ui_->scndAlgoComboBox, sub_algos);
1217+
1218+
gen_subkey_info_->SetSubAlgo(algo);
1219+
slot_set_easy_key_algo_2_custom();
11621220
refresh_hybrid_algo_widgets_state();
11631221
});
11641222

@@ -1705,24 +1763,28 @@ void KeyGenerateDialog::refresh_primary_hybrid_algo_widgets_state() {
17051763
PopulateAlgoComboBox(ui_->pScndAlgoComboBox, sub_algos);
17061764
ui_->pScndAlgoComboBox->blockSignals(false);
17071765

1708-
if (!gen_key_info_->SubAlgo().Id().isEmpty() &&
1709-
gen_key_info_->SubAlgo() != KeyGenerateInfo::kNoneAlgo) {
1710-
ui_->pScndAlgoComboBox->blockSignals(true);
1711-
SetComboCurrentAlgo(ui_->pScndAlgoComboBox, gen_key_info_->SubAlgo());
1712-
ui_->pScndAlgoComboBox->blockSignals(false);
1766+
const auto current_sub_algo = gen_key_info_->SubAlgo();
1767+
1768+
const bool current_sub_algo_supported =
1769+
current_sub_algo != KeyGenerateInfo::kNoneAlgo &&
1770+
std::any_of(
1771+
sub_algos.cbegin(), sub_algos.cend(),
1772+
[&](const KeyAlgo& algo) { return algo == current_sub_algo; });
1773+
1774+
ui_->pScndAlgoComboBox->blockSignals(true);
1775+
1776+
if (current_sub_algo_supported) {
1777+
SetComboCurrentAlgo(ui_->pScndAlgoComboBox, current_sub_algo);
17131778
} else if (ui_->pScndAlgoComboBox->count() > 0) {
1714-
ui_->pScndAlgoComboBox->blockSignals(true);
17151779
ui_->pScndAlgoComboBox->setCurrentIndex(0);
17161780

1717-
const auto [id, type] = ComboCurrentIdType(ui_->pScndAlgoComboBox);
1718-
auto [found, algo] = GetAlgoByIdType(id, type, sub_algos);
1719-
if (found) {
1720-
gen_key_info_->SetSubAlgo(algo);
1721-
}
1722-
1723-
ui_->pScndAlgoComboBox->blockSignals(false);
1781+
const auto algo = ComboCurrentAlgo(ui_->pScndAlgoComboBox, sub_algos);
1782+
gen_key_info_->SetSubAlgo(algo);
1783+
} else {
1784+
gen_key_info_->SetSubAlgo(KeyGenerateInfo::kNoneAlgo);
17241785
}
17251786

1787+
ui_->pScndAlgoComboBox->blockSignals(false);
17261788
ui_->pScndKeyLengthLabel->setHidden(false);
17271789
ui_->pScndKeyLengthComboBox->setEnabled(true);
17281790
ui_->pScndKeyLengthComboBox->setHidden(false);

0 commit comments

Comments
 (0)