Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check command violates data immutability principle #60

Closed
yoursunny opened this issue Mar 7, 2021 · 10 comments
Closed

check command violates data immutability principle #60

yoursunny opened this issue Mar 7, 2021 · 10 comments

Comments

@yoursunny
Copy link
Contributor

According to NDN Protocol Design Principle:

Data-Centricity and Data Immutability: NDN should fetch uniquely named, immutable “data packets” requested using “interest packets”.
Data packet immutability allows disambiguation of coordination in distributed system that may not be always connected. Although data packets are immutable, applications can make changes to the communicated content by creating new versions of immutable data packets.

However, CommandChecker._check function is repeatedly sending an Interest with the same name and CanBePrefix=false, expecting to receive a different answer.
This implies that this protocol depends on Data that is not immutable.

data_name, meta_info, content = await self.app.express_interest(
name, must_be_fresh=True, can_be_prefix=False, lifetime=1000)

@yoursunny
Copy link
Contributor Author

This issue is the same as #39 . @JonnyKong reverted the commit in an attempt to solve #46 , so that the problem came back.
One solution to solve this issue without running into #46 again is appending an additional random name component to the check Data name; the corresponding check Interest should either set CanBePrefix element or have that additional component.

@zjkmxy
Copy link
Collaborator

zjkmxy commented Aug 9, 2022

I think the problem does not apply to the current code because the check Interest is signed now.

@zjkmxy zjkmxy closed this as completed Aug 9, 2022
@yoursunny
Copy link
Contributor Author

yoursunny commented Aug 9, 2022

This is NOT solved, so please reopen.

Specification - Check says nothing about "signed".

The code added ApplicationParameters but it's not signed.
The ApplicationParameters doesn't contain any random element and thus generates the same ParametersSha256DigestComponent.

data_name, meta_info, content = await self.app.express_interest(
name, cmd_param_bytes, must_be_fresh=True, can_be_prefix=False, lifetime=1000)

@zjkmxy
Copy link
Collaborator

zjkmxy commented Aug 9, 2022

This is NOT solved, so please reopen.

Specification - Check says nothing about "signed".

The code added ApplicationParameters but it's not signed. The ApplicationParameters doesn't contain any random element and thus generates the same ParametersSha256DigestComponent.

It is signed. Every Interest with an AppParam in python ndn is signed.
https://github.com/named-data/python-ndn/blob/b9e0dfe6b7e5fdd731d59425c6cdd9e0f885271f/src/ndn/app.py#L210-L211

Python-ndn's App does not give a nonce or timestamp to the SignatureInfo for now, but It's a different problem.

@yoursunny
Copy link
Contributor Author

Every Interest with an AppParam in python ndn is signed.

This is very surprising behavior and it essentially means it's impossible to create a parameterized but not signed Interest.

Python-ndn's App does not give a nonce or timestamp to the SignatureInfo for now, but It's a different problem.

Then the original problem is NOT solved, because neither ApplicationParameters nor SignatureInfo contains any random element, and the signing algorithm is not guaranteed to be non-deterministic.

Moreover, documentation must be updated.

@zjkmxy
Copy link
Collaborator

zjkmxy commented Aug 9, 2022

This is very surprising behavior and it essentially means it's impossible to create a parameterized but not signed Interest.

There are multiple ways depending on the need:

  • If an empty SignatureValue is acceptable, one can use NullSigner, which set SignatureType=200.
  • If one only accepts an Interest without SignatureInfo and SignatureValue, then he must use the low-level API make_interest.

Then the original problem is NOT solved, because neither ApplicationParameters nor SignatureInfo contains any random element, and the signing algorithm is not guaranteed to be non-deterministic.

OK. Let me reopen the issue. Ref: named-data/python-ndn#46.

Moreover, documentation must be updated.

I have updated the Specification page, but have no time to handle the rest.

@zjkmxy zjkmxy reopened this Aug 9, 2022
@yoursunny
Copy link
Contributor Author

I have updated the Specification page, but have no time to handle the rest.

This is the page I'm referring to.
The page currently indicates the Check command is a parameterized Interest, not a signed Interest.

When using signed Interests, application protocol must define:

  • What fields are required in SigInfo.
  • What's the trust schema, including a list of acceptable crypto algorithms and the criteria of signing keys.

@killerdbob
Copy link

Hi, how to solve this problem? I cannot use ndnts-repo to communicate with python-ndn-repo.

@yoursunny
Copy link
Contributor Author

yoursunny commented Sep 27, 2022

@killerdbob

NDNts @ndn/repo-external package can communicate with ndn-python-repo version 0.2a5, which does not suffer from this protocol design flaw.
See NDNts documentation for how to install that version, as well as example code snippets.

NDNts is unable to support more recent versions of ndn-python-repo due to the design flaw indicated in the top post of this issue.
While several other libraries were able to support this version, they rely on specific implementation behaviors and cannot work reliably in all situations with all network forwarders.
Thus, I decide not to support this faulty protocol design.

As soon as ndn-python-repo design updates their protocol documentation and implementation that resolves this design flaw, I will update NDNts to support the newer version.

@yoursunny
Copy link
Contributor Author

Resolved in dda1dce .

I have re-enabled ndn-python-repo client in NDNts @ndn/repo-external package, cc @killerdbob .

yoursunny added a commit to yoursunny/NDNts that referenced this issue Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants