diff --git a/docs/topics/SSHAgent.adoc b/docs/topics/SSHAgent.adoc index 8b385c64c..e1f298dcb 100644 --- a/docs/topics/SSHAgent.adoc +++ b/docs/topics/SSHAgent.adoc @@ -177,4 +177,85 @@ If you chose to not autoload the key on database unlock, you can manually make t .SSH Agent Load Key from Context Menu image::sshagent_context_menu.png[] + + +=== Using destination constraints +SSH Agent destination constraints may be used to restrict the usage of an SSH key to specific hosts or to specific destinations. +This is especially useful when forwarding the SSH agent to machines that are not fully trustworthy. +The feature requires OpenSSH 8.9 or later on the client and server. +Please refer to https://www.openssh.com/agent-restrict.html for more details. + +To enable support for SSH Agent destination constraints, follow the steps below: + + 1. Select _Tools > Settings_ from the menu + 2. Select _SSH Agent_ category on the left sidebar + 3. Check _Enable destination contraints_ + +As of now KeepassXC lacks a UI for configuring the destination constraints on a specific SSH key. +However, the settings format is compatible to the Keepass plugin https://lechnology.com/software/keeagent/[KeeAgent]. +Therefore you could either load your database in _Keepass_ with _KeeAgent_ and use the _KeeAgent_ UI to configure the destination constraints. + +Alternatively you could alter the settings file manually: + + 1. Create a new entry, or open an existing entry in edit mode. + 2. Go to the advanced category and _Save_ the _KeeAgent.settings_ file to your filesystem + 3. Edit the file with your editor (see below) + 4. Add the edited file back to the SSH key entry in the database, overwriting the old version. + 5. Go to the SSH agent category and _Remove from agent_ and then _Add to agent_ + +The settings file is an UTF-16 encoded XML file consisting a large `EntrySettings` block. +This block contains an `UseDestinationConstraintWhenAdding` tag the is set to `false` by default. +You must set that to `true` to enable destination constraints on the specific SSH key entry. + The `DestinationConstraints` is a list of zero or more `Constraint` blocks. + Each `Constraint` block contains the origin and destination host both identified by their name (`FromHost` / `ToHost`) and their host keys (`FromHostKeys` / `ToHostKeys`). + An empty origin host (`FromHost` / `FromHostKeys`) depicts the machine the SSH agent running on. + The destination can be further restricted to a specific user by setting the user name in `ToUser`. +The host keys are in the format `$algorithm $pubkey` and can be retrieved with the command `ssh-keyscan $HOST` or from the `known_hosts` file. + +NOTE: Both return the format `$hostname $algorithm $pubkey`. Therefore the hostname must be omitted. + +The following example corresponds to the command + `ssh-add -h 'perseus@cetus.example.org' -h 'cetus.example.org>github.com' ~/.ssh/id_ed25519`: + + + + … + true + + + + + perseus + cetus.example.org + + + ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAZEGN4X9luxQr0Q+xEiB8NLK+E2m2/9DeT98hhiT8wn + false + + + + + cetus.example.org + + + ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAZEGN4X9luxQr0Q+xEiB8NLK+E2m2/9DeT98hhiT8wn + false + + + + github.com + + + ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEmKSENjQEezOmxkZMy7opKgwFB9nkt5YRrYMjNuG5N87uRgg6CLrbo5wAdT/y6v0mKV0U2w0WZ2YB/++Tpockg= + false + + + ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEmKSENjQEezOmxkZMy7opKgwFB9nkt5YRrYMjNuG5N87uRgg6CLrbo5wAdT/y6v0mKV0U2w0WZ2YB/++Tpockg= + false + + + + + + // end::content[] diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 183837fae..4f9291287 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -156,6 +156,14 @@ SSH Agent connection is working! + + Enable destination constraints + + + + Destination contrains can have unexpected side effects. Make sure to read the <a href="https://keepassxc.org/docs/KeePassXC_UserGuide#_using_destination_constraints">documentation</a>. + + ApplicationSettingsWidget @@ -9855,6 +9863,10 @@ This option is deprecated, use --set-key-file instead. All SSH identities removed from agent. + + Destination constraints are invalid or not supported by the agent (check options). + + SearchHelpWidget diff --git a/src/core/Config.cpp b/src/core/Config.cpp index f1a78830d..e0a413898 100644 --- a/src/core/Config.cpp +++ b/src/core/Config.cpp @@ -183,6 +183,7 @@ static const QHash configStrings = { {Config::SSHAgent_Enabled, {QS("SSHAgent/Enabled"), Roaming, false}}, {Config::SSHAgent_UseOpenSSH, {QS("SSHAgent/UseOpenSSH"), Roaming, false}}, {Config::SSHAgent_UsePageant, {QS("SSHAgent/UsePageant"), Roaming, true} }, + {Config::SSHAgent_EnableDestinationConstraints, {QS("SSHAgent/EnableDestinationConstraints"), Roaming, false} }, {Config::SSHAgent_AuthSockOverride, {QS("SSHAgent/AuthSockOverride"), Local, {}}}, {Config::SSHAgent_SecurityKeyProviderOverride, {QS("SSHAgent/SecurityKeyProviderOverride"), Local, {}}}, diff --git a/src/core/Config.h b/src/core/Config.h index b4f3bdc04..05cfa7871 100644 --- a/src/core/Config.h +++ b/src/core/Config.h @@ -162,6 +162,7 @@ public: SSHAgent_Enabled, SSHAgent_UseOpenSSH, SSHAgent_UsePageant, + SSHAgent_EnableDestinationConstraints, SSHAgent_AuthSockOverride, SSHAgent_SecurityKeyProviderOverride, diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index d2213a669..66f27e5d1 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/src/gui/entry/EditEntryWidget.cpp @@ -661,6 +661,12 @@ void EditEntryWidget::updateSSHAgentAttachments() setSSHAgentSettings(); } + if (KeeAgentSettings::inEntryAttachments(m_attachments.data())) { + m_sshAgentSettings.reset(); + m_sshAgentSettings.fromEntryAttachments(m_attachments.data()); + setSSHAgentSettings(); + } + m_sshAgentUi->attachmentComboBox->clear(); m_sshAgentUi->attachmentComboBox->addItem(""); @@ -728,6 +734,12 @@ void EditEntryWidget::updateSSHAgentKeyInfo() void EditEntryWidget::toKeeAgentSettings(KeeAgentSettings& settings) const { + // set from attachment to load settings aren't supported by the UI (e.g. + // destination constraints) + if (KeeAgentSettings::inEntryAttachments(m_attachments.data())) { + settings.fromEntryAttachments(m_attachments.data()); + } + settings.setAddAtDatabaseOpen(m_sshAgentUi->addKeyToAgentCheckBox->isChecked()); settings.setRemoveAtDatabaseClose(m_sshAgentUi->removeKeyFromAgentCheckBox->isChecked()); settings.setUseConfirmConstraintWhenAdding(m_sshAgentUi->requireUserConfirmationCheckBox->isChecked()); diff --git a/src/sshagent/AgentSettingsWidget.cpp b/src/sshagent/AgentSettingsWidget.cpp index bdad6e30e..8ddccbe68 100644 --- a/src/sshagent/AgentSettingsWidget.cpp +++ b/src/sshagent/AgentSettingsWidget.cpp @@ -35,7 +35,21 @@ AgentSettingsWidget::AgentSettingsWidget(QWidget* parent) m_ui->sshAuthSockMessageWidget->setVisible(sshAgent()->isEnabled()); m_ui->sshAuthSockMessageWidget->setCloseButtonVisible(false); m_ui->sshAuthSockMessageWidget->setAutoHideTimeout(-1); + + m_ui->destinationConstraintsMessageWidget->setCloseButtonVisible(false); + m_ui->destinationConstraintsMessageWidget->setAutoHideTimeout(-1); + m_ui->destinationConstraintsMessageWidget->showMessage( + tr("Destination contrains can have unexpected side effects. " + "Make sure to read the " + "documentation."), + MessageWidget::Warning); + m_ui->destinationConstraintsMessageWidget->setVisible(sshAgent()->enableDestinationConstraints()); + connect(m_ui->enableSSHAgentCheckBox, SIGNAL(stateChanged(int)), SLOT(toggleSettingsEnabled())); + connect(m_ui->enableDestinationConstraintsCheckBox, + SIGNAL(stateChanged(int)), + SLOT(toggleDestinationConstraintsEnabled())); } AgentSettingsWidget::~AgentSettingsWidget() @@ -66,6 +80,9 @@ void AgentSettingsWidget::loadSettings() m_ui->sshAuthSockMessageWidget->setVisible(sshAgentEnabled); + auto destinationConstraintsEnabled = sshAgent()->enableDestinationConstraints(); + m_ui->enableDestinationConstraintsCheckBox->setChecked(destinationConstraintsEnabled); + if (sshAgentEnabled) { #ifndef Q_OS_WIN if (sshAuthSock.isEmpty() && sshAuthSockOverride.isEmpty()) { @@ -98,6 +115,7 @@ void AgentSettingsWidget::saveSettings() sshAgent()->setUsePageant(m_ui->usePageantRadioButton->isChecked() || m_ui->useBothRadioButton->isChecked()); sshAgent()->setUseOpenSSH(m_ui->useOpenSSHRadioButton->isChecked() || m_ui->useBothRadioButton->isChecked()); #endif + sshAgent()->setEnableDestinationConstraints(m_ui->enableDestinationConstraintsCheckBox->isChecked()); sshAgent()->setEnabled(m_ui->enableSSHAgentCheckBox->isChecked()); } @@ -105,3 +123,8 @@ void AgentSettingsWidget::toggleSettingsEnabled() { m_ui->agentConfigPageBody->setEnabled(m_ui->enableSSHAgentCheckBox->isChecked()); } + +void AgentSettingsWidget::toggleDestinationConstraintsEnabled() +{ + m_ui->destinationConstraintsMessageWidget->setVisible(m_ui->enableDestinationConstraintsCheckBox->isChecked()); +} diff --git a/src/sshagent/AgentSettingsWidget.h b/src/sshagent/AgentSettingsWidget.h index 67c41ffde..265d2c9a1 100644 --- a/src/sshagent/AgentSettingsWidget.h +++ b/src/sshagent/AgentSettingsWidget.h @@ -39,6 +39,7 @@ public slots: void loadSettings(); void saveSettings(); void toggleSettingsEnabled(); + void toggleDestinationConstraintsEnabled(); private: QScopedPointer m_ui; diff --git a/src/sshagent/AgentSettingsWidget.ui b/src/sshagent/AgentSettingsWidget.ui index abd056c8a..99ae5e062 100644 --- a/src/sshagent/AgentSettingsWidget.ui +++ b/src/sshagent/AgentSettingsWidget.ui @@ -7,7 +7,7 @@ 0 0 400 - 300 + 443 @@ -93,6 +93,16 @@ + + + + Enable destination constraints + + + + + + @@ -107,42 +117,29 @@ 8 - - + + - SSH_AUTH_SOCK override + SSH_SK_PROVIDER override Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter - - - - SSH_AUTH_SOCK value - - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter - - - - - - - Qt::Vertical - - - - 20 - 40 - - - - + + + + SSH_SK_PROVIDER value + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + @@ -158,10 +155,36 @@ - - + + + + + + + Qt::Vertical + + + + 20 + 40 + + + + + + - SSH_SK_PROVIDER value + SSH_AUTH_SOCK value + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + SSH_AUTH_SOCK override Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter @@ -183,19 +206,6 @@ - - - - SSH_SK_PROVIDER override - - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter - - - - - - diff --git a/src/sshagent/KeeAgentSettings.cpp b/src/sshagent/KeeAgentSettings.cpp index a794eac93..ccf48f912 100644 --- a/src/sshagent/KeeAgentSettings.cpp +++ b/src/sshagent/KeeAgentSettings.cpp @@ -35,6 +35,29 @@ KeeAgentSettings::KeeAgentSettings() reset(); } +bool KeeAgentSettings::KeySpec::operator==(const KeeAgentSettings::KeySpec& other) const +{ + return (key == other.key && isCertificateAuthority == other.isCertificateAuthority); +} + +QByteArray KeeAgentSettings::KeySpec::getKeyBlob() const +{ + // In KeeAgent the key data is the second word in the string. First is the + // key type. Third is the key comment which is optional. + auto words = key.split(" "); + if (words.length() >= 2) { + return QByteArray::fromBase64(words[1].toLatin1(), QByteArray::Base64Encoding); + } else { + return QByteArray(); + } +} + +bool KeeAgentSettings::DestinationConstraint::operator==(const KeeAgentSettings::DestinationConstraint& other) const +{ + return (fromHost == other.fromHost && fromHostKeys == other.fromHostKeys && toUser == other.toUser + && toHost == other.toHost && toHostKeys == other.toHostKeys); +} + bool KeeAgentSettings::operator==(const KeeAgentSettings& other) const { // clang-format off @@ -43,6 +66,8 @@ bool KeeAgentSettings::operator==(const KeeAgentSettings& other) const && m_useConfirmConstraintWhenAdding == other.m_useConfirmConstraintWhenAdding && m_useLifetimeConstraintWhenAdding == other.m_useLifetimeConstraintWhenAdding && m_lifetimeConstraintDuration == other.m_lifetimeConstraintDuration + && m_useDestinationConstraintsWhenAdding == other.m_useDestinationConstraintsWhenAdding + && m_destinationConstraints == other.m_destinationConstraints && m_selectedType == other.m_selectedType && m_attachmentName == other.m_attachmentName && m_saveAttachmentToTempFile == other.m_saveAttachmentToTempFile @@ -77,6 +102,8 @@ void KeeAgentSettings::reset() m_useConfirmConstraintWhenAdding = false; m_useLifetimeConstraintWhenAdding = false; m_lifetimeConstraintDuration = 600; + m_useDestinationConstraintsWhenAdding = false; + m_destinationConstraints.clear(); m_selectedType = QStringLiteral("file"); m_attachmentName.clear(); @@ -125,6 +152,16 @@ int KeeAgentSettings::lifetimeConstraintDuration() const return m_lifetimeConstraintDuration; } +bool KeeAgentSettings::useDestinationConstraintsWhenAdding() const +{ + return m_useDestinationConstraintsWhenAdding; +} + +QList KeeAgentSettings::destinationConstraints() const +{ + return m_destinationConstraints; +} + const QString KeeAgentSettings::selectedType() const { return m_selectedType; @@ -180,6 +217,16 @@ void KeeAgentSettings::setLifetimeConstraintDuration(int lifetimeConstraintDurat m_lifetimeConstraintDuration = lifetimeConstraintDuration; } +void KeeAgentSettings::setUseDestinationConstraintsWhenAdding(bool useDestinationConstraintsWhenAdding) +{ + m_useDestinationConstraintsWhenAdding = useDestinationConstraintsWhenAdding; +} + +void KeeAgentSettings::setDestinationConstraints(const QList& destinationConstraints) +{ + m_destinationConstraints = destinationConstraints; +} + void KeeAgentSettings::setSelectedType(const QString& selectedType) { m_selectedType = selectedType; @@ -229,6 +276,8 @@ bool KeeAgentSettings::fromXml(const QByteArray& ba) QXmlStreamReader reader; reader.addData(ba); + reset(); + if (reader.error() || !reader.readNextStartElement()) { m_error = reader.errorString(); return false; @@ -273,6 +322,88 @@ bool KeeAgentSettings::fromXml(const QByteArray& ba) reader.skipCurrentElement(); } } + if (!reader.error()) + reader.readNext(); + } else if (reader.name() == "UseDestinationConstraintWhenAdding") { + m_useDestinationConstraintsWhenAdding = readBool(reader); + } else if (reader.name() == "DestinationConstraints") { + while (!reader.error() && reader.readNextStartElement()) { + if (reader.name() == "Constraint") { + KeeAgentSettings::DestinationConstraint constraint; + while (!reader.error() && reader.readNextStartElement()) { + if (reader.name() == "FromHostKeys" || reader.name() == "ToHostKeys") { + QString section = reader.name().toString(); + while (!reader.error() && reader.readNextStartElement()) { + if (reader.name() == "KeySpec") { + KeeAgentSettings::KeySpec keyspec; + while (!reader.error() && reader.readNextStartElement()) { + if (reader.name() == "HostKey") { + reader.readNext(); + keyspec.key = reader.text().toString(); + reader.readNext(); + } else if (reader.name() == "IsCA") { + keyspec.isCertificateAuthority = readBool(reader); + } else { + qWarning() << "Skipping KeySpec element" << reader.name(); + reader.skipCurrentElement(); + } + } + + if (keyspec.getKeyBlob().isEmpty()) { + return false; + } + + if (section == "FromHostKeys") { + constraint.fromHostKeys.append(std::move(keyspec)); + } else { + constraint.toHostKeys.append(std::move(keyspec)); + } + if (!reader.error()) + reader.readNext(); + } else { + qWarning() << "Skipping " << section << " element" << reader.name(); + reader.skipCurrentElement(); + } + } + if (!reader.error()) + reader.readNext(); + } else if (reader.name() == "FromHost") { + reader.readNext(); + constraint.fromHost = reader.text().toString(); + reader.readNext(); + } else if (reader.name() == "ToUser") { + reader.readNext(); + constraint.toUser = reader.text().toString(); + reader.readNext(); + } else if (reader.name() == "ToHost") { + reader.readNext(); + constraint.toHost = reader.text().toString(); + reader.readNext(); + } else { + qWarning() << "Skipping Constraint element" << reader.name(); + reader.skipCurrentElement(); + } + } + + if ((constraint.fromHost.isEmpty() && !constraint.fromHostKeys.isEmpty()) + || (!constraint.fromHost.isEmpty() && constraint.fromHostKeys.isEmpty())) { + return false; + } + if (constraint.toHost.isEmpty() || constraint.toHostKeys.isEmpty()) { + return false; + } + + m_destinationConstraints.append(std::move(constraint)); + + if (!reader.error()) + reader.readNext(); + } else { + qWarning() << "Skipping DestinationConstraints element" << reader.name(); + reader.skipCurrentElement(); + } + } + if (!reader.error()) + reader.readNext(); } else { qWarning() << "Skipping element" << reader.name(); reader.skipCurrentElement(); @@ -309,6 +440,53 @@ QByteArray KeeAgentSettings::toXml() const writer.writeTextElement("UseConfirmConstraintWhenAdding", m_useConfirmConstraintWhenAdding ? "true" : "false"); writer.writeTextElement("UseLifetimeConstraintWhenAdding", m_useLifetimeConstraintWhenAdding ? "true" : "false"); writer.writeTextElement("LifetimeConstraintDuration", QString::number(m_lifetimeConstraintDuration)); + writer.writeTextElement("UseDestinationConstraintWhenAdding", + m_useDestinationConstraintsWhenAdding ? "true" : "false"); + + writer.writeStartElement("DestinationConstraints"); + + foreach (const auto& constraint, m_destinationConstraints) { + writer.writeStartElement("Constraint"); + + if (constraint.fromHost.isEmpty()) { + writer.writeEmptyElement("FromHost"); + } else { + writer.writeTextElement("FromHost", constraint.fromHost); + } + + writer.writeStartElement("FromHostKeys"); + foreach (const auto& keyspec, constraint.fromHostKeys) { + writer.writeStartElement("KeySpec"); + writer.writeTextElement("HostKey", keyspec.key); + writer.writeTextElement("IsCA", keyspec.isCertificateAuthority ? "true" : "false"); + writer.writeEndElement(); // KeySpec + } + writer.writeEndElement(); // FromHostKeys + + if (constraint.toUser.isEmpty()) { + writer.writeEmptyElement("ToUser"); + } else { + writer.writeTextElement("ToUser", constraint.toUser); + } + if (constraint.toHost.isEmpty()) { + writer.writeEmptyElement("ToHost"); + } else { + writer.writeTextElement("ToHost", constraint.toHost); + } + + writer.writeStartElement("ToHostKeys"); + foreach (const auto& keyspec, constraint.toHostKeys) { + writer.writeStartElement("KeySpec"); + writer.writeTextElement("HostKey", keyspec.key); + writer.writeTextElement("IsCA", keyspec.isCertificateAuthority ? "true" : "false"); + writer.writeEndElement(); // KeySpec + } + writer.writeEndElement(); // ToHostKeys + + writer.writeEndElement(); // Constraint + } + + writer.writeEndElement(); // DestinationConstraints writer.writeStartElement("Location"); writer.writeTextElement("SelectedType", m_selectedType); @@ -355,7 +533,19 @@ bool KeeAgentSettings::inEntryAttachments(const EntryAttachments* attachments) */ bool KeeAgentSettings::fromEntry(const Entry* entry) { - const auto attachments = entry->attachments(); + return KeeAgentSettings::fromEntryAttachments(entry->attachments()); +} + +/** + * Read settings from entry attachments as an XML attachment. + * + * Sets error string on error. + * + * @param entry EntryAttachments to read the attachment from + * @return true if XML document was loaded + */ +bool KeeAgentSettings::fromEntryAttachments(const EntryAttachments* attachments) +{ if (attachments->hasKey("KeeAgent.settings")) { return fromXml(attachments->value("KeeAgent.settings")); } diff --git a/src/sshagent/KeeAgentSettings.h b/src/sshagent/KeeAgentSettings.h index ffc14044e..4b63019c3 100644 --- a/src/sshagent/KeeAgentSettings.h +++ b/src/sshagent/KeeAgentSettings.h @@ -29,6 +29,27 @@ class QXmlStreamReader; class KeeAgentSettings { public: + struct KeySpec + { + QString key; + bool isCertificateAuthority; + + bool operator==(const KeySpec& other) const; + + QByteArray getKeyBlob() const; + }; + + struct DestinationConstraint + { + QString fromHost; + QList fromHostKeys; + QString toUser; + QString toHost; + QList toHostKeys; + + bool operator==(const DestinationConstraint& other) const; + }; + KeeAgentSettings(); bool operator==(const KeeAgentSettings& other) const; bool operator!=(const KeeAgentSettings& other) const; @@ -40,6 +61,7 @@ public: static bool inEntryAttachments(const EntryAttachments* attachments); bool fromEntry(const Entry* entry); + bool fromEntryAttachments(const EntryAttachments* attachments); void toEntry(Entry* entry) const; bool keyConfigured() const; bool toOpenSSHKey(const Entry* entry, OpenSSHKey& key, bool decrypt); @@ -58,6 +80,8 @@ public: bool useConfirmConstraintWhenAdding() const; bool useLifetimeConstraintWhenAdding() const; int lifetimeConstraintDuration() const; + bool useDestinationConstraintsWhenAdding() const; + QList destinationConstraints() const; const QString selectedType() const; const QString attachmentName() const; @@ -71,6 +95,8 @@ public: void setUseConfirmConstraintWhenAdding(bool useConfirmConstraintWhenAdding); void setUseLifetimeConstraintWhenAdding(bool useLifetimeConstraintWhenAdding); void setLifetimeConstraintDuration(int lifetimeConstraintDuration); + void setUseDestinationConstraintsWhenAdding(bool useDestinationConstraintsWhenAdding); + void setDestinationConstraints(const QList& destinationConstraints); void setSelectedType(const QString& type); void setAttachmentName(const QString& attachmentName); @@ -87,6 +113,8 @@ private: bool m_useConfirmConstraintWhenAdding; bool m_useLifetimeConstraintWhenAdding; int m_lifetimeConstraintDuration; + bool m_useDestinationConstraintsWhenAdding; + QList m_destinationConstraints; // location QString m_selectedType; diff --git a/src/sshagent/SSHAgent.cpp b/src/sshagent/SSHAgent.cpp index a8aa695ce..ac639d80b 100644 --- a/src/sshagent/SSHAgent.cpp +++ b/src/sshagent/SSHAgent.cpp @@ -98,6 +98,16 @@ void SSHAgent::setUsePageant(bool usePageant) } #endif +bool SSHAgent::enableDestinationConstraints() const +{ + return config()->get(Config::SSHAgent_EnableDestinationConstraints).toBool(); +} + +void SSHAgent::setEnableDestinationConstraints(bool enableDestinationConstraints) +{ + config()->set(Config::SSHAgent_EnableDestinationConstraints, enableDestinationConstraints); +} + QString SSHAgent::socketPath(bool allowOverride) const { QString socketPath; @@ -305,6 +315,12 @@ bool SSHAgent::addIdentity(OpenSSHKey& key, const KeeAgentSettings& settings, co request.writeString(securityKeyProvider()); } + if (enableDestinationConstraints() && settings.useDestinationConstraintsWhenAdding()) { + request.write(SSH_AGENT_CONSTRAIN_EXTENSION); + request.writeString(QString("restrict-destination-v00@openssh.com")); + encodeDestinationConstraints(settings.destinationConstraints(), request); + } + QByteArray responseData; if (!sendMessage(requestData, responseData)) { return false; @@ -322,6 +338,10 @@ bool SSHAgent::addIdentity(OpenSSHKey& key, const KeeAgentSettings& settings, co m_error += "\n" + tr("A confirmation request is not supported by the agent (check options)."); } + if (enableDestinationConstraints() && settings.useDestinationConstraintsWhenAdding()) { + m_error += "\n" + tr("Destination constraints are invalid or not supported by the agent (check options)."); + } + if (isSecurityKey) { m_error += "\n" + tr("Security keys are not supported by the agent or the security key provider is unavailable."); @@ -336,6 +356,54 @@ bool SSHAgent::addIdentity(OpenSSHKey& key, const KeeAgentSettings& settings, co return true; } +bool SSHAgent::encodeDestinationConstraints(const QList& constraints, + BinaryStream& out) +{ + QByteArray data; + BinaryStream stream(&data); + + foreach (const auto& constraint, constraints) { + encodeDestinationConstraint(constraint, stream); + } + + out.writeString(data); + return true; +} + +bool SSHAgent::encodeDestinationConstraint(const KeeAgentSettings::DestinationConstraint& constraint, BinaryStream& out) +{ + QByteArray data; + BinaryStream stream(&data); + + encodeDestinationConstraintHost("", constraint.fromHost, constraint.fromHostKeys, stream); + encodeDestinationConstraintHost(constraint.toUser, constraint.toHost, constraint.toHostKeys, stream); + stream.writeString(QString("")); // reserved + + out.writeString(data); + return true; +} + +bool SSHAgent::encodeDestinationConstraintHost(const QString user, + const QString hostname, + const QList& keys, + BinaryStream& out) +{ + QByteArray data; + BinaryStream stream(&data); + + stream.writeString(user); + stream.writeString(hostname); + stream.writeString(QString("")); // reserved + + foreach (const auto& key, keys) { + stream.writeString(key.getKeyBlob()); + stream.write(static_cast(key.isCertificateAuthority)); + } + + out.writeString(data); + return true; +} + /** * Remove an identity from the SSH agent. * diff --git a/src/sshagent/SSHAgent.h b/src/sshagent/SSHAgent.h index d3eeb4ebc..5c02dabf3 100644 --- a/src/sshagent/SSHAgent.h +++ b/src/sshagent/SSHAgent.h @@ -21,9 +21,9 @@ #include +#include "KeeAgentSettings.h" #include "OpenSSHKey.h" -class KeeAgentSettings; class Database; class SSHAgent : public QObject @@ -48,6 +48,8 @@ public: void setUseOpenSSH(bool useOpenSSH); void setUsePageant(bool usePageant); #endif + bool enableDestinationConstraints() const; + void setEnableDestinationConstraints(bool enableDestinationConstraints); const QString errorString() const; bool isAgentRunning() const; @@ -91,6 +93,14 @@ private: const quint32 AGENT_COPYDATA_ID = 0x804e50ba; #endif + bool encodeDestinationConstraints(const QList& constraints, + BinaryStream& out); + bool encodeDestinationConstraint(const KeeAgentSettings::DestinationConstraint& constraint, BinaryStream& out); + bool encodeDestinationConstraintHost(const QString user, + const QString hostname, + const QList& keys, + BinaryStream& out); + QHash> m_addedKeys; QString m_error; }; diff --git a/tests/TestSSHAgent.cpp b/tests/TestSSHAgent.cpp index 986def7b8..bb14ae022 100644 --- a/tests/TestSSHAgent.cpp +++ b/tests/TestSSHAgent.cpp @@ -24,9 +24,56 @@ #include "sshagent/SSHAgent.h" #include +#include QTEST_GUILESS_MAIN(TestSSHAgent) +static const QList githubKeys = { + { + .key = "ssh-rsa " + "AAAAB3NzaC1yc2EAAAADAQABAAABgQCj7ndNxQowgcQnjshcLrqPEiiphnt+" + "VTTvDP6mHBL9j1aNUkY4Ue1gvwnGLVlOhGeYrnZaMgRK6+PKCUXaDbC7qtbW8gIkhL7aGCsOr/C56SJMy/" + "BCZfxd1nWzAOxSDPgVsmerOBYfNqltV9/" + "hWCqBywINIR+5dIg6JTJ72pcEpEjcYgXkE2YEFXV1JHnsKgbLWNlhScqb2UmyRkQyytRLtL+38TGxkxCflmO+" + "5Z8CSSNY7GidjMIZ7Q4zMjA2n1nGrlTDkzwDCsw+" + "wqFPGQA179cnfGWOWRVruj16z6XyvxvjJwbz0wQZ75XK5tKSb7FNyeIEs4TT4jk+S4dhPeAUC5y+" + "bDYirYgM4GC7uEnztnZyaVWQ7B381AK4Qdrwt51ZqExKbQpTUNn+EjqoTwvqNj4kqx5QUCI0ThS/" + "YkOxJCXmPUWZbhjpCg56i+2aB6CmK2JGhn57K5mj0MNdBXA4/WnwH6XoPWJzK5Nyu2zB3nAZp+S5hpQs+p1vN1/wsjk=", + .isCertificateAuthority = false, + }, + { + .key = "ecdsa-sha2-nistp256 " + "AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEmKSENjQEezOmxkZMy7opKgwFB9nkt5YRrYMjNuG5N" + "87uRgg6CLrbo5wAdT/y6v0mKV0U2w0WZ2YB/++Tpockg=", + .isCertificateAuthority = false, + }, + { + .key = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl", + .isCertificateAuthority = false, + }}; + +static const QList gitlabKeys = { + { + .key = "ssh-rsa " + "AAAAB3NzaC1yc2EAAAADAQABAAABAQCsj2bNKTBSpIYDEGk9KxsGh3mySTRgMtXL583qmBpzeQ+jqCMRgBqB98u3z++" + "J1sKlXHWfM9dyhSevkMwSbhoR8XIq/U0tCNyokEi/" + "ueaBMCvbcTHhO7FcwzY92WK4Yt0aGROY5qX2UKSeOvuP4D6TPqKF1onrSzH9bx9XUf2lEdWT/ia1NEKjunUqu1xOB/" + "StKDHMoX4/OKyIzuS0q/" + "T1zOATthvasJFoPrAjkohTyaDUz2LN5JoH839hViyEG82yB+MjcFV5MU3N1l1QL3cVUCh93xSaua1N85qivl+" + "siMkPGbO5xR/En4iEY6K2XPASUEMaieWVNTRCtJ4S8H+9", + .isCertificateAuthority = false, + }, + { + .key = "ecdsa-sha2-nistp256 " + "AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBFSMqzJeV9rUzU4kWitGjeR4PWSa29SPqJ1fVkhtj3H" + "w9xjLVXVYrU9QlYWrOLXBpQ6KWjbjTDTdDkoohFzgbEY=", + .isCertificateAuthority = false, + }, + { + .key = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAfuCHKVTjquxvt6CM6tdG4SLp1Btn/nOeHHE5UOzRdf", + .isCertificateAuthority = false, + }}; + void TestSSHAgent::initTestCase() { QVERIFY(Crypto::init()); @@ -117,6 +164,110 @@ void TestSSHAgent::testConfiguration() QCOMPARE(agent.socketPath(false), defaultSocketPath); } +void TestSSHAgent::testKeeAgentSettings() +{ + KeeAgentSettings settings; + KeeAgentSettings settings2; + + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings == settings2); + + QVERIFY(!settings.allowUseOfSshKey()); + settings.setAllowUseOfSshKey(true); + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings2.allowUseOfSshKey()); + QVERIFY(settings == settings2); + + QVERIFY(!settings.addAtDatabaseOpen()); + settings.setAddAtDatabaseOpen(true); + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings2.addAtDatabaseOpen()); + QVERIFY(settings == settings2); + + QVERIFY(!settings.removeAtDatabaseClose()); + settings.setRemoveAtDatabaseClose(true); + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings2.removeAtDatabaseClose()); + QVERIFY(settings == settings2); + + QVERIFY(!settings.useConfirmConstraintWhenAdding()); + settings.setUseConfirmConstraintWhenAdding(true); + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings2.useConfirmConstraintWhenAdding()); + QVERIFY(settings == settings2); + + QVERIFY(!settings.useLifetimeConstraintWhenAdding()); + settings.setUseLifetimeConstraintWhenAdding(true); + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings2.useLifetimeConstraintWhenAdding()); + QVERIFY(settings == settings2); + + QVERIFY(settings.lifetimeConstraintDuration() == 600); + settings.setLifetimeConstraintDuration(120); + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings2.lifetimeConstraintDuration()); + QVERIFY(settings == settings2); + + QVERIFY(settings.fileName().isEmpty()); + settings.setFileName("dummy.pkey"); + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings2.fileName() == "dummy.pkey"); + QVERIFY(settings == settings2); + + QVERIFY(settings.selectedType() == "file"); + settings.setSelectedType(QStringLiteral("attachment")); + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings2.selectedType() == "attachment"); + QVERIFY(settings == settings2); + + QVERIFY(settings.attachmentName().isEmpty()); + settings.setAttachmentName("dummy.pkey"); + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings2.attachmentName() == "dummy.pkey"); + QVERIFY(settings == settings2); + + QVERIFY(!settings.saveAttachmentToTempFile()); + settings.setSaveAttachmentToTempFile(true); + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings2.saveAttachmentToTempFile()); + QVERIFY(settings == settings2); + + QVERIFY(!settings.useDestinationConstraintsWhenAdding()); + settings.setUseDestinationConstraintsWhenAdding(true); + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings2.useDestinationConstraintsWhenAdding()); + QVERIFY(settings == settings2); + + QList destinationConstraints; + + // ssh-add -h github.com + destinationConstraints.append({ + .fromHost = "", + .fromHostKeys = {}, + .toUser = "", + .toHost = "github.com", + .toHostKeys = githubKeys, + }); + QVERIFY(settings.destinationConstraints().isEmpty()); + settings.setDestinationConstraints(destinationConstraints); + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings2.destinationConstraints() == destinationConstraints); + QVERIFY(settings == settings2); + + // ssh-add -h github.com -h "github.com>git@gitlab.com" + destinationConstraints.append({ + .fromHost = "github.com", + .fromHostKeys = githubKeys, + .toUser = "git", + .toHost = "gitlab.com", + .toHostKeys = gitlabKeys, + }); + settings.setDestinationConstraints(destinationConstraints); + QVERIFY(settings2.fromXml(settings.toXml())); + QVERIFY(settings2.destinationConstraints() == destinationConstraints); + QVERIFY(settings == settings2); +} + void TestSSHAgent::testIdentity() { SSHAgent agent; @@ -219,6 +370,58 @@ void TestSSHAgent::testConfirmConstraint() QVERIFY(agent.checkIdentity(m_key, keyInAgent) && !keyInAgent); } +void TestSSHAgent::testDestinationConstraints() +{ + // ssh-agent does not support destination constraints before OpenSSH + // version 8.9. Therefore we want to skip this test on older versions. + // Unfortunately ssh-agent does not give us any way to retrieve its version + // number neither via protocol nor on the command line. Therefore we use + // the version number of the SSH client and assume it to be the same. + QProcess ssh; + ssh.setReadChannel(QProcess::StandardError); + ssh.start("ssh", QStringList() << "-V"); + ssh.waitForFinished(); + auto ssh_version = QString::fromUtf8(ssh.readLine()); + ssh_version.remove(QRegExp("^OpenSSH_")); + if (QVersionNumber ::fromString(ssh_version) < QVersionNumber(8, 9)) { + QSKIP("Test requires ssh-agent >= 8.9"); + } + + SSHAgent agent; + agent.setEnabled(true); + agent.setAuthSockOverride(m_agentSocketFileName); + + QVERIFY(agent.isAgentRunning()); + + KeeAgentSettings settings; + bool keyInAgent; + + // ssh-add -h github.com -h "github.com>git@gitlab.com" + settings.setUseDestinationConstraintsWhenAdding(true); + settings.setDestinationConstraints({{ + .fromHost = "", + .fromHostKeys = {}, + .toUser = "", + .toHost = "github.com", + .toHostKeys = githubKeys, + }, + { + .fromHost = "github.com", + .fromHostKeys = githubKeys, + .toUser = "git", + .toHost = "gitlab.com", + .toHostKeys = gitlabKeys, + }}); + + QVERIFY(agent.addIdentity(m_key, settings, m_uuid)); + + // we can't test destination constraints itself is working but we can test the agent accepts the key + QVERIFY(agent.checkIdentity(m_key, keyInAgent) && keyInAgent); + + QVERIFY(agent.removeIdentity(m_key)); + QVERIFY(agent.checkIdentity(m_key, keyInAgent) && !keyInAgent); +} + void TestSSHAgent::testToOpenSSHKey() { KeeAgentSettings settings; diff --git a/tests/TestSSHAgent.h b/tests/TestSSHAgent.h index db06fd806..62af4f839 100644 --- a/tests/TestSSHAgent.h +++ b/tests/TestSSHAgent.h @@ -31,10 +31,12 @@ private slots: void initTestCase(); void init(); void testConfiguration(); + void testKeeAgentSettings(); void testIdentity(); void testRemoveOnClose(); void testLifetimeConstraint(); void testConfirmConstraint(); + void testDestinationConstraints(); void testToOpenSSHKey(); void testKeyGenRSA(); void testKeyGenECDSA();