Skip to content

Commit

Permalink
fix(routing/http): delegated routing of legacy RSA Peer IDs (#609)
Browse files Browse the repository at this point in the history 
This ensures /routing/v1/peers endpoint accepts all variants of PeerIDs
we see in the wild, namely:

- cidv1-libp2p-key-ed25519-peerid
- base58-ed25519-peerid
- cidv1-libp2p-key-rsa-peerid
- base58-rsa-peerid
  • Loading branch information
@lidel
lidel committed May 15, 2024
1 parent 0f223aa commit ac0bab3
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 75 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ The following emojis are used to highlight certain changes:
### Fixed

* `routing/http/server` now returns 404 Status Not Found when no records can be found.
* `routing/http/server` now supports legacy RSA PeerIDs encoded as Base58 Multihash

### Security

Expand Down
24 changes: 15 additions & 9 deletions routing/http/server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,20 +254,26 @@ func (s *server) findPeers(w http.ResponseWriter, r *http.Request) {
// See https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#string-representation
// We are liberal in inputs here, and uplift legacy PeerID to CID if necessary.
// Rationale: it is better to fix this common mistake than to error and break peer routing.
cid, err := cid.Decode(pidStr)

// Attempt to parse PeerID
pid, err := peer.Decode(pidStr)

if err != nil {
// check if input is peer ID in legacy format
if pid, err2 := peer.Decode(pidStr); err2 == nil {
cid = peer.ToCid(pid)
} else {
writeErr(w, "FindPeers", http.StatusBadRequest, fmt.Errorf("unable to parse peer ID as libp2p-key CID: %w", err))
return
// Retry by parsing PeerID as CID, then setting codec to libp2p-key
// and turning that back to PeerID.
// This is necessary to make sure legacy keys like:
// - RSA QmYyQSo1c1Ym7orWxLYvCrM2EmxFTANf8wXmmE7DWjhx5N
// - ED25519 12D3KooWD3eckifWpRn9wQpMG9R9hX3sD158z7EqHWmweQAJU5SA
// are parsed correctly.
pidAsCid, err2 := cid.Decode(pidStr)
if err2 == nil {
pidAsCid = cid.NewCidV1(cid.Libp2pKey, pidAsCid.Hash())
pid, err = peer.FromCid(pidAsCid)
}
}

pid, err := peer.FromCid(cid)
if err != nil {
writeErr(w, "FindPeers", http.StatusBadRequest, fmt.Errorf("unable to parse peer ID: %w", err))
writeErr(w, "FindPeers", http.StatusBadRequest, fmt.Errorf("unable to parse PeerID %q: %w", pidStr, err))
return
}

Expand Down
151 changes: 85 additions & 66 deletions routing/http/server/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestHeaders(t *testing.T) {
require.Equal(t, "text/plain; charset=utf-8", header)
}

func makePeerID(t *testing.T) (crypto.PrivKey, peer.ID) {
func makeEd25519PeerID(t *testing.T) (crypto.PrivKey, peer.ID) {
sk, _, err := crypto.GenerateEd25519Key(rand.Reader)
require.NoError(t, err)

Expand All @@ -71,6 +71,16 @@ func makePeerID(t *testing.T) (crypto.PrivKey, peer.ID) {
return sk, pid
}

func makeLegacyRSAPeerID(t *testing.T) (crypto.PrivKey, peer.ID) {
sk, _, err := crypto.GenerateRSAKeyPair(2048, rand.Reader)
require.NoError(t, err)

pid, err := peer.IDFromPrivateKey(sk)
require.NoError(t, err)

return sk, pid
}

func requireCloseToNow(t *testing.T, lastModified string) {
// inspecting fields like 'Last-Modified' is prone to one-off errors, we test with 1m buffer
lastModifiedTime, err := time.Parse(http.TimeFormat, lastModified)
Expand Down Expand Up @@ -220,22 +230,22 @@ func TestPeers(t *testing.T) {
return resp
}

t.Run("GET /routing/v1/peers/{non-peer-valid-cid} returns 400", func(t *testing.T) {
t.Run("GET /routing/v1/peers/{non-peerid} returns 400", func(t *testing.T) {
t.Parallel()

router := &mockContentRouter{}
resp := makeRequest(t, router, mediaTypeJSON, "bafkqaaa")
resp := makeRequest(t, router, mediaTypeJSON, "nonpeerid")
require.Equal(t, 400, resp.StatusCode)
})

t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 404 with correct body and headers (No Results, explicit JSON)", func(t *testing.T) {
t.Parallel()

_, pid := makePeerID(t)
_, pid := makeEd25519PeerID(t)
results := iter.FromSlice([]iter.Result[*types.PeerRecord]{})

router := &mockContentRouter{}
router.On("FindPeers", mock.Anything, pid, 20).Return(results, nil)
router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(results, nil)

resp := makeRequest(t, router, mediaTypeJSON, peer.ToCid(pid).String())
require.Equal(t, 404, resp.StatusCode)
Expand All @@ -250,11 +260,11 @@ func TestPeers(t *testing.T) {
t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 404 with correct body and headers (No Results, implicit JSON, wildcard Accept header)", func(t *testing.T) {
t.Parallel()

_, pid := makePeerID(t)
_, pid := makeEd25519PeerID(t)
results := iter.FromSlice([]iter.Result[*types.PeerRecord]{})

router := &mockContentRouter{}
router.On("FindPeers", mock.Anything, pid, 20).Return(results, nil)
router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(results, nil)

// Simulate request with Accept header that includes wildcard match
resp := makeRequest(t, router, "text/html,*/*", peer.ToCid(pid).String())
Expand All @@ -268,11 +278,11 @@ func TestPeers(t *testing.T) {
t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 404 with correct body and headers (No Results, implicit JSON, no Accept header)", func(t *testing.T) {
t.Parallel()

_, pid := makePeerID(t)
_, pid := makeEd25519PeerID(t)
results := iter.FromSlice([]iter.Result[*types.PeerRecord]{})

router := &mockContentRouter{}
router.On("FindPeers", mock.Anything, pid, 20).Return(results, nil)
router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(results, nil)

// Simulate request without Accept header
resp := makeRequest(t, router, "", peer.ToCid(pid).String())
Expand All @@ -285,10 +295,10 @@ func TestPeers(t *testing.T) {
t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 404 when router returns routing.ErrNotFound", func(t *testing.T) {
t.Parallel()

_, pid := makePeerID(t)
_, pid := makeEd25519PeerID(t)

router := &mockContentRouter{}
router.On("FindPeers", mock.Anything, pid, 20).Return(nil, routing.ErrNotFound)
router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(nil, routing.ErrNotFound)

// Simulate request without Accept header
resp := makeRequest(t, router, "", peer.ToCid(pid).String())
Expand All @@ -301,7 +311,7 @@ func TestPeers(t *testing.T) {
t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body and headers (JSON)", func(t *testing.T) {
t.Parallel()

_, pid := makePeerID(t)
_, pid := makeEd25519PeerID(t)
results := iter.FromSlice([]iter.Result[*types.PeerRecord]{
{Val: &types.PeerRecord{
Schema: types.SchemaPeer,
Expand All @@ -318,7 +328,7 @@ func TestPeers(t *testing.T) {
})

router := &mockContentRouter{}
router.On("FindPeers", mock.Anything, pid, 20).Return(results, nil)
router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(results, nil)

libp2pKeyCID := peer.ToCid(pid).String()
resp := makeRequest(t, router, mediaTypeJSON, libp2pKeyCID)
Expand All @@ -340,11 +350,11 @@ func TestPeers(t *testing.T) {
t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 404 with correct body and headers (No Results, NDJSON)", func(t *testing.T) {
t.Parallel()

_, pid := makePeerID(t)
_, pid := makeEd25519PeerID(t)
results := iter.FromSlice([]iter.Result[*types.PeerRecord]{})

router := &mockContentRouter{}
router.On("FindPeers", mock.Anything, pid, 0).Return(results, nil)
router.On("FindPeers", mock.Anything, pid, DefaultStreamingRecordsLimit).Return(results, nil)

resp := makeRequest(t, router, mediaTypeNDJSON, peer.ToCid(pid).String())
require.Equal(t, 404, resp.StatusCode)
Expand All @@ -359,7 +369,7 @@ func TestPeers(t *testing.T) {
t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body and headers (NDJSON)", func(t *testing.T) {
t.Parallel()

_, pid := makePeerID(t)
_, pid := makeEd25519PeerID(t)
results := iter.FromSlice([]iter.Result[*types.PeerRecord]{
{Val: &types.PeerRecord{
Schema: types.SchemaPeer,
Expand All @@ -376,7 +386,7 @@ func TestPeers(t *testing.T) {
})

router := &mockContentRouter{}
router.On("FindPeers", mock.Anything, pid, 0).Return(results, nil)
router.On("FindPeers", mock.Anything, pid, DefaultStreamingRecordsLimit).Return(results, nil)

libp2pKeyCID := peer.ToCid(pid).String()
resp := makeRequest(t, router, mediaTypeNDJSON, libp2pKeyCID)
Expand All @@ -393,11 +403,34 @@ func TestPeers(t *testing.T) {
require.Equal(t, expectedBody, string(body))
})

t.Run("GET /routing/v1/peers/{legacy-base58-peer-id} returns 200 with correct body (JSON)", func(t *testing.T) {
t.Parallel()
// Test matrix that runs the HTTP 200 scenario against different flavours of PeerID
// to ensure consistent behavior
peerIdtestCases := []struct {
peerIdType string
makePeerId func(t *testing.T) (crypto.PrivKey, peer.ID)
peerIdAsCidV1 bool
}{
// Test against current and past PeerID key types and string representations.
// https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#string-representation
{"cidv1-libp2p-key-ed25519-peerid", makeEd25519PeerID, true},
{"base58-ed25519-peerid", makeEd25519PeerID, false},
{"cidv1-libp2p-key-rsa-peerid", makeLegacyRSAPeerID, true},
{"base58-rsa-peerid", makeLegacyRSAPeerID, false},
}

_, pid := makePeerID(t)
results := iter.FromSlice([]iter.Result[*types.PeerRecord]{
for _, tc := range peerIdtestCases {
_, pid := tc.makePeerId(t)
var peerIDStr string
if tc.peerIdAsCidV1 {
// PeerID represented by CIDv1 with libp2p-key codec
// https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#string-representation
peerIDStr = peer.ToCid(pid).String()
} else {
// Legacy PeerID starting with "123..." or "Qm.."
// https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#string-representation
peerIDStr = b58.Encode([]byte(pid))
}
results := []iter.Result[*types.PeerRecord]{
{Val: &types.PeerRecord{
Schema: types.SchemaPeer,
ID: &pid,
Expand All @@ -410,67 +443,53 @@ func TestPeers(t *testing.T) {
Protocols: []string{"transport-foo"},
Addrs: []types.Multiaddr{},
}},
})

router := &mockContentRouter{}
router.On("FindPeers", mock.Anything, pid, 20).Return(results, nil)
}

legacyPeerID := b58.Encode([]byte(pid))
resp := makeRequest(t, router, mediaTypeJSON, legacyPeerID)
require.Equal(t, 200, resp.StatusCode)
t.Run("GET /routing/v1/peers/{"+tc.peerIdType+"} returns 200 with correct body and headers (NDJSON streaming response)", func(t *testing.T) {
t.Parallel()

header := resp.Header.Get("Content-Type")
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, mediaTypeJSON, header)
router := &mockContentRouter{}
router.On("FindPeers", mock.Anything, pid, DefaultStreamingRecordsLimit).Return(iter.FromSlice(results), nil)

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
resp := makeRequest(t, router, mediaTypeNDJSON, peerIDStr)
require.Equal(t, 200, resp.StatusCode)

expectedBody := `{"Peers":[{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-bitswap","transport-foo"],"Schema":"peer"},{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-foo"],"Schema":"peer"}]}`
require.Equal(t, expectedBody, string(body))
})
require.Equal(t, mediaTypeNDJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))

t.Run("GET /routing/v1/peers/{legacy-base58-peer-id} returns 200 with correct body (NDJSON)", func(t *testing.T) {
t.Parallel()
body, err := io.ReadAll(resp.Body)
require.NoError(t, err)

_, pid := makePeerID(t)
results := iter.FromSlice([]iter.Result[*types.PeerRecord]{
{Val: &types.PeerRecord{
Schema: types.SchemaPeer,
ID: &pid,
Protocols: []string{"transport-bitswap", "transport-foo"},
Addrs: []types.Multiaddr{},
}},
{Val: &types.PeerRecord{
Schema: types.SchemaPeer,
ID: &pid,
Protocols: []string{"transport-foo"},
Addrs: []types.Multiaddr{},
}},
expectedBody := `{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-bitswap","transport-foo"],"Schema":"peer"}` + "\n" + `{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-foo"],"Schema":"peer"}` + "\n"
require.Equal(t, expectedBody, string(body))
})

router := &mockContentRouter{}
router.On("FindPeers", mock.Anything, pid, 0).Return(results, nil)
t.Run("GET /routing/v1/peers/{"+tc.peerIdType+"} returns 200 with correct body and headers (JSON response)", func(t *testing.T) {
t.Parallel()

legacyPeerID := b58.Encode([]byte(pid))
resp := makeRequest(t, router, mediaTypeNDJSON, legacyPeerID)
require.Equal(t, 200, resp.StatusCode)
router := &mockContentRouter{}
router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(iter.FromSlice(results), nil)

header := resp.Header.Get("Content-Type")
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, mediaTypeNDJSON, header)
resp := makeRequest(t, router, mediaTypeJSON, peerIDStr)
require.Equal(t, 200, resp.StatusCode)

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type"))
require.Equal(t, "Accept", resp.Header.Get("Vary"))
require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control"))

expectedBody := `{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-bitswap","transport-foo"],"Schema":"peer"}` + "\n" + `{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-foo"],"Schema":"peer"}` + "\n"
require.Equal(t, expectedBody, string(body))
})
body, err := io.ReadAll(resp.Body)
require.NoError(t, err)

expectedBody := `{"Peers":[{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-bitswap","transport-foo"],"Schema":"peer"},{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-foo"],"Schema":"peer"}]}`
require.Equal(t, expectedBody, string(body))
})
}

}

func makeName(t *testing.T) (crypto.PrivKey, ipns.Name) {
sk, pid := makePeerID(t)
sk, pid := makeEd25519PeerID(t)
return sk, ipns.NameFromPeer(pid)
}

Expand Down

0 comments on commit ac0bab3

Please sign in to comment.