Project

General

Profile

Bug #3645

Checkers in ValidatorConfig directly invoke ValidationFailureCallback

Added by Yingdi Yu over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
High
Assignee:
Category:
Security
Target version:
Start date:
06/08/2016
Due date:
% Done:

100%

Estimated time:

Description

When a rule (security::conf:Rule) invokes check method, it should try each checkers until one valid checker is found.

This implies that a checker should call ValidationSuccess callback when a packet pass the check, but if the checking fails, the checker should simply return the failure reasons. The ValidationFailure callback should be invoked by the rule when none of checkers can validate the packet.

However, current implementation invoke ValidationFailure callback directly in Checkers, as a result, only the first checker will be used.


Related issues

Blocks NFD - Bug #3640: ECDSA key unusable with automatic prefix propagationClosed2016-06-02

Blocks ndn-cxx - Task #2418: Improve ValidatorConfig test suiteClosed

History

#1 Updated by Yingdi Yu over 2 years ago

  • Blocks Bug #3640: ECDSA key unusable with automatic prefix propagation added

#2 Updated by Davide Pesavento over 2 years ago

  • Subject changed from Checkers in ValidatorConfig directly invoke ValidationFailiureCallback to Checkers in ValidatorConfig directly invoke ValidationFailureCallback
  • Target version set to v0.5

#3 Updated by Zhiyi Zhang over 2 years ago

  • Status changed from New to In Progress

#4 Updated by Junxiao Shi over 2 years ago

When reviewing http://gerrit.named-data.net/#/c/2879/2, I feel I may not understand the semantics of Checker correctly.

As stated in ValidatorConfig as of ndn-cxx 0.4.1:

A rule must have at least one checker property, a packet is treated as invalid if it cannot pass none of the checkers.

The first half means: "a rule with zero checker is ill-formed".

The second half is more difficult to understand because it has triple negations.
Literally it means: "a packet is invalid if it can pass any checker", which is counter-intuitive.
I guess there's a grammar error in there, and it actually means "a packet is valid if it can pass any checker".


I also have doubts on the API of Checker::check and Rule::check.
Doxygen says:

   * @param onValidated Callback function which is called when packet is immediately valid
   * @param onValidationFailed Call function which is called when packet is immediately invalid
   * @return -1 if packet is immediately invalid (onValidationFailed has been called)
   *          1 if packet is immediately valid (onValidated has been called)
   *          0 if further signature verification is needed.

It's clear that when check must invoke onValidated before returning 1, and must invoke onValidationFailed before returning -1, and must not return any value other than 1 -1 0.

However, I don't understand who is responsible for "further signature verification".
If Checker::check and Rule::check are responsible for performing the verification and passing back the result via one of the callbacks, onValidated and onValidationFailed shouldn't be described as "packet is immediately valid/invalid".
If the caller is responsible for performing the verification, the caller could have received the "immediate" result via the return value, and there's no need to have these two callbacks.

#5 Updated by Yingdi Yu over 2 years ago

Junxiao Shi wrote:

I guess there's a grammar error in there, and it actually means "a packet is valid if it can pass any checker".

Yes, this was what we intended to say. The commit should also fixes this error in doc.

Doxygen says:

   * @param onValidated Callback function which is called when packet is immediately valid
   * @param onValidationFailed Call function which is called when packet is immediately invalid
   * @return -1 if packet is immediately invalid (onValidationFailed has been called)
   *          1 if packet is immediately valid (onValidated has been called)
   *          0 if further signature verification is needed.

It's clear that when check must invoke onValidated before returning 1, and must invoke onValidationFailed before returning -1, and must not return any value other than 1 -1 0.

However, I don't understand who is responsible for "further signature verification".

In most cases, checker only check whether key name and data name comply with the rule. So in most cases, a rule (or checkers in a rule) does not contain any actual key bits, so there is no way to validate the signature. In most cases, a checker either returns 0, or -1. However, there is a special checker (Fixed Signer), which contains the actual key bits, so only this checker may return 1.

When we designed FixedSigner, we expect it will be used in some web-of-trust like model, where the you do not really check the key name, but simply check if you trust the key or not. However, since we did not have many web-of-trust style app, so we did not see many usage of FixedChecker.

#6 Updated by Junxiao Shi over 2 years ago

From note-5 I understand that the correct behavior of Rule::check is:

  • If at least one Checker::check returns 1, Rule::check should return 1, and onValidated should be invoked once.
  • Otherwise, if at least one Checker::check returns 0, Rule::check should return 0, and neither callback should be invoked. In this case, the caller is responsible for further signature verification.
  • Otherwise, Rule::check should return -1, and onValidationFailed should be invoked once.

Since the return value gives all information, and the callbacks will not be invoked after check returns.
I wonder what's the design rationale of having these callbacks?

#7 Updated by Yingdi Yu over 2 years ago

I do not remember the rationale of this callback. And I also feel it is not unnecessary now.

#8 Updated by Zhiyi Zhang over 2 years ago

I am sorry for my late push. I am now busy dealing with my graduation business. The commencement ceremony will be held on the last day of June. I will back to work immediately after that.

#9 Updated by Zhiyi Zhang over 2 years ago

About Junxiao's comment, I know using base64 is a better way, however after I searching the interfaces by github, I have no idea how to get base64 output of the identity certificate. Which interface should I use to realize the function or if there is an existing example for me to learn?

#10 Updated by Junxiao Shi over 2 years ago

I do not know the answer to note-9.
I have asked this question on ndn-lib mailing list. You may wait for a response.

#11 Updated by Zhiyi Zhang over 2 years ago

Notice that the file-name in a config can only be a file name. When the file name is a relative path, there will be an error.

e.g. file-name "build/tmp-files/cert001.cert" cannot work well.

This means the cert file should under the same directory of conf file.

#12 Updated by Junxiao Shi over 2 years ago

  • Blocks Task #2418: Improve ValidatorConfig test suite added

#13 Updated by Junxiao Shi over 2 years ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

Also available in: Atom PDF