Task #4089
closedAdapt to ndn-cxx v2::KeyChain and Validator
Added by Zhiyi Zhang over 7 years ago. Updated about 7 years ago.
100%
Description
Adapt NFD and ndn-tools codebase to use ndn-cxx v2::KeyChain
and v2::Validator
.
Updated by Zhiyi Zhang over 7 years ago
- Related to Task #3098: Merge KeyChain branch added
Updated by Junxiao Shi over 7 years ago
- Tracker changed from Task to Bug
- Subject changed from Urgent bugs caused by ndn-cxx change 3547 to Build broken by ndn-cxx v2::KeyChain
- Description updated (diff)
- Category set to Build
- Status changed from New to In Progress
- Assignee set to Junxiao Shi
- Target version set to v0.6
- Estimated time changed from 1.00 h to 3.00 h
I can reproduce this issue for NFD and ndn-tools on Ubuntu 16.04.
I'm working on patches.
Updated by Junxiao Shi over 7 years ago
- % Done changed from 0 to 30
ndn-tools https://gerrit.named-data.net/3897 is done.
NFD https://gerrit.named-data.net/3898 can compile, but unit tests and features using nfd::CommandAuthenticator
are not working.
nfd::CommandAuthenticator
depends on ndn::security::CommandInterestValidator
to check the timestamp, but that class is only compatible with v1 Key names.
Updated by Junxiao Shi over 7 years ago
- Blocked by Task #3920: Convert existing validator implementations to v2::Validator added
Updated by Davide Pesavento over 7 years ago
- Priority changed from Urgent to Immediate
Change 3547 has been merged, thus NFD build is broken right now. Raising priority.
Updated by Junxiao Shi over 7 years ago
https://gerrit.named-data.net/3898 patchset2 disables timestamp checking in nfd::CommandAuthenticator
so that unit testing can pass.
This is a temporary solution until ndn-cxx can provide a suitable validator.
NFD can still run, and local ndnping
+ ndnpingserver
works. However, prefix registration via /localhop/nfd
command prefix probably will not work because it relies on v1 validator.
Mgmt/TestCommandAuthenticator/BadKeyLocator_BadCertName
test case is deleted because KeyLocator carries key name in v2 so a transformation step from unversioned certificate name to key name is unnecessary. It is processed in the same way as NotAuthorized
test case.
Updated by Junxiao Shi over 7 years ago
- Tracker changed from Bug to Task
- Subject changed from Build broken by ndn-cxx v2::KeyChain to Adapt to ndn-cxx v2::KeyChain and Validator
- Description updated (diff)
- Priority changed from Immediate to High
- Estimated time changed from 3.00 h to 9.00 h
I intend to keep this issue open, and use the same issue to follow up with upcoming breaking changes from ndn-cxx.
Updated by Junxiao Shi over 7 years ago
Updated by Junxiao Shi over 7 years ago
- % Done changed from 30 to 40
https://gerrit.named-data.net/3951 provides ndn::security::v2::getKeyLocatorName
helper function which extracts KeyLocator Name from Data or signed Interest. This is needed to implement a validation policy in nfd::CommandAuthenticator
and avoid duplicating code. ValidationPolicySimpleHierarchy
and ValidationPolicyCommandInterest
have been simplified using this helper function. No new test cases are added because it's already covered by ValidationPolicySimpleHierarchy
and ValidationPolicyCommandInterest
test suites.
Updated by Junxiao Shi over 7 years ago
- % Done changed from 40 to 70
https://gerrit.named-data.net/3974 changes CommandAuthenticator
to use v2 ValidationPolicy
.
A Validator
is created for each command module.
If "certfile any" is specified, ValidationPolicyAcceptAll
is used.
Otherwise, certificates are installed as trust anchors in the validator, and ValidationPolicyCommandInterest
+ a custom policy that accepts any Interest directly signed by a trust anchor are used. The custom policy also stores KeyLocatorName as a packet tag on the "original Interest"; tag type 20 will be reserved in PacketTagTypes once we agree with this design.
In case both "certfile any" and individual certificates are specified for the same module, ValidationPolicyAcceptAll
is used but the validator may still have trust anchors; those trust anchors will not be used and are harmless.
Updated by Davide Pesavento over 7 years ago
Junxiao Shi wrote:
The custom policy also stores KeyLocatorName as a packet tag on the "original Interest"; tag type 20 will be reserved in PacketTagTypes once we agree with this design.
Please explain why this tag is needed.
Updated by Junxiao Shi over 7 years ago
Many test cases were using KeyChain::sign
to create command Interests, but KeyChain::sign
only creates signed Interests in v2. https://gerrit.named-data.net/3976 changes them to be signed by CommandInterestSigner
via CommandInterestSignerFixture
.
Regarding SignerTag
in CommandAuthenticator
: This tag allows the authentication function to obtain the signer name without calling getKeyLocatorName
again. The signer name needs to be passed to AcceptContinuation
, and can also enhance logging messages.
It's inefficient to call getKeyLocatorName
again, and it's also non-trivial to do so because getKeyLocatorName
requires ValidationState
which is not intended for use outside of Validator
.
Updated by Davide Pesavento over 7 years ago
Junxiao Shi wrote:
Regarding
SignerTag
inCommandAuthenticator
: This tag allows the authentication function to obtain the signer name without callinggetKeyLocatorName
again. The signer name needs to be passed toAcceptContinuation
, and can also enhance logging messages.
It's inefficient to callgetKeyLocatorName
again, and it's also non-trivial to do so becausegetKeyLocatorName
requiresValidationState
which is not intended for use outside ofValidator
.
Understood, thanks. And why can't the signer name be passed around as a regular function argument when needed?
Updated by Junxiao Shi over 7 years ago
why can't the signer name be passed around as a regular function argument when needed?
Validator API does not have a mechanism for a policy to directly communicate with calling code.
Both the policy and the calling code are reused across multiple packets, so std::bind
is not an option.
Changing that API is out of scope of this issue.
Updated by Junxiao Shi over 7 years ago
https://gerrit.named-data.net/3974 patchset6 fixes a potential memory error when config reload occurs during validation.
As Zhiyi answered on ndn-lib, if Validator
is destructed during validation, it would trigger undefined behavior in current version, and would cause validation to fail in a future revision.
The Validator
may be destructed if config reload occurs.
To solve this problem, CommandAuthenticator
retains the validator through shared_ptr during validation.
This situation is untestable at this moment because all validation policies used by CommandAuthenticator
complete synchronously.
However, CommandAuthenticator
cannot rely on such undocumented synchronous completion, and thus needs to use shared_ptr.
Updated by Davide Pesavento over 7 years ago
Junxiao Shi wrote:
This situation is untestable at this moment because all validation policies used by
CommandAuthenticator
complete synchronously.
Can you create a dummy async policy for unit test purposes?
Updated by Junxiao Shi over 7 years ago
This situation is untestable at this moment because all validation policies used by
CommandAuthenticator
complete synchronously.Can you create a dummy async policy for unit test purposes?
No. CommandAuthenticator
manages the policies internally. There is no external control on what policies are used.
Updated by Junxiao Shi over 7 years ago
https://gerrit.named-data.net/4059 v2::ValidatorNull
in ndncatchunks.
Updated by Junxiao Shi over 7 years ago
https://gerrit.named-data.net/4060 ValidatorNull
in nfdc. This commit is intended for a quick fix.
"nfdid" is unavailable. I'll re-implement NfdIdCollector
as a ValidationPolicy in another commit.
Updated by Junxiao Shi over 7 years ago
- % Done changed from 70 to 80
"nfdid" is unavailable. I'll re-implement
NfdIdCollector
as a ValidationPolicy in another commit.
20170814 NFD call decides to delete nfdId altogether, because it hasn't been useful in identifying testbed nodes.
https://gerrit.named-data.net/4121 performs the deletion.
Updated by Junxiao Shi about 7 years ago
- Blocks Task #4275: Release 0.6.0 added
Updated by Junxiao Shi about 7 years ago
- Status changed from In Progress to Closed
- % Done changed from 80 to 100
ndn-cxx has no more security-related breaking changes, thus this issue is complete.