https://redmine.named-data.net/https://redmine.named-data.net/favicon.ico?14759811232018-01-05T13:47:43ZNDN project issue tracking systemndn-cxx - Feature #4294: ndnsec: Extend key-gen command line to allow selection of different KeyId typeshttps://redmine.named-data.net/issues/4294?journal_id=216912018-01-05T13:47:43ZZhiyi Zhangzhangzhiyi1919@gmail.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Code review</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>80</i></li></ul> ndn-cxx - Feature #4294: ndnsec: Extend key-gen command line to allow selection of different KeyId typeshttps://redmine.named-data.net/issues/4294?journal_id=217002018-01-07T11:30:43ZJunxiao Shi
<ul><li><strong>Tracker</strong> changed from <i>Task</i> to <i>Feature</i></li></ul><p>I notice that <a href="https://gerrit.named-data.net/4427">https://gerrit.named-data.net/4427</a> patchset3 interprets user-specified KeyId with <code>name::Component::Component(const char*)</code> constructor that takes the string as-is. I think it is better to interpret the KeyId string as URI component with <code>name::Component::fromEscapedString</code>.</p>
<p>As specified in certificate-format spec, KeyId is an opaque name component. It is not restricted to printable characters.<br>
Therefore, interpreting the command line argument as URI makes it easier to specify a non-printable KeyId.</p>
ndn-cxx - Feature #4294: ndnsec: Extend key-gen command line to allow selection of different KeyId typeshttps://redmine.named-data.net/issues/4294?journal_id=217072018-01-08T12:32:06ZJunxiao Shi
<ul></ul><p>Change 4427,7 test log:</p>
<pre><code>ubuntu@m0213:~/ndn-cxx$ ndnsec key-gen /A --key_id ...
ndnsec: ../src/security/key-params.cpp:43: ndn::KeyParams::KeyParams(ndn::KeyType, const ndn::name::Component&): Assertion `!keyId.empty()' failed.
Aborted (core dumped)
</code></pre>
<p>It's okay to reject zero-octet name component as KeyId, but it should be a graceful failure instead of hitting an assertion.</p>
<pre><code>ubuntu@m0213:~/ndn-cxx$ ndnsec key-gen /A --key_id DD
Bv0ClwcdCAFBCANLRVkIAkRECARzZWxmCAn9AAABYNdvfRUUCRgBAhkEADbugBX9
ASYwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDMgsudIVrNx0+TUmF8
(omitted)
ubuntu@m0213:~/ndn-cxx$ ndnsec list -c
* /A
+->* /A/KEY/DD
+->* /A/KEY/DD/self/%FD%00%00%01%60%D7o%7D%15
(omitted)
ubuntu@m0213:~/ndn-cxx$ ndnsec key-gen /A --key_id DD
Error: Key `/A/KEY/DD` already exists
</code></pre>
<p>OK.</p>
<pre><code>ubuntu@m0213:~/ndn-cxx$ ndnsec key-gen /A --key_id %00
Bv0ClQccCAFBCANLRVkIAQAIBHNlbGYICf0AAAFg13QmAxQJGAECGQQANu6AFf0B
JjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANMFThhG/Ty8YCjOhBkj
(omitted)
</code></pre>
<p>OK.</p>
<pre><code>ubuntu@m0213:~/ndn-cxx$ ndnsec key-gen /A --key_id sha256digest=XX
Error: Cannot convert to ImplicitSha256DigestComponent(expected sha256 in hex encoding)
</code></pre>
<p>OK.</p>
<pre><code>ubuntu@m0213:~/ndn-cxx$ ndnsec key-gen /A --key_id sha256digest=0000000000000000000000000000000000000000000000000000000000000000
Bv0C0wc7CAFBCANLRVkBIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
CARzZWxmCAn9AAABYNdxa80UCRgBAhkEADbugBX9ASYwggEiMA0GCSqGSIb3DQEB
(omitted)
</code></pre>
<p>This one should be rejected. The resulting certificate name would have a <code>ImplicitSha256DigestComponent</code> in the middle, which is an invalid Data packet.</p>
<pre><code>ubuntu@m0213:~/ndn-cxx$ ndnsec key-gen /A --key_id $(python -c "print '%CC'*12000") | head -2
Bv1gXwf9Lv0IAUEIA0tFWQj9LuDMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzM
zMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzMzM
</code></pre>
<p>This is acceptable. Although the resulting Data exceeds the "practical" 8800-octet limit, it is not a protocol violation.<br>
<code>ndnsec cert-dump</code> works properly with this certificate.<br>
The difficulty in certificate retrieval is not <code>ndnsec</code>'s concern.</p>
ndn-cxx - Feature #4294: ndnsec: Extend key-gen command line to allow selection of different KeyId typeshttps://redmine.named-data.net/issues/4294?journal_id=217142018-01-08T13:49:41ZZhiyi Zhangzhangzhiyi1919@gmail.com
<ul></ul><p>Junxiao Shi wrote:</p>
<blockquote>
<pre><code>ubuntu@m0213:~/ndn-cxx$ ndnsec key-gen /A --key_id sha256digest=0000000000000000000000000000000000000000000000000000000000000000
Bv0C0wc7CAFBCANLRVkBIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
CARzZWxmCAn9AAABYNdxa80UCRgBAhkEADbugBX9ASYwggEiMA0GCSqGSIb3DQEB
(omitted)
</code></pre>
<p>This one should be rejected. The resulting certificate name would have a <code>ImplicitSha256DigestComponent</code> in the middle, which is an invalid Data packet.</p>
</blockquote>
<p>I thought the component::fromEscapedString() function should have already erased sha256digest=. Why does it still exist in the name? I paste the code here:</p>
<pre><code>Component
Component::fromEscapedString(const char* escapedString, size_t beginOffset, size_t endOffset)
{
std::string trimmedString(escapedString + beginOffset, escapedString + endOffset);
boost::algorithm::trim(trimmedString);
if (trimmedString.compare(0, getSha256DigestUriPrefix().size(),
getSha256DigestUriPrefix()) == 0) {
if (trimmedString.size() != getSha256DigestUriPrefix().size() + util::Sha256::DIGEST_SIZE * 2)
BOOST_THROW_EXCEPTION(Error("Cannot convert to ImplicitSha256DigestComponent"
"(expected sha256 in hex encoding)"));
try {
trimmedString.erase(0, getSha256DigestUriPrefix().size());
return fromImplicitSha256Digest(fromHex(trimmedString));
}
catch (const StringHelperError&) {
BOOST_THROW_EXCEPTION(Error("Cannot convert to a ImplicitSha256DigestComponent (invalid hex "
"encoding)"));
}
...
</code></pre> ndn-cxx - Feature #4294: ndnsec: Extend key-gen command line to allow selection of different KeyId typeshttps://redmine.named-data.net/issues/4294?journal_id=217152018-01-08T14:04:14ZAlex Afanasyev
<ul></ul><p>I think you're misunderstanding. What Junxiao is saying this component type should not be allowed as key id. fromEscapedString() is doing what it is suppose to do. Just add check that after decoding the component type (.type()) is not <code>ImplicitSha256DigestComponent</code></p>
ndn-cxx - Feature #4294: ndnsec: Extend key-gen command line to allow selection of different KeyId typeshttps://redmine.named-data.net/issues/4294?journal_id=217162018-01-08T14:08:21ZJunxiao Shi
<ul></ul><p>I’d suggest allowing only <em>GenericNameComponent</em> and rejecting all other types. With “typed component” coming, it’s best to whitelist each type individually.</p>
ndn-cxx - Feature #4294: ndnsec: Extend key-gen command line to allow selection of different KeyId typeshttps://redmine.named-data.net/issues/4294?journal_id=217482018-01-09T00:58:03ZJunxiao Shi
<ul></ul><blockquote>
<p>Exactly the opposite. He current way is correct and whitelisting would lead to problems</p>
</blockquote>
<p>Let me elaborate why I think KeyId should whitelist <em>GenericNameComponent</em>.</p>
<ol>
<li>When typed component is implemented, I anticipate there will be a “key id” component type, along with many other component types that cannot be “key id”.</li>
<li>“key id” is the recommended component type for key id. GenericNameComponent is still accepted for backwards compatibility.</li>
<li>Blacklisting ImplicitSha256DigestComponent would cause <code>ndnsec key-gen</code> to incorrectly accept other component types.</li>
<li>Whitelisting GenericNameComponent for now would then require extending the whitelist to include “key id” component, which should be part of the typed component implementation commit.</li>
</ol>
ndn-cxx - Feature #4294: ndnsec: Extend key-gen command line to allow selection of different KeyId typeshttps://redmine.named-data.net/issues/4294?journal_id=217642018-01-09T10:05:34ZAlex Afanasyev
<ul><li><strong>Status</strong> changed from <i>Code review</i> to <i>Closed</i></li><li><strong>% Done</strong> changed from <i>80</i> to <i>100</i></li></ul>