Use PasswordKey for storing transformed secrets.
The transformed secrets were stored in normal QByteArrays, which are at risk of being swapped out. We now use secure PasswordKey objects instead. There are still a few areas where QByteArrays are used for storing secrets, but since they are all temporary, they are less critical. It may be worth hunting those down as well, though.
This commit is contained in:
@@ -316,7 +316,11 @@ bool Database::writeDatabase(QIODevice* device, QString* error)
|
||||
return false;
|
||||
}
|
||||
|
||||
QByteArray oldTransformedKey = m_data.transformedMasterKey;
|
||||
PasswordKey oldTransformedKey;
|
||||
if (m_data.hasKey) {
|
||||
oldTransformedKey.setHash(m_data.transformedMasterKey->rawKey());
|
||||
}
|
||||
|
||||
KeePass2Writer writer;
|
||||
setEmitModified(false);
|
||||
writer.writeDatabase(device, this);
|
||||
@@ -329,9 +333,10 @@ bool Database::writeDatabase(QIODevice* device, QString* error)
|
||||
return false;
|
||||
}
|
||||
|
||||
Q_ASSERT(!m_data.transformedMasterKey.isEmpty());
|
||||
Q_ASSERT(m_data.transformedMasterKey != oldTransformedKey);
|
||||
if (m_data.transformedMasterKey.isEmpty() || m_data.transformedMasterKey == oldTransformedKey) {
|
||||
QByteArray newKey = m_data.transformedMasterKey->rawKey();
|
||||
Q_ASSERT(!newKey.isEmpty());
|
||||
Q_ASSERT(newKey != oldTransformedKey.rawKey());
|
||||
if (newKey.isEmpty() || newKey == oldTransformedKey.rawKey()) {
|
||||
if (error) {
|
||||
*error = tr("Key not transformed. This is a bug, please report it to the developers!");
|
||||
}
|
||||
@@ -382,6 +387,7 @@ bool Database::import(const QString& xmlExportPath, QString* error)
|
||||
*
|
||||
* A previously reparented root group will not be freed.
|
||||
*/
|
||||
|
||||
void Database::releaseData()
|
||||
{
|
||||
s_uuidMap.remove(m_uuid);
|
||||
@@ -391,7 +397,7 @@ void Database::releaseData()
|
||||
emit databaseDiscarded();
|
||||
}
|
||||
|
||||
m_data = DatabaseData();
|
||||
m_data.clear();
|
||||
|
||||
if (m_rootGroup && m_rootGroup->parent() == this) {
|
||||
delete m_rootGroup;
|
||||
@@ -630,19 +636,24 @@ Database::CompressionAlgorithm Database::compressionAlgorithm() const
|
||||
|
||||
QByteArray Database::transformedMasterKey() const
|
||||
{
|
||||
return m_data.transformedMasterKey;
|
||||
return m_data.transformedMasterKey->rawKey();
|
||||
}
|
||||
|
||||
QByteArray Database::challengeResponseKey() const
|
||||
{
|
||||
return m_data.challengeResponseKey;
|
||||
return m_data.challengeResponseKey->rawKey();
|
||||
}
|
||||
|
||||
bool Database::challengeMasterSeed(const QByteArray& masterSeed)
|
||||
{
|
||||
if (m_data.key) {
|
||||
m_data.masterSeed = masterSeed;
|
||||
return m_data.key->challenge(masterSeed, m_data.challengeResponseKey);
|
||||
m_data.masterSeed->setHash(masterSeed);
|
||||
QByteArray response;
|
||||
bool ok = m_data.key->challenge(masterSeed, response);
|
||||
if (ok && !response.isEmpty()) {
|
||||
m_data.challengeResponseKey->setHash(response);
|
||||
}
|
||||
return ok;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
@@ -679,7 +690,7 @@ bool Database::setKey(const QSharedPointer<const CompositeKey>& key,
|
||||
|
||||
if (!key) {
|
||||
m_data.key.reset();
|
||||
m_data.transformedMasterKey = {};
|
||||
m_data.transformedMasterKey.reset(new PasswordKey());
|
||||
m_data.hasKey = false;
|
||||
return true;
|
||||
}
|
||||
@@ -689,22 +700,29 @@ bool Database::setKey(const QSharedPointer<const CompositeKey>& key,
|
||||
Q_ASSERT(!m_data.kdf->seed().isEmpty());
|
||||
}
|
||||
|
||||
QByteArray oldTransformedMasterKey = m_data.transformedMasterKey;
|
||||
PasswordKey oldTransformedMasterKey;
|
||||
if (m_data.hasKey) {
|
||||
oldTransformedMasterKey.setHash(m_data.transformedMasterKey->rawKey());
|
||||
}
|
||||
|
||||
QByteArray transformedMasterKey;
|
||||
|
||||
if (!transformKey) {
|
||||
transformedMasterKey = oldTransformedMasterKey;
|
||||
transformedMasterKey = QByteArray(oldTransformedMasterKey.rawKey());
|
||||
} else if (!key->transform(*m_data.kdf, transformedMasterKey)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
m_data.key = key;
|
||||
m_data.transformedMasterKey = transformedMasterKey;
|
||||
if (!transformedMasterKey.isEmpty()) {
|
||||
m_data.transformedMasterKey->setHash(transformedMasterKey);
|
||||
}
|
||||
m_data.hasKey = true;
|
||||
if (updateChangedTime) {
|
||||
m_metadata->setMasterKeyChanged(Clock::currentDateTimeUtc());
|
||||
}
|
||||
|
||||
if (oldTransformedMasterKey != m_data.transformedMasterKey) {
|
||||
if (oldTransformedMasterKey.rawKey() != m_data.transformedMasterKey->rawKey()) {
|
||||
markAsModified();
|
||||
}
|
||||
|
||||
@@ -720,15 +738,15 @@ bool Database::verifyKey(const QSharedPointer<CompositeKey>& key) const
|
||||
{
|
||||
Q_ASSERT(hasKey());
|
||||
|
||||
if (!m_data.challengeResponseKey.isEmpty()) {
|
||||
if (!m_data.challengeResponseKey->rawKey().isEmpty()) {
|
||||
QByteArray result;
|
||||
|
||||
if (!key->challenge(m_data.masterSeed, result)) {
|
||||
if (!key->challenge(m_data.masterSeed->rawKey(), result)) {
|
||||
// challenge failed, (YubiKey?) removed?
|
||||
return false;
|
||||
}
|
||||
|
||||
if (m_data.challengeResponseKey != result) {
|
||||
if (m_data.challengeResponseKey->rawKey() != result) {
|
||||
// wrong response from challenged device(s)
|
||||
return false;
|
||||
}
|
||||
@@ -893,7 +911,7 @@ bool Database::changeKdf(const QSharedPointer<Kdf>& kdf)
|
||||
}
|
||||
|
||||
setKdf(kdf);
|
||||
m_data.transformedMasterKey = transformedMasterKey;
|
||||
m_data.transformedMasterKey->setHash(transformedMasterKey);
|
||||
markAsModified();
|
||||
|
||||
return true;
|
||||
|
||||
Reference in New Issue
Block a user