Project

General

Profile

Actions

Bug #4359

closed

KeyChain::importSafeBag does not properly check for error in Tpm::importPrivateKey

Added by Anonymous over 6 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Normal
Category:
Security
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:

Description

Tpm::importPrivateKey does not thrown an exception. Instead it catches BackEnd::Error and returns false for an error.
https://github.com/named-data/ndn-cxx/blob/794f687e16e9b836d3f3703d5ee0fc84e7895056/src/security/tpm/tpm.cpp#L145

try {
  m_backEnd->importKey(keyName, pkcs8, pkcs8Len, pw, pwLen);
}
catch (const BackEnd::Error&) {
  return false;
}
return true;

However, when KeyChain::importSafeBag calls importPrivateKey, it checks for an exception instead of checking for a return value of false:
https://github.com/named-data/ndn-cxx/blob/5d0b0106dfc7675f9048976d4dd4ea00e46e6c39/src/security/v2/key-chain.cpp#L387

try {
  m_tpm->importPrivateKey(keyName,
                          safeBag.getEncryptedKeyBag().data(), safeBag.getEncryptedKeyBag().size(),
                          pw, pwLen);
}
catch (const std::runtime_error&) {
  BOOST_THROW_EXCEPTION(Error("Fail to import private key `" + keyName.toUri() + "`"));
}

Therefore, if the TPM back end throws BackEnd::Error, it is ignored by KeyChain::importSafeBag. It should check for a return value of false when calling Tpm::importPrivateKey (or Tpm::importPrivateKey should throw the exception instead of returning false).


Related issues 1 (0 open1 closed)

Related to ndn-cxx - Bug #4314: ndnsec import: segfault upon wrong passphraseClosed

Actions
Actions #1

Updated by Junxiao Shi over 6 years ago

  • Related to Bug #4314: ndnsec import: segfault upon wrong passphrase added
Actions #2

Updated by Davide Pesavento over 6 years ago

  • Category set to Security
  • Status changed from New to Code review
  • Assignee set to Davide Pesavento
  • Target version set to v0.7
  • Start date deleted (10/26/2017)
  • % Done changed from 0 to 100
Actions #3

Updated by Davide Pesavento over 6 years ago

  • Status changed from Code review to Closed
Actions

Also available in: Atom PDF