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

embed public keys inside ipns records, use for validation #5079

Merged
merged 4 commits into from
Jun 5, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions namesys/pb/namesys.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions namesys/pb/namesys.proto
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,10 @@ message IpnsEntry {
optional uint64 sequence = 5;

optional uint64 ttl = 6;

// in order for nodes to properly validate a record upon receipt, they need the public
// key associated with it. For old RSA keys, its easiest if we just send this as part of
// the record itself. For newer ed25519 keys, the public key can be embedded in the
// peerID, making this field unnecessary.
optional bytes pubKey = 7;
}
13 changes: 13 additions & 0 deletions namesys/publisher.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,26 @@ func PutRecordToRouting(ctx context.Context, r routing.ValueStore, k ci.PubKey,
return err
}

// if we can't derive the public key from the peerID, embed the entire pubkey in
// the record to make the verifiers job easier
if extractedPublicKey == nil {
pubkeyBytes, err := k.Bytes()
if err != nil {
return err
}

entry.PubKey = pubkeyBytes
}

namekey, ipnskey := IpnsKeysForID(id)

go func() {
errs <- PublishEntry(ctx, r, ipnskey, entry)
}()

// Publish the public key if a public key cannot be extracted from the ID
// TODO: once v0.4.16 is widespread enough, we can stop doing this
// and at that point we can even deprecate the /pk/ namespace in the dht
if extractedPublicKey == nil {
go func() {
errs <- PublishPublicKey(ctx, r, namekey, k)
Expand Down
38 changes: 34 additions & 4 deletions namesys/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package namesys
import (
"bytes"
"errors"
"fmt"
"time"

pb "github.com/ipfs/go-ipfs/namesys/pb"
peer "gx/ipfs/QmcJukH2sAFjY3HdBKq35WDzWoL3UUu2gt9wdfqZTUyM74/go-libp2p-peer"
pstore "gx/ipfs/QmdeiKhUy1TVGBaKxt7y1QmBDLBdisSrLJ1x58Eoj4PXUh/go-libp2p-peerstore"
ic "gx/ipfs/Qme1knMqwt1hKZbc1BmQFmnm9f36nyQGwXxPGVpVJ9rMK5/go-libp2p-crypto"

u "gx/ipfs/QmNiJuT8Ja3hMVpBHXv3Q6dwmperaQ6JjLtpMQgMCD7xvx/go-ipfs-util"
record "gx/ipfs/QmTUyK82BVPA6LmSzEJpfEunk9uBaQzWtMsNP917tVj4sT/go-libp2p-record"
Expand Down Expand Up @@ -65,10 +67,10 @@ func (v IpnsValidator) Validate(key string, value []byte) error {
log.Debugf("failed to parse ipns record key %s into peer ID", pidString)
return ErrKeyFormat
}
pubk := v.KeyBook.PubKey(pid)
if pubk == nil {
log.Debugf("public key with hash %s not found in peer store", pid)
return ErrPublicKeyNotFound

pubk, err := v.getPublicKey(pid, entry)
if err != nil {
return fmt.Errorf("getting public key failed: %s", err)
}

// Check the ipns record signature with the public key
Expand All @@ -94,6 +96,34 @@ func (v IpnsValidator) Validate(key string, value []byte) error {
return nil
}

func (v IpnsValidator) getPublicKey(pid peer.ID, entry *pb.IpnsEntry) (ic.PubKey, error) {
if entry.PubKey != nil {
pk, err := ic.UnmarshalPublicKey(entry.PubKey)
if err != nil {
// TODO: i think this counts as a 'malformed record' and should be discarded
Copy link
Member Author

Choose a reason for hiding this comment

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

@Stebalien WDYT? I think enforcing that the data in that field is valid and that it matches the expected peerID is the right thing to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, makes sense.

Copy link
Member

Choose a reason for hiding this comment

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

I agree. If the user specifies the public key, it had better damn well be correct.

log.Debugf("public key in ipns record failed to parse: ", err)
return nil, err
}
expPid, err := peer.IDFromPublicKey(pk)
Copy link
Member Author

Choose a reason for hiding this comment

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

This could fail if the user manually created the peerID differently than this function does (for example, made their key the hash of a public key that this function would choose to embed). I'm not sure how much of a problem is

if err != nil {
return nil, fmt.Errorf("could not regenerate peerID from pubkey: %s", err)
}

if pid != expPid {
return nil, fmt.Errorf("pubkey in record did not match expected pubkey")
}

return pk, nil
}

pubk := v.KeyBook.PubKey(pid)
if pubk == nil {
log.Debugf("public key with hash %s not found in peer store", pid)
return nil, ErrPublicKeyNotFound
}
return pubk, nil
}

// IpnsSelectorFunc selects the best record by checking which has the highest
// sequence number and latest EOL
func (v IpnsValidator) Select(k string, vals [][]byte) (int, error) {
Expand Down