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

add shs/350 #36

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 29 additions & 1 deletion codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strconv"
"strings"

b58 "github.com/jbenet/go-base58"
mh "github.com/jbenet/go-multihash"
)

Expand Down Expand Up @@ -243,6 +244,16 @@ func addressStringToBytes(p Protocol, s string) ([]byte, error) {
size := CodeToVarint(len(m))
b := append(size, m...)
return b, nil

case P_SHS: // shs
// the address is a varint prefixed multihash string representation
a := b58.Decode(s[:])
if len(a) == 0 {
return nil, fmt.Errorf("failed to parse shs addr: %s", s)
}
size := CodeToVarint(len(a))
b := append(size, a...)
return b, nil
}

return []byte{}, fmt.Errorf("failed to parse %s addr: unknown", p.Name)
Expand Down Expand Up @@ -280,7 +291,24 @@ func addressBytesToString(p Protocol, b []byte) (string, error) {
case P_ONION:
addr := strings.ToLower(base32.StdEncoding.EncodeToString(b[0:10]))
port := binary.BigEndian.Uint16(b[10:12])
return addr + ":"+ strconv.Itoa(int(port)), nil
return addr + ":" + strconv.Itoa(int(port)), nil

case P_SHS: // shs
// the address is a varint-prefixed multihash string representation
size, n, err := ReadVarintCode(b)
if err != nil {
return "", err
}

b = b[n:]
if len(b) != size {
return "", fmt.Errorf("inconsistent lengths")
}
m := b58.Encode(b)
if len(m) == 0 {
return m, fmt.Errorf("could not decode address")
}
return m, nil

default:
return "", fmt.Errorf("unknown protocol")
Expand Down
10 changes: 9 additions & 1 deletion multiaddr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ func TestConstructFails(t *testing.T) {
"/ip4/127.0.0.1/udp",
"/ip4/127.0.0.1/tcp/jfodsajfidosajfoidsa",
"/ip4/127.0.0.1/tcp",
"/ip4/127.0.0.1/shs",
"/ip4/127.0.0.1/tcp/8008/shs/7ZVaSM9sQ4mCCkn4MVIAhkcmSqF9V4u8p8r5hvxX4FnW", // contains illegal char 'I'
"/ip4/127.0.0.1/ipfs",
"/ip4/127.0.0.1/ipfs/tcp",
}
Expand Down Expand Up @@ -68,6 +70,7 @@ func TestConstructSucceeds(t *testing.T) {
"/sctp/1234",
"/udp/65535",
"/tcp/65535",
"/shs/7ZVaSM9sQ4mCCkn4MV1AhkcmSqF9V4u8p8r5hvxX4FnW",
"/ipfs/QmcgpsyWgH8Y8ajJz1Cu72KnS5uo2Aa2LpzU7kinSupNKC",
"/udp/1234/sctp/1234",
"/udp/1234/udt",
Expand All @@ -79,6 +82,7 @@ func TestConstructSucceeds(t *testing.T) {
"/ip4/127.0.0.1/udp/0",
"/ip4/127.0.0.1/tcp/1234",
"/ip4/127.0.0.1/tcp/1234/",
"/ip4/127.0.0.1/tcp/8008/shs/7ZVaSM9sQ4mCCkn4MV1AhkcmSqF9V4u8p8r5hvxX4FnW",
"/ip4/127.0.0.1/ipfs/QmcgpsyWgH8Y8ajJz1Cu72KnS5uo2Aa2LpzU7kinSupNKC",
"/ip4/127.0.0.1/ipfs/QmcgpsyWgH8Y8ajJz1Cu72KnS5uo2Aa2LpzU7kinSupNKC/tcp/1234",
}
Expand Down Expand Up @@ -353,6 +357,10 @@ func TestGetValue(t *testing.T) {
assertValueForProto(t, a, P_IP4, "0.0.0.0")
assertValueForProto(t, a, P_UDP, "12345")
assertValueForProto(t, a, P_UTP, "")

a = newMultiaddr(t, "/ip4/0.0.0.0/shs/7ZVaSM9sQ4mCCkn4MV1AhkcmSqF9V4u8p8r5hvxX4FnW") // test shs.
assertValueForProto(t, a, P_IP4, "0.0.0.0")
assertValueForProto(t, a, P_SHS, "7ZVaSM9sQ4mCCkn4MV1AhkcmSqF9V4u8p8r5hvxX4FnW")
}

func TestFuzzBytes(t *testing.T) {
Expand All @@ -374,7 +382,7 @@ func TestFuzzBytes(t *testing.T) {
}

func randMaddrString() string {
good_corpus := []string{"tcp", "ip", "udp", "ipfs", "0.0.0.0", "127.0.0.1", "12345", "QmbHVEEepCi7rn7VL7Exxpd2Ci9NNB6ifvqwhsrbRMgQFP"}
good_corpus := []string{"tcp", "ip", "udp", "ipfs", "shs", "0.0.0.0", "127.0.0.1", "12345", "QmbHVEEepCi7rn7VL7Exxpd2Ci9NNB6ifvqwhsrbRMgQFP", ""}
Copy link
Member

@jbenet jbenet Sep 17, 2016

Choose a reason for hiding this comment

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

empty is fine? this is the good corpus so should be a valid shs addr, no?

Weird that tests pass... cc @Kubuxu

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, empty is not fine. Copypaste mistake.

I realized that the shs protocol only uses ed25519 keys, so I changed it from length prefixed var size to ed25519.PublicKeySize and added length checks in codec.go. The test still passes both with the correct and with empty string, though.

Should I import agl's ed25519 just for using that constant or should I copy it into this package? In case I should import it, how? By vendoring, gx or directly?

Copy link
Member

@jbenet jbenet Sep 18, 2016

Choose a reason for hiding this comment

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

I realized that the shs protocol only uses ed25519 keys

Ahh, only ed25519? bummer

The test still passes both with the correct and with empty string, though.

yeah this is weird, and seems incorrect. @Kubuxu thoughts?

Should I import agl's ed25519 just for using that constant or should I copy it into this package? In case I should import it, how? By vendoring, gx or directly?

if its just that constant, nah, just copy it in, i think. (deps are unfortunately expensive :( )

Copy link
Member

Choose a reason for hiding this comment

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

We had similar issue in go-ipfs, even empty, no protocol maddr is a valid maddr.

Should we change it?

Copy link

Choose a reason for hiding this comment

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

I realized that the shs protocol only uses ed25519 keys, so I changed it from length prefixed var size to ed25519.PublicKeySize and added length checks in codec.go. The test still passes both with the correct and with empty string, though.

is there a chance that SHS will support other key algorithms in the future? if so, it should stay variable-length. cc @dominictarr

Choose a reason for hiding this comment

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

short answer: no.

shs is designed to be maximally metadata private (as is reasonable). so for this reason, there is no ciphersuite negioation. That also means downgrade attacks are simply not possible.
This also means you can't change shs -- to change shs you have to change it into another protocol, and then the new protocol gets a new multiformat name.
The plan is, as you are phasing in a new protocol, you just run it in parallel, say, on a different port.
then remove it when enough of the network has upgraded.


size := rand.Intn(256)
parts := make([]string, 0, size)
Expand Down
1 change: 1 addition & 0 deletions protocols.csv
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ code size name
132 16 sctp
301 0 utp
302 0 udt
350 V shs
421 V ipfs
480 0 http
443 0 https
Expand Down
2 changes: 2 additions & 0 deletions protocols.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const (
P_SCTP = 132
P_UTP = 301
P_UDT = 302
P_SHS = 350
P_IPFS = 421
P_HTTP = 480
P_HTTPS = 443
Expand All @@ -50,6 +51,7 @@ var Protocols = []Protocol{
Protocol{P_ONION, 96, "onion", CodeToVarint(P_ONION)},
Protocol{P_UTP, 0, "utp", CodeToVarint(P_UTP)},
Protocol{P_UDT, 0, "udt", CodeToVarint(P_UDT)},
Protocol{P_SHS, LengthPrefixedVarSize, "shs", CodeToVarint(P_SHS)},
Protocol{P_HTTP, 0, "http", CodeToVarint(P_HTTP)},
Protocol{P_HTTPS, 0, "https", CodeToVarint(P_HTTPS)},
Protocol{P_IPFS, LengthPrefixedVarSize, "ipfs", CodeToVarint(P_IPFS)},
Expand Down