Task #4089
closed
Adapt to ndn-cxx v2::KeyChain and Validator
100%
Description
Adapt NFD and ndn-tools codebase to use ndn-cxx v2::KeyChain and v2::Validator.
Updated by Zhiyi Zhang almost 7 years ago
- Related to Task #3098: Merge KeyChain branch added
Updated by Junxiao Shi almost 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 almost 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 almost 7 years ago
- Blocked by Task #3920: Convert existing validator implementations to v2::Validator added
Updated by Davide Pesavento almost 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 almost 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 almost 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 almost 7 years ago
Updated by Junxiao Shi almost 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 almost 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 almost 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 almost 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 almost 7 years ago
Junxiao Shi wrote:
Regarding
SignerTaginCommandAuthenticator: This tag allows the authentication function to obtain the signer name without callinggetKeyLocatorNameagain. The signer name needs to be passed toAcceptContinuation, and can also enhance logging messages.
It's inefficient to callgetKeyLocatorNameagain, and it's also non-trivial to do so becausegetKeyLocatorNamerequiresValidationStatewhich 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 almost 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 almost 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 almost 7 years ago
Junxiao Shi wrote:
This situation is untestable at this moment because all validation policies used by
CommandAuthenticatorcomplete synchronously.
Can you create a dummy async policy for unit test purposes?
Updated by Junxiao Shi almost 7 years ago
This situation is untestable at this moment because all validation policies used by
CommandAuthenticatorcomplete 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 6 years ago
https://gerrit.named-data.net/4059 v2::ValidatorNull in ndncatchunks.
Updated by Junxiao Shi over 6 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 6 years ago
- % Done changed from 70 to 80
"nfdid" is unavailable. I'll re-implement
NfdIdCollectoras 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 over 6 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.