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

IPRS Proof of Concept #4502

Closed
wants to merge 1 commit into from
Closed

IPRS Proof of Concept #4502

wants to merge 1 commit into from

Conversation

dirkmc
Copy link
Contributor

@dirkmc dirkmc commented Dec 18, 2017

This PR is a proof of concept demonstrating that the go-iprs implementation at https://github.com/dirkmc/go-iprs can be used to replace IPNS. Please provide feedback on the go-iprs implementation.
Thank you
Dirk

@daviddias
Copy link
Member

Nice going @dirkmc :) 👏🏽

I have a couple of questions but I also want to admit that my proficiency reviewing Go is still not at the level where I would like to be and so my apologies if I miss something obvious.

  • This IPRS implementation augmented the previous record implementation but didn't convert it to IPLD, any reason why that wasn't included?
  • The new IPRS protobuf is packing everything in one struct -- https://github.com/dirkmc/go-iprs/blob/master/pb/iprs.pb.go#L106-L115 --, where the spec says each record will be at least 3 IPLD nodes, one for:
    • Validity Scheme
    • Signature of the record
    • The record itself

So it should be a graph with <Signature> => <Record with Data> => <Validity Scheme>

This means that the Records will start to get stored through the DAG API and into the blocks datastore, instead of the datastore datastore.

Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So basically you are replacing ipns with iprs, but does it provide equivalent functionality?
It doesn't seem so -- I think we need to find a way for the two to coexist.

// setup IPRS
kvs := iprsvs.NewKadValueStore(n.Repo.Datastore(), n.Routing)
n.Iprs = iprs.NewRecordSystem(kvs, size)
n.RecordFactory = iprsrec.NewRecordFactory(kvs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these details should be hidden behind the iprs facade.

Copy link
Contributor

@vyzo vyzo Dec 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I refer to the record factory and key-value store details.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I agree. At present the code in namesys that increments the record sequence number looks in the routing system's local datastore to avoid going out to the network, so I copied that same mechanism, but probably there is a better way of managing sequence numbers.

The Records are composed of a RecordSigner (eg private key or certificate) and RecordValidity (eg EOL or Time Range). These can be used in any permutation, and in future there will be more of them, eg Signed, based on ancestry (chain) so I thought it would get unwieldy to have a method to create every permutation in the interface itself.

@@ -782,6 +796,10 @@ func (n *IpfsNode) SetupOfflineRouting() error {
return err
}

kvs := iprsvs.NewKadValueStore(n.Repo.Datastore(), n.Routing)
n.Iprs = iprs.NewRecordSystem(kvs, size)
n.RecordFactory = iprsrec.NewRecordFactory(kvs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

@dirkmc
Copy link
Contributor Author

dirkmc commented Dec 19, 2017

Thanks for your feedback @diasdavid and @vyzo

@diasdavid I agree that it would make sense for IPRS to use IPLD, I will add that functionality next.
With respect to the record structure, the namesys implementation of IPNS in go-ipfs provides a Validator that is invoked by the DHT on each host as the record passes through the network. The signature of the validator (it does not have a context.Context parameter) and the way that it is invoked by the DHT imply that validation should happen locally on the record itself (ie, validation should not go out to the network). So in order to be able to use the DHT implementation that go-libp2p provides, I broke the model into validation functions that check things locally on the record like record EOL, and verification functions that can go out to the network to retrieve public keys, certificates etc to verify the record.

So with respect to the model suggested in the spec

  • Validity Scheme
  • Signature of the record (ie verification data)
  • The record itself

Should there also be?

  • Validity data (EOL, time range etc)
  • Verification Scheme (Verify signature by retrieving a public key / certificate etc)

In our implementation of IPRS we have provided two validity schemes (EOL and Time Range) and two Verification schemes (Public Key and Certificate) and these can be combined in any permutation (eg Key / EOL or Cert / Time Range etc) which is why I thought it might make sense to keep them separate. Please let me know your thoughts.

Do you think we should use the go-libp2p DHT implementation or should we build a new Routing system for IPRS? Note that the go-libp2p implementation requires a Validator for each kind of record (eg /pk/, /ipns/, etc)

@vyzo I agree that the IPRS implementation should be able to resolve IPNS records (and DNS and proquint). Do you think it should also be able to publish IPNS records? I was imagining that would not be necessary, I just want to confirm that's what you're thinking

@dirkmc
Copy link
Contributor Author

dirkmc commented Dec 19, 2017

@vyzo today I added a commit that enables our IPRS implementation to resolve IPNS records. See this test for an example.

@dirkmc
Copy link
Contributor Author

dirkmc commented Dec 19, 2017

I should also mention that my understanding of the requirements for IPRS come entirely from looking at the IPNS code and reading the IPRS spec, so I'm not sure what you guys have been discussing amongst yourselves with respect to where you see it going. So I apologize if I'm asking questions that don't make sense, and happy to let you guys set the direction for where you would like it to go.

@dirkmc
Copy link
Contributor Author

dirkmc commented Dec 19, 2017

@vyzo with respect to your question about whether this IPRS implementation provides equivalent functionality to IPNS, it does, the EOL / Key record:

An IPNS record

  • has an EOL
  • is signed with a private key
  • has a path of /ipns/<public key hash>

The IPRS EOL / Key record

  • has an EOL
  • is signed with a private key
  • has a path of /iprs/<public key hash>

Example:
https://github.com/dirkmc/go-iprs#creating-an-eol-record-signed-with-a-private-key

@vyzo
Copy link
Contributor

vyzo commented Dec 20, 2017

@dirkmc there is additional functionality in namesys (like pubsub) that has non analogue in iprs.

I don't think it's realistic to completely replace ipns like this, but it would be possible to merge it by making it complementary to ipns.

So the command should not replace resolution of ipns by iprs; instead it should recognize /iprs prefix and resolve with iprs otherwise resolve with ipns.
In fact it may be preferable to integrate iprs as a namesys resolver. So implement the other way around, instead of trying to replace ipns just extend it to handle iprs records.

@dirkmc
Copy link
Contributor Author

dirkmc commented Dec 20, 2017

@vyzo I noticed that a pubsub mechanism was recently added to namesys. Could you explain why this change was made? Maybe it would make sense to use pibsub for IPRS also

@vyzo
Copy link
Contributor

vyzo commented Dec 20, 2017

@dirkmc the intention is to enable push distribution of records, so that clients can resolve instantly once they are subscribed to the name topic.

@dirkmc
Copy link
Contributor Author

dirkmc commented Dec 20, 2017

@vyzo I see. I guess I will add that to IPRS too, does that make sense to you?

@vyzo
Copy link
Contributor

vyzo commented Dec 20, 2017

@dirkmc So basically you want to re-implement the whole of ipns inside iprs? Not sure it's the right thing to do, for a pragmatic integration approach.

Why not try the other way around? Ie make a publisher/resolver that integrates iprs inside ipns?

@dirkmc
Copy link
Contributor Author

dirkmc commented Dec 21, 2017

@vyzo the context is that we are building a system that needs some functionality provided by iprs externally to ipfs, ie we need to include it as a separate library. So Why suggested we build out iprs for the ipfs project as a way of achieving that goal. @whyrusleeping just want to make sure we’re on the right track here, can you confirm?

@whyrusleeping
Copy link
Member

@vyzo we've been meaning to use ipld to replace the existing protobuf ipns records. So this is generally the direction we want to go, however @diasdavid is the one who really knows what we should be doing precisely

@vyzo
Copy link
Contributor

vyzo commented Dec 21, 2017

@whyrusleeping @dirkmc that's fine for long term plans, but I was thinking in terms of ease of integration and evolutionary path forward.

It would be nice to quickly integrate it as an experimental resolver under ipns, and have it used and tested in the real world before endeavoring to replace namsys.

@daviddias
Copy link
Member

The decisions we make tend to stay for a while. The "let's get just something quick" are some famous last words just as "IPv4 is just a lab experiment" and "Let's just get some name service working and call it DNS".

If there is a real path moving forward out of the quick implementation, let's have it clearly documented to make sure we are not missing anything.

As @whyrusleeping said, we really want IPNS records to be IPLD nodes, we know we are going to need them just like we know we will need Keys to be represented by IPLD nodes and/or graphs.

We also want the Provider records to be IPLD nodes and we want that both Provider and IPNS records are just an application of the IPRS spec.

Happy to have more conversation about this, either in a call or through spec work.

@dirkmc
Copy link
Contributor Author

dirkmc commented Dec 23, 2017

@diasdavid that makes sense to me. I think it would be good to have a call in the new year to understand in more detail where you guys would like to go with this. In the mean time I will change the proof of concept to use IPLD

@mildred
Copy link
Contributor

mildred commented Jan 3, 2018

This is probably too early, but I suppose we'll need to have a plugin system integrated with IPRS so we can have different schemes.

@dirkmc
Copy link
Contributor Author

dirkmc commented Jan 4, 2018

@mildred could you explain a little more what you're thinking? It will be possible to add more record signers (eg public key, certificate, etc) and record validators (eg EOL, Time Range etc) as long as they implement the corresponding interfaces

@mildred
Copy link
Contributor

mildred commented Jan 4, 2018

@dirkmc I started a new issue to discuss about it: #4544 I don't think here is the place to do so.

@whyrusleeping
Copy link
Member

@diasdavid @dirkmc want to set up a call sometime? I would really love to start pushing IPRS forward, and investing some resources into both getting that done, and making the whole naming system better. Its sorely needed

@dirkmc
Copy link
Contributor Author

dirkmc commented Jan 7, 2018

@whyrusleeping sure sounds good. I’m in Perth, Western Australia till Wednesday so it will probably be easier once I’m back in the New York time zone this weekend (it takes a couple of days to get back). FYI I’m also almost done converting the proof of concept to use IPLD so it would be good to go over the architecture with you guys by phone to see where there could be improvements

@whyrusleeping
Copy link
Member

@dirkmc great! lets catch up when youre back

@vyzo
Copy link
Contributor

vyzo commented Jan 7, 2018

@whyrusleeping @dirkmc i'd like to participate in this call too.

@whyrusleeping
Copy link
Member

@vyzo sure thing!

@dirkmc
Copy link
Contributor Author

dirkmc commented Jan 8, 2018

@diasdavid @whyrusleeping I've converted the go-iprs proof of concept to use IPLD, it's on the feat/ipld branch here:
https://github.com/dirkmc/go-iprs/tree/feat/ipld

Note that there is currently no protection against modification of DHT records.

@dirkmc
Copy link
Contributor Author

dirkmc commented Jan 25, 2018

I've now merged the IPLD changes suggested by @diasdavid into master:
https://github.com/dirkmc/go-iprs

The publisher publishes a Record object that is an IPLD node and the resolver returns an IPLD node:

Publish(ctx context.Context, iprsKey rsp.IprsPath, record *r.Record) error
Resolve(ctx context.Context, name string) (*node.Link, []string, error)

The IPLD node structure:

type Node struct {
	cborld.Node

	Version   uint64
	Value     []byte
	Validity  *Validity
	Signature []byte
}
type Validity struct {
	VerificationType IprsVerificationType
	Verification     interface{}
	ValidationType   IprsValidationType
	Validation       interface{}
}

Verification data like public keys and certificates are stored in the block store

IPRS Records are published to a path that consists of a validation CID (eg CID of a public key) and an ID, eg /iprs/<cid>/photos

The record value is the path to an IPLD node in the block store. It can be

  • an IPFS path eg /ipfs/<B58hash>/some/path
  • the raw bytes of a CID pointing to an IPLD node
  • a CID in string format with an optional path, eg <cid>/some/path
  • an IPNS path eg /ipns/<B58hash>/some/path or /ipns/ipfs.io/some/path
  • an IPRS path eg /iprs/<cid>/photos/3/size or /iprs/ipfs.io/some/path

So a structure can be set up like the following:

/iprs/<alice pubk cid>/photos => /ipfs/<hash>/files/photos
/iprs/<bob pubk cid>/favorite-photo => /iprs/<alice pubk cid>/photos/3
/iprs/<bob pubk cid>/favorite-photo-size => /iprs/<bob pubk cid>/favorite-photo/size

Records are created with a RecordValidation and a RecordSigner. RecordValidation indicates under what conditions the record is considered valid, for example before a certain date (EOL) or between certain dates (TimeRange). RecordSigner adds verification data to a record, by signing it, eg with a private key, or with an x509 certificate.

I'd love to hear feedback on these structures and on the architecture in general.

@dirkmc
Copy link
Contributor Author

dirkmc commented Jan 27, 2018

@Stebalien with regards to your comment over on #4152:

I've been working on adapting PubSub to use IPLD for messages. Instead of sending arbitrary data, one would send a CID of an IPLD object along with (optionally) a dag rooted at this CID.
For security (DoS) reasons, we need to be able to validate pubsub messages before we forward them (otherwise, it's trivial to DoS pubsub).
I'd like to be able to validate received DAGs using both blocks attached to the messages and blocks already present on the local node but I do not want nodes to start bitswaping for blocks during validation; otherwise, peers could trivially trick other peers into downloading arbitrary data.

This is similar to an issue I've been trying to find a solution to with the IPRS proof of concept. Currently it uses the DHT to pass around an IPLD CID that points to an IPRS Record with signatures etc. I store the public key / x509 certs in the block store, and the DHT can verify them on GET or PUT. However I'm concerned about the performance of bitswapping to get public keys / certs. Do you have a sense of how that would perform, and how it could be improved?

@markg85
Copy link
Contributor

markg85 commented Nov 22, 2018

Just curious as this has been silent for nearly a year.
Is there an update on this? As it sounds quite interesting to have!

@momack2 momack2 added this to Inbox in ipfs/go-ipfs May 9, 2019
@Stebalien
Copy link
Member

At this point, we've moved on and will likely use https://github.com/libp2p/go-smart-record/ for flexible records.

@Stebalien Stebalien closed this Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

8 participants