security: Fix signing by identity (ECDSA)
When signing by identity, if no certificate is available for the default
key and its type does not corresponds to the `DEFAULT_KEY_PARAMS` a new
pair of `DEFAULT_KEY_PARAMS` keys is created, set as default and used for
signing. Solved by checking the type of key of the default key pair for
the identity.
Change-Id: I75c117cea17cbbfda410da9a83dd16b92d345d21
Refs: #3438
diff --git a/src/security/key-chain.cpp b/src/security/key-chain.cpp
index 16bc879..c1c605a 100644
--- a/src/security/key-chain.cpp
+++ b/src/security/key-chain.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2013-2015 Regents of the University of California.
+ * Copyright (c) 2013-2016 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -22,6 +22,7 @@
*/
#include "key-chain.hpp"
+#include "signing-helpers.hpp"
#include "../util/random.hpp"
#include "../util/config-file.hpp"
@@ -259,7 +260,7 @@
BOOST_THROW_EXCEPTION(MismatchError("TPM locator supplied does not match TPM locator in PIB: "
+ m_pib->getTpmLocator() + " != " + canonicalTpmLocator));
}
- catch (SecPublicInfo::Error&) {
+ catch (const SecPublicInfo::Error&) {
// TPM locator is not set in PIB yet.
}
@@ -276,35 +277,30 @@
m_pib->addIdentity(identityName);
Name keyName;
- try
- {
- keyName = m_pib->getDefaultKeyNameForIdentity(identityName);
+ try {
+ keyName = m_pib->getDefaultKeyNameForIdentity(identityName);
- shared_ptr<PublicKey> key = m_pib->getPublicKey(keyName);
+ shared_ptr<PublicKey> key = m_pib->getPublicKey(keyName);
- if (key->getKeyType() != params.getKeyType())
- {
- keyName = generateKeyPair(identityName, true, params);
- m_pib->setDefaultKeyNameForIdentity(keyName);
- }
- }
- catch (SecPublicInfo::Error& e)
- {
+ if (key->getKeyType() != params.getKeyType()) {
keyName = generateKeyPair(identityName, true, params);
m_pib->setDefaultKeyNameForIdentity(keyName);
}
+ }
+ catch (const SecPublicInfo::Error& e) {
+ keyName = generateKeyPair(identityName, true, params);
+ m_pib->setDefaultKeyNameForIdentity(keyName);
+ }
Name certName;
- try
- {
- certName = m_pib->getDefaultCertificateNameForKey(keyName);
- }
- catch (SecPublicInfo::Error& e)
- {
- shared_ptr<IdentityCertificate> selfCert = selfSign(keyName);
- m_pib->addCertificateAsIdentityDefault(*selfCert);
- certName = selfCert->getName();
- }
+ try {
+ certName = m_pib->getDefaultCertificateNameForKey(keyName);
+ }
+ catch (const SecPublicInfo::Error& e) {
+ shared_ptr<IdentityCertificate> selfCert = selfSign(keyName);
+ m_pib->addCertificateAsIdentityDefault(*selfCert);
+ certName = selfCert->getName();
+ }
return certName;
}
@@ -357,14 +353,12 @@
const Name& certPrefix)
{
shared_ptr<PublicKey> publicKey;
- try
- {
- publicKey = m_pib->getPublicKey(keyName);
- }
- catch (SecPublicInfo::Error& e)
- {
- return shared_ptr<IdentityCertificate>();
- }
+ try {
+ publicKey = m_pib->getPublicKey(keyName);
+ }
+ catch (const SecPublicInfo::Error& e) {
+ return nullptr;
+ }
return prepareUnsignedIdentityCertificate(keyName, *publicKey, signingIdentity,
notBefore, notAfter,
@@ -381,64 +375,59 @@
const Name& certPrefix)
{
if (keyName.size() < 1)
- return shared_ptr<IdentityCertificate>();
+ return nullptr;
std::string keyIdPrefix = keyName.get(-1).toUri().substr(0, 4);
if (keyIdPrefix != "ksk-" && keyIdPrefix != "dsk-")
- return shared_ptr<IdentityCertificate>();
+ return nullptr;
- shared_ptr<IdentityCertificate> certificate = make_shared<IdentityCertificate>();
Name certName;
- if (certPrefix == KeyChain::DEFAULT_PREFIX)
- {
- // No certificate prefix hint, infer the prefix
- if (signingIdentity.isPrefixOf(keyName))
- certName.append(signingIdentity)
- .append("KEY")
- .append(keyName.getSubName(signingIdentity.size()))
- .append("ID-CERT")
- .appendVersion();
- else
- certName.append(keyName.getPrefix(-1))
- .append("KEY")
- .append(keyName.get(-1))
- .append("ID-CERT")
- .appendVersion();
- }
- else
- {
- // cert prefix hint is supplied, determine the cert name.
- if (certPrefix.isPrefixOf(keyName) && certPrefix != keyName)
- certName.append(certPrefix)
- .append("KEY")
- .append(keyName.getSubName(certPrefix.size()))
- .append("ID-CERT")
- .appendVersion();
- else
- return shared_ptr<IdentityCertificate>();
- }
+ if (certPrefix == KeyChain::DEFAULT_PREFIX) {
+ // No certificate prefix hint, infer the prefix
+ if (signingIdentity.isPrefixOf(keyName))
+ certName.append(signingIdentity)
+ .append("KEY")
+ .append(keyName.getSubName(signingIdentity.size()))
+ .append("ID-CERT")
+ .appendVersion();
+ else
+ certName.append(keyName.getPrefix(-1))
+ .append("KEY")
+ .append(keyName.get(-1))
+ .append("ID-CERT")
+ .appendVersion();
+ }
+ else {
+ // cert prefix hint is supplied, determine the cert name.
+ if (certPrefix.isPrefixOf(keyName) && certPrefix != keyName)
+ certName.append(certPrefix)
+ .append("KEY")
+ .append(keyName.getSubName(certPrefix.size()))
+ .append("ID-CERT")
+ .appendVersion();
+ else
+ return nullptr;
+ }
-
+ auto certificate = make_shared<IdentityCertificate>();
certificate->setName(certName);
certificate->setNotBefore(notBefore);
certificate->setNotAfter(notAfter);
certificate->setPublicKeyInfo(publicKey);
- if (subjectDescription.empty())
- {
- CertificateSubjectDescription subjectName(oid::ATTRIBUTE_NAME, keyName.getPrefix(-1).toUri());
- certificate->addSubjectDescription(subjectName);
- }
- else
- {
- std::vector<CertificateSubjectDescription>::const_iterator sdIt =
- subjectDescription.begin();
- std::vector<CertificateSubjectDescription>::const_iterator sdEnd =
- subjectDescription.end();
- for(; sdIt != sdEnd; sdIt++)
- certificate->addSubjectDescription(*sdIt);
- }
+ if (subjectDescription.empty()) {
+ CertificateSubjectDescription subjectName(oid::ATTRIBUTE_NAME, keyName.getPrefix(-1).toUri());
+ certificate->addSubjectDescription(subjectName);
+ }
+ else {
+ std::vector<CertificateSubjectDescription>::const_iterator sdIt =
+ subjectDescription.begin();
+ std::vector<CertificateSubjectDescription>::const_iterator sdEnd =
+ subjectDescription.end();
+ for(; sdIt != sdEnd; sdIt++)
+ certificate->addSubjectDescription(*sdIt);
+ }
certificate->encode();
@@ -453,35 +442,32 @@
shared_ptr<IdentityCertificate> signingCert;
switch (params.getSignerType()) {
- case SigningInfo::SIGNER_TYPE_NULL:
- {
+ case SigningInfo::SIGNER_TYPE_NULL: {
if (m_pib->getDefaultCertificate() == nullptr)
setDefaultCertificateInternal();
signingCert = m_pib->getDefaultCertificate();
break;
}
- case SigningInfo::SIGNER_TYPE_ID:
- {
+ case SigningInfo::SIGNER_TYPE_ID: {
Name signingCertName;
try {
signingCertName = m_pib->getDefaultCertificateNameForIdentity(params.getSignerName());
}
- catch (SecPublicInfo::Error&) {
- signingCertName = createIdentity(params.getSignerName());
+ catch (const SecPublicInfo::Error&) {
+ signingCertName = createIdentity(params.getSignerName(), getDefaultKeyParamsForIdentity(params.getSignerName()));
}
signingCert = m_pib->getCertificate(signingCertName);
break;
}
- case SigningInfo::SIGNER_TYPE_KEY:
- {
+ case SigningInfo::SIGNER_TYPE_KEY: {
Name signingCertName;
try {
signingCertName = m_pib->getDefaultCertificateNameForKey(params.getSignerName());
}
- catch (SecPublicInfo::Error&) {
+ catch (const SecPublicInfo::Error&) {
BOOST_THROW_EXCEPTION(Error("signing certificate does not exist"));
}
@@ -489,21 +475,19 @@
break;
}
- case SigningInfo::SIGNER_TYPE_CERT:
- {
+ case SigningInfo::SIGNER_TYPE_CERT: {
signingCert = m_pib->getCertificate(params.getSignerName());
if (signingCert == nullptr)
BOOST_THROW_EXCEPTION(Error("signing certificate does not exist"));
break;
}
- case SigningInfo::SIGNER_TYPE_SHA256:
- {
+ case SigningInfo::SIGNER_TYPE_SHA256: {
sigInfo.setSignatureType(tlv::DigestSha256);
return std::make_tuple(DIGEST_SHA256_IDENTITY, sigInfo);
}
- default:
- BOOST_THROW_EXCEPTION(Error("Unrecognized signer type"));
+ default:
+ BOOST_THROW_EXCEPTION(Error("Unrecognized signer type"));
}
sigInfo.setSignatureType(getSignatureType(signingCert->getPublicKeyInfo().getKeyType(),
@@ -539,8 +523,9 @@
{
shared_ptr<IdentityCertificate> certificate = m_pib->getCertificate(certificateName);
- if (certificate == nullptr)
+ if (certificate == nullptr) {
BOOST_THROW_EXCEPTION(SecPublicInfo::Error("certificate does not exist"));
+ }
Signature sig;
@@ -559,8 +544,8 @@
try {
pubKey = m_pib->getPublicKey(keyName); // may throw an exception.
}
- catch (SecPublicInfo::Error&) {
- return shared_ptr<IdentityCertificate>();
+ catch (const SecPublicInfo::Error&) {
+ return nullptr;
}
auto certificate = make_shared<IdentityCertificate>();
@@ -606,30 +591,24 @@
Name keyName = m_pib->getDefaultKeyNameForIdentity(identity);
ConstBufferPtr pkcs5;
- try
- {
- pkcs5 = m_tpm->exportPrivateKeyPkcs5FromTpm(keyName, passwordStr);
- }
- catch (SecTpm::Error& e)
- {
- BOOST_THROW_EXCEPTION(SecPublicInfo::Error("Fail to export PKCS5 of private key"));
- }
+ try {
+ pkcs5 = m_tpm->exportPrivateKeyPkcs5FromTpm(keyName, passwordStr);
+ }
+ catch (const SecTpm::Error& e) {
+ BOOST_THROW_EXCEPTION(SecPublicInfo::Error("Fail to export PKCS5 of private key"));
+ }
shared_ptr<IdentityCertificate> cert;
- try
- {
- cert = m_pib->getCertificate(m_pib->getDefaultCertificateNameForKey(keyName));
- }
- catch (SecPublicInfo::Error& e)
- {
- cert = selfSign(keyName);
- m_pib->addCertificateAsIdentityDefault(*cert);
- }
+ try {
+ cert = m_pib->getCertificate(m_pib->getDefaultCertificateNameForKey(keyName));
+ }
+ catch (const SecPublicInfo::Error& e) {
+ cert = selfSign(keyName);
+ m_pib->addCertificateAsIdentityDefault(*cert);
+ }
// make_shared on OSX 10.9 has some strange problem here
- shared_ptr<SecuredBag> secureBag(new SecuredBag(*cert, pkcs5));
-
- return secureBag;
+ return shared_ptr<SecuredBag>(new SecuredBag(*cert, pkcs5));
}
void
@@ -657,28 +636,53 @@
m_pib->addCertificateAsIdentityDefault(securedBag.getCertificate());
}
+const KeyParams&
+KeyChain::getDefaultKeyParamsForIdentity(const Name &identityName) const
+{
+ KeyType keyType = KEY_TYPE_NULL;
+ try {
+ keyType = m_pib->getPublicKeyType(m_pib->getDefaultKeyNameForIdentity(identityName));
+ }
+ catch (const SecPublicInfo::Error& e) { // @TODO Switch to Pib::Error
+ return DEFAULT_KEY_PARAMS;
+ }
+
+ switch (keyType) {
+ case KEY_TYPE_RSA: {
+ static RsaKeyParams defaultRsaParams;
+ return defaultRsaParams;
+ }
+ case KEY_TYPE_ECDSA: {
+ static EcdsaKeyParams defaultEcdsaParams;
+ return defaultEcdsaParams;
+ }
+ case KEY_TYPE_NULL: {
+ return DEFAULT_KEY_PARAMS;
+ }
+ default:
+ BOOST_THROW_EXCEPTION(Error("Unsupported key type"));
+ }
+}
+
void
KeyChain::setDefaultCertificateInternal()
{
m_pib->refreshDefaultCertificate();
- if (!static_cast<bool>(m_pib->getDefaultCertificate()))
- {
- Name defaultIdentity;
- try
- {
- defaultIdentity = m_pib->getDefaultIdentity();
- }
- catch (SecPublicInfo::Error& e)
- {
- uint32_t random = random::generateWord32();
- defaultIdentity.append("tmp-identity")
- .append(reinterpret_cast<uint8_t*>(&random), 4);
- }
- createIdentity(defaultIdentity);
- m_pib->setDefaultIdentity(defaultIdentity);
- m_pib->refreshDefaultCertificate();
+ if (m_pib->getDefaultCertificate() == nullptr) {
+ Name defaultIdentity;
+ try {
+ defaultIdentity = m_pib->getDefaultIdentity();
}
+ catch (const SecPublicInfo::Error& e) {
+ uint32_t random = random::generateWord32();
+ defaultIdentity.append("tmp-identity")
+ .append(reinterpret_cast<uint8_t*>(&random), 4);
+ }
+ createIdentity(defaultIdentity);
+ m_pib->setDefaultIdentity(defaultIdentity);
+ m_pib->refreshDefaultCertificate();
+ }
}
Name
@@ -713,10 +717,9 @@
const Name& keyName, DigestAlgorithm digestAlgorithm)
{
time::milliseconds timestamp = time::toUnixTimestamp(time::system_clock::now());
- if (timestamp <= m_lastTimestamp)
- {
- timestamp = m_lastTimestamp + time::milliseconds(1);
- }
+ if (timestamp <= m_lastTimestamp) {
+ timestamp = m_lastTimestamp + time::milliseconds(1);
+ }
Name signedName = interest.getName();
signedName
@@ -747,40 +750,15 @@
Signature
KeyChain::signByIdentity(const uint8_t* buffer, size_t bufferLength, const Name& identityName)
{
- Name signingCertificateName;
- try
- {
- signingCertificateName = m_pib->getDefaultCertificateNameForIdentity(identityName);
- }
- catch (SecPublicInfo::Error& e)
- {
- signingCertificateName = createIdentity(identityName);
- // Ideally, no exception will be thrown out, unless something goes wrong in the TPM, which
- // is a fatal error.
- }
-
- // We either get or create the signing certificate, sign data! (no exception unless fatal error
- // in TPM)
Signature sig;
-
- // For temporary usage, we support SHA256 only, but will support more.
- sig.setValue(sign(buffer, bufferLength, SigningInfo(SigningInfo::SIGNER_TYPE_CERT,
- signingCertificateName)));
-
+ sig.setValue(sign(buffer, bufferLength, signingByIdentity(identityName)));
return sig;
}
void
KeyChain::signWithSha256(Data& data)
{
- DigestSha256 sig;
- data.setSignature(sig);
-
- Block sigValue(tlv::SignatureValue,
- crypto::sha256(data.wireEncode().value(),
- data.wireEncode().value_size() -
- data.getSignature().getValue().size()));
- data.setSignatureValue(sigValue);
+ return sign(data, signingWithSha256());
}
void
diff --git a/src/security/key-chain.hpp b/src/security/key-chain.hpp
index c5082d3..28c026e 100644
--- a/src/security/key-chain.hpp
+++ b/src/security/key-chain.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2013-2015 Regents of the University of California.
+ * Copyright (c) 2013-2016 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -528,6 +528,16 @@
return m_pib->getDefaultKeyNameForIdentity(identityName);
}
+ /**
+ * @brief Get default key parameters for the specified identity
+ *
+ * If identity has a previously generated key, the returned parameters
+ * will include the same type of the key. If there are no existing
+ * keys, DEFAULT_KEY_PARAMS is used.
+ */
+ const KeyParams&
+ getDefaultKeyParamsForIdentity(const Name& identityName) const;
+
Name
getDefaultCertificateNameForKey(const Name& keyName) const
{
diff --git a/tests/unit-tests/security/key-chain.t.cpp b/tests/unit-tests/security/key-chain.t.cpp
index a6c0cbe..5eae0c4 100644
--- a/tests/unit-tests/security/key-chain.t.cpp
+++ b/tests/unit-tests/security/key-chain.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2013-2015 Regents of the University of California.
+ * Copyright (c) 2013-2016 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -21,11 +21,13 @@
#include "security/key-chain.hpp"
#include "security/validator.hpp"
-#include "../util/test-home-environment-fixture.hpp"
-#include <boost/filesystem.hpp>
+#include "security/signing-helpers.hpp"
#include "boost-test.hpp"
#include "dummy-keychain.hpp"
+#include "../util/test-home-environment-fixture.hpp"
+
+#include <boost/filesystem.hpp>
namespace ndn {
namespace security {
@@ -417,6 +419,27 @@
interest5.getName()[-1].blockFromValue()))));
}
+BOOST_AUTO_TEST_CASE(EcdsaSigningByIdentityNoCert)
+{
+ KeyChain keyChain;
+ Data data("/test/data");
+
+ Name nonExistingIdentity = Name("/non-existing/identity").appendVersion();
+
+ BOOST_CHECK_NO_THROW(keyChain.sign(data, signingByIdentity(nonExistingIdentity)));
+ BOOST_CHECK_EQUAL(data.getSignature().getType(),
+ KeyChain::getSignatureType(KeyChain::DEFAULT_KEY_PARAMS.getKeyType(),
+ DIGEST_ALGORITHM_SHA256));
+ BOOST_CHECK(nonExistingIdentity.isPrefixOf(data.getSignature().getKeyLocator().getName()));
+
+ Name ecdsaIdentity = Name("/ndn/test/ecdsa").appendVersion();
+ Name ecdsaKeyName = keyChain.generateEcdsaKeyPairAsDefault(ecdsaIdentity, false, 256);
+ BOOST_CHECK_NO_THROW(keyChain.sign(data, signingByIdentity(ecdsaIdentity)));
+ BOOST_CHECK_EQUAL(data.getSignature().getType(),
+ KeyChain::getSignatureType(EcdsaKeyParams().getKeyType(), DIGEST_ALGORITHM_SHA256));
+ BOOST_CHECK(ecdsaIdentity.isPrefixOf(data.getSignature().getKeyLocator().getName()));
+}
+
BOOST_AUTO_TEST_SUITE_END()
} // namespace tests