-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
embed public keys inside ipns records, use for validation #5079
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
Changes from 1 commit
bc129ac
af68a38
c66c5c6
cc37903
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
|
@@ -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 | ||
|
@@ -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 | ||
log.Debugf("public key in ipns record failed to parse: ", err) | ||
return nil, err | ||
} | ||
expPid, err := peer.IDFromPublicKey(pk) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, makes sense.
There was a problem hiding this comment.
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.