Skip to content

Commit

Permalink
remove new DHT record author check
Browse files Browse the repository at this point in the history
We're going to just fix this a future commit. *This* change breaks publishing
IPNS records using alternative IPNS keys (because the author signature (peer ID)
differs from the record signature).

We're going to fix it by validating the IPNS signature and ditching the
author/signature fields.

License: MIT
Signed-off-by: Steven Allen <[email protected]>
  • Loading branch information
Stebalien committed Jan 26, 2018
1 parent 9536a1e commit 2c39138
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 16 deletions.
4 changes: 3 additions & 1 deletion namesys/ipns_validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
func TestValidation(t *testing.T) {
// Create a record validator
validator := make(record.Validator)
validator["ipns"] = &record.ValidChecker{ValidateIpnsRecord, true}
validator["ipns"] = &record.ValidChecker{Func: ValidateIpnsRecord, Sign: true}

// Generate a key for signing the records
r := u.NewSeededRand(15) // generate deterministic keypair
Expand Down Expand Up @@ -46,6 +46,7 @@ func TestValidation(t *testing.T) {
t.Fatal(err)
}

/* TODO(#4613)
// Create IPNS record path with a different private key
_, ipnsWrongAuthor := genKeys(t, r)
wrongAuthorRec, err := record.MakePutRecord(priv, ipnsWrongAuthor, val, true)
Expand Down Expand Up @@ -97,6 +98,7 @@ func TestValidation(t *testing.T) {
if err != ErrInvalidAuthor {
t.Fatal("ValidateIpnsRecord should have returned ErrInvalidAuthor")
}
*/

// Create expired entry
expiredEntry, err := CreateRoutingEntryData(priv, path.Path("foo"), 1, ts.Add(-1*time.Hour))
Expand Down
33 changes: 18 additions & 15 deletions namesys/publisher.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ var ErrExpiredRecord = errors.New("expired record")
// unknown validity type.
var ErrUnrecognizedValidity = errors.New("unrecognized validity type")

// ErrInvalidAuthor is returned when an IpnsRecord has an
// author that does not match the IPNS path
var ErrInvalidAuthor = errors.New("author does not match path")

// ErrInvalidPath should be returned when an ipns record path
// is not in a valid format
var ErrInvalidPath = errors.New("record path invalid")
Expand Down Expand Up @@ -314,17 +310,24 @@ func ValidateIpnsRecord(r *record.ValidationRecord) error {
return err
}

// Note: The DHT will actually check the signature so we don't
// need to do that here

// Author in key must match author in record
pid, err := peer.IDFromString(r.Key)
if err != nil {
return ErrInvalidAuthor
}
if pid != r.Author {
return ErrInvalidAuthor
}
// NOTE/FIXME(#4613): We're not checking the DHT signature/author here.
// We're going to remove them in a followup commit and then check the
// *IPNS* signature. However, to do that, we need to ensure we *have*
// the public key and:
//
// 1. Don't want to fetch it from the network when handling PUTs.
// 2. Do want to fetch it from the network when handling GETs.
//
// Therefore, we'll need to either:
//
// 1. Pass some for of offline hint to the validator (e.g., using a context).
// 2. Ensure we pre-fetch the key when performing gets.
//
// This PR is already *way* too large so we're punting that fix to a new
// PR.
//
// This is not a regression, it just restores the current (bad)
// behavior.

// Check that record has not expired
switch entry.GetValidityType() {
Expand Down

0 comments on commit 2c39138

Please sign in to comment.