From 6d5accb338a41e3fceb77e536b913cfb93a4eac3 Mon Sep 17 00:00:00 2001 From: Sarah Jamie Lewis Date: Mon, 27 Sep 2021 19:52:25 -0700 Subject: [PATCH 1/3] Make IsValidHostname More Robust IsValidHostname now rejects public keys that are invalid ed25519 curve points in addition to ed25519 points that contain torsion components (which are defined to be invalid Tor Hostnames). Note: The lack of these checks previously would have been unlikely to manifest as an issue further up the stack because Tor would have prevented Cwtch from connecting to bad curve points, the Tapir authentication protocol would have failed with invalid curve points, and the experimental group chats only rely on signatures for voluntary authorship attribution, rather than e.g. consensus or security. --- go.mod | 1 + go.sum | 2 ++ tor/torUtils.go | 32 +++++++++++++++++- tor/torUtils_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index fcb6e13..7d0bf24 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module git.openprivacy.ca/openprivacy/connectivity go 1.13 require ( + filippo.io/edwards25519 v1.0.0-rc.1 git.openprivacy.ca/openprivacy/bine v0.0.4 git.openprivacy.ca/openprivacy/log v1.0.2 golang.org/x/crypto v0.0.0-20201012173705-84dcc777aaee diff --git a/go.sum b/go.sum index 3541254..f9bd8bf 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +filippo.io/edwards25519 v1.0.0-rc.1 h1:m0VOOB23frXZvAOK44usCgLWvtsxIoMCTBGJZlpmGfU= +filippo.io/edwards25519 v1.0.0-rc.1/go.mod h1:N1IkdkCkiLB6tki+MYJoSx2JTY9NUlxZE7eHn5EwJns= git.openprivacy.ca/openprivacy/bine v0.0.4 h1:CO7EkGyz+jegZ4ap8g5NWRuDHA/56KKvGySR6OBPW+c= git.openprivacy.ca/openprivacy/bine v0.0.4/go.mod h1:13ZqhKyqakDsN/ZkQkIGNULsmLyqtXc46XBcnuXm/mU= git.openprivacy.ca/openprivacy/log v1.0.2 h1:HLP4wsw4ljczFAelYnbObIs821z+jgMPCe8uODPnGQM= diff --git a/tor/torUtils.go b/tor/torUtils.go index cd4c233..8b194a8 100644 --- a/tor/torUtils.go +++ b/tor/torUtils.go @@ -3,6 +3,7 @@ package tor import ( "encoding/base32" "errors" + "filippo.io/edwards25519" "git.openprivacy.ca/openprivacy/bine/control" "git.openprivacy.ca/openprivacy/bine/tor" "golang.org/x/crypto/ed25519" @@ -39,12 +40,41 @@ func GetTorV3Hostname(pub ed25519.PublicKey) string { return strings.ToLower(serviceID) } + + // IsValidHostname returns true if the given address is a valid onion v3 address func IsValidHostname(address string) bool { - if len(address) == V3HostnameLength { + if len(address) == V3HostnameLength { data, err := base32.StdEncoding.DecodeString(strings.ToUpper(address)) if err == nil { pubkey := data[0:ed25519.PublicKeySize] + + // Tor won't allow us to connect to a hostname containing a torsion component + // However because we permit authentication over inbound connections we would like to + // be extra safe and reject all *invalid* hostnames that contain a torsion component... + + // to do this we need to multiply the point by the order of the group and check that the + // result is the ed25519 identity element. + // l = order of the group (minus 1) + l_bytes := []byte{236, 211, 245, 92, 26, 99, 18, 88, 214, 156, 247, 162, 222, 249, 222, 20, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 16} + l,_ := edwards25519.NewScalar().SetCanonicalBytes(l_bytes) + + // construct a curve point from the public key + // if this fails then the hostname is invalid + p,err := new(edwards25519.Point).SetBytes(pubkey) + if err != nil { + return false + } + + // Calculate l*P (actually (l-1)P + P because of the limitations of the scalar library... + result := new(edwards25519.Point).ScalarMult(l, p) + result = new(edwards25519.Point).Add(result, p) + // The result should be the identity point..assuming the hostname contains no torsion components... + if result.Equal(edwards25519.NewIdentityPoint()) == 0 { + return false + } + + // Finally check that we arrive at the same hostname as the one we were given... if GetTorV3Hostname(ed25519.PublicKey(pubkey)) == address { return true } diff --git a/tor/torUtils_test.go b/tor/torUtils_test.go index e5bc476..0c1437e 100644 --- a/tor/torUtils_test.go +++ b/tor/torUtils_test.go @@ -1,6 +1,9 @@ package tor import ( + "encoding/hex" + "filippo.io/edwards25519" + "git.openprivacy.ca/openprivacy/bine/torutil" "os" "testing" ) @@ -13,6 +16,81 @@ func TestGenerateHashedPassword(t *testing.T) { } } +func TestIsValidHostname(t *testing.T) { + openprivonion := "openpravyvc6spbd4flzn4g2iqu4sxzsizbtb5aqec25t76dnoo5w7yd" + t.Logf("testing: %v", openprivonion) + if IsValidHostname(openprivonion) == false { + t.Fatalf("open privacy onion should validate as a valid hostname") + } + + sarahonion := "icyt7rvdsdci42h6si2ibtwucdmjrlcb2ezkecuagtquiiflbkxf2cqd" + t.Logf("testing: %v", sarahonion) + if IsValidHostname(sarahonion) == false { + t.Fatalf("sarah onion should validate as a valid hostname") + } + + // First we will construct a torsion point from our Valid Onion + pubKey,_ := torutil.PublicKeyFromV3OnionServiceID(openprivonion) + pubKeyPoint,_ := new(edwards25519.Point).SetBytes(pubKey) + torsionPubKeyBytes,_ := hex.DecodeString("26e8958fc2b227b045c3f489f2ef98f0d5dfac05d3c63339b13802886d53fc05") + torsionHostname,_ := torutil.PublicKeyFromV3OnionServiceID(GetTorV3Hostname(torsionPubKeyBytes)) + torsionPoint,_ := new(edwards25519.Point).SetBytes(torsionHostname) + malformedKey := new(edwards25519.Point).Add(pubKeyPoint, torsionPoint) + + t.Logf("testing: %v", GetTorV3Hostname(malformedKey.Bytes())) + if IsValidHostname( GetTorV3Hostname(malformedKey.Bytes())) == true { + t.Fatalf("torsion onion should not validate as a valid hostname") + } + + // Testing a few torsion points taken from https://lists.torproject.org/pipermail/tor-dev/2017-April/012226.html + torsionPubKey,_ := hex.DecodeString("0000000000000000000000000000000000000000000000000000000000000000") + t.Logf("testing: %v", GetTorV3Hostname(torsionPubKey)) + if IsValidHostname( GetTorV3Hostname(torsionPubKey)) == true { + t.Fatalf("torsion onion should not validate as a valid hostname") + } + + torsionPubKey,_ = hex.DecodeString("26e8958fc2b227b045c3f489f2ef98f0d5dfac05d3c63339b13802886d53fc05") + t.Logf("testing: %v", GetTorV3Hostname(torsionPubKey)) + if IsValidHostname( GetTorV3Hostname(torsionPubKey)) == true { + t.Fatalf("torsion onion should not validate as a valid hostname") + } + + torsionPubKey,_ = hex.DecodeString("c9fff3af0471c28e33e98c2043e44f779d0427b1e37c521a6bddc011ed1869af") + t.Logf("testing: %v", GetTorV3Hostname(torsionPubKey)) + if IsValidHostname( GetTorV3Hostname(torsionPubKey)) == true { + t.Fatalf("torsion onion should not validate as a valid hostname") + } + + torsionPubKey,_ = hex.DecodeString("f43e3a046db8749164c6e69b193f1e942c7452e7d888736f40b98093d814d5e7") + t.Logf("testing: %v", GetTorV3Hostname(torsionPubKey)) + if IsValidHostname( GetTorV3Hostname(torsionPubKey)) == true { + t.Fatalf("torsion onion should not should validate as a valid hostname") + } + + torsionPubKey,_ = hex.DecodeString("300ef2e64e588e1df55b48e4da0416ffb64cc85d5b00af6463d5cc6c2b1c185e") + t.Logf("testing: %v", GetTorV3Hostname(torsionPubKey)) + if IsValidHostname( GetTorV3Hostname(torsionPubKey)) == true { + t.Fatalf("torsion onion should not validate as a valid hostname") + } + + + // this should pass + // (also from https://lists.torproject.org/pipermail/tor-dev/2017-April/012230.html) + validPubKey,_ := hex.DecodeString("4ba2e44760dff4c559ef3c38768c1c14a8a54740c782c8d70803e9d6e3ad8794") + t.Logf("testing: %v", GetTorV3Hostname(validPubKey)) + if IsValidHostname( GetTorV3Hostname(validPubKey)) == false { + t.Fatalf("valid onion should validate as a valid hostname") + } + + + // Finally test a completely invalid key... + badPubKey,_ := hex.DecodeString("e19c65de75c68cf3b7643ea732ba9eb1a3d20d6d57ba223c2ece1df66feb5af0") + t.Logf("testing: %v", GetTorV3Hostname(badPubKey)) + if IsValidHostname( GetTorV3Hostname(badPubKey)) == true { + t.Fatalf("invalid ed25519 point should not validate as a valid hostname") + } +} + func TestGenerateTorrc(t *testing.T) { path := "./torrc.test" password := "examplehashedpassword" From 8fe2974aaae2ed32cd0bdca8b0c881917811b2a2 Mon Sep 17 00:00:00 2001 From: Sarah Jamie Lewis Date: Tue, 28 Sep 2021 14:01:42 -0700 Subject: [PATCH 2/3] Go conventions --- tor/torUtils.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tor/torUtils.go b/tor/torUtils.go index 8b194a8..4118d38 100644 --- a/tor/torUtils.go +++ b/tor/torUtils.go @@ -56,8 +56,8 @@ func IsValidHostname(address string) bool { // to do this we need to multiply the point by the order of the group and check that the // result is the ed25519 identity element. // l = order of the group (minus 1) - l_bytes := []byte{236, 211, 245, 92, 26, 99, 18, 88, 214, 156, 247, 162, 222, 249, 222, 20, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 16} - l,_ := edwards25519.NewScalar().SetCanonicalBytes(l_bytes) + lBytes := []byte{236, 211, 245, 92, 26, 99, 18, 88, 214, 156, 247, 162, 222, 249, 222, 20, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 16} + l,_ := edwards25519.NewScalar().SetCanonicalBytes(lBytes) // construct a curve point from the public key // if this fails then the hostname is invalid From b36f6dc33f71e3b8a035e0ae16fb6cf2288798fc Mon Sep 17 00:00:00 2001 From: Sarah Jamie Lewis Date: Tue, 28 Sep 2021 14:03:15 -0700 Subject: [PATCH 3/3] Format --- tor/torUtils.go | 8 +++----- tor/torUtils_test.go | 42 ++++++++++++++++++++---------------------- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/tor/torUtils.go b/tor/torUtils.go index 4118d38..e210be4 100644 --- a/tor/torUtils.go +++ b/tor/torUtils.go @@ -40,11 +40,9 @@ func GetTorV3Hostname(pub ed25519.PublicKey) string { return strings.ToLower(serviceID) } - - // IsValidHostname returns true if the given address is a valid onion v3 address func IsValidHostname(address string) bool { - if len(address) == V3HostnameLength { + if len(address) == V3HostnameLength { data, err := base32.StdEncoding.DecodeString(strings.ToUpper(address)) if err == nil { pubkey := data[0:ed25519.PublicKeySize] @@ -57,11 +55,11 @@ func IsValidHostname(address string) bool { // result is the ed25519 identity element. // l = order of the group (minus 1) lBytes := []byte{236, 211, 245, 92, 26, 99, 18, 88, 214, 156, 247, 162, 222, 249, 222, 20, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 16} - l,_ := edwards25519.NewScalar().SetCanonicalBytes(lBytes) + l, _ := edwards25519.NewScalar().SetCanonicalBytes(lBytes) // construct a curve point from the public key // if this fails then the hostname is invalid - p,err := new(edwards25519.Point).SetBytes(pubkey) + p, err := new(edwards25519.Point).SetBytes(pubkey) if err != nil { return false } diff --git a/tor/torUtils_test.go b/tor/torUtils_test.go index 0c1437e..0416c2a 100644 --- a/tor/torUtils_test.go +++ b/tor/torUtils_test.go @@ -30,63 +30,61 @@ func TestIsValidHostname(t *testing.T) { } // First we will construct a torsion point from our Valid Onion - pubKey,_ := torutil.PublicKeyFromV3OnionServiceID(openprivonion) - pubKeyPoint,_ := new(edwards25519.Point).SetBytes(pubKey) - torsionPubKeyBytes,_ := hex.DecodeString("26e8958fc2b227b045c3f489f2ef98f0d5dfac05d3c63339b13802886d53fc05") - torsionHostname,_ := torutil.PublicKeyFromV3OnionServiceID(GetTorV3Hostname(torsionPubKeyBytes)) - torsionPoint,_ := new(edwards25519.Point).SetBytes(torsionHostname) + pubKey, _ := torutil.PublicKeyFromV3OnionServiceID(openprivonion) + pubKeyPoint, _ := new(edwards25519.Point).SetBytes(pubKey) + torsionPubKeyBytes, _ := hex.DecodeString("26e8958fc2b227b045c3f489f2ef98f0d5dfac05d3c63339b13802886d53fc05") + torsionHostname, _ := torutil.PublicKeyFromV3OnionServiceID(GetTorV3Hostname(torsionPubKeyBytes)) + torsionPoint, _ := new(edwards25519.Point).SetBytes(torsionHostname) malformedKey := new(edwards25519.Point).Add(pubKeyPoint, torsionPoint) t.Logf("testing: %v", GetTorV3Hostname(malformedKey.Bytes())) - if IsValidHostname( GetTorV3Hostname(malformedKey.Bytes())) == true { + if IsValidHostname(GetTorV3Hostname(malformedKey.Bytes())) == true { t.Fatalf("torsion onion should not validate as a valid hostname") } // Testing a few torsion points taken from https://lists.torproject.org/pipermail/tor-dev/2017-April/012226.html - torsionPubKey,_ := hex.DecodeString("0000000000000000000000000000000000000000000000000000000000000000") + torsionPubKey, _ := hex.DecodeString("0000000000000000000000000000000000000000000000000000000000000000") t.Logf("testing: %v", GetTorV3Hostname(torsionPubKey)) - if IsValidHostname( GetTorV3Hostname(torsionPubKey)) == true { + if IsValidHostname(GetTorV3Hostname(torsionPubKey)) == true { t.Fatalf("torsion onion should not validate as a valid hostname") } - torsionPubKey,_ = hex.DecodeString("26e8958fc2b227b045c3f489f2ef98f0d5dfac05d3c63339b13802886d53fc05") + torsionPubKey, _ = hex.DecodeString("26e8958fc2b227b045c3f489f2ef98f0d5dfac05d3c63339b13802886d53fc05") t.Logf("testing: %v", GetTorV3Hostname(torsionPubKey)) - if IsValidHostname( GetTorV3Hostname(torsionPubKey)) == true { + if IsValidHostname(GetTorV3Hostname(torsionPubKey)) == true { t.Fatalf("torsion onion should not validate as a valid hostname") } - torsionPubKey,_ = hex.DecodeString("c9fff3af0471c28e33e98c2043e44f779d0427b1e37c521a6bddc011ed1869af") + torsionPubKey, _ = hex.DecodeString("c9fff3af0471c28e33e98c2043e44f779d0427b1e37c521a6bddc011ed1869af") t.Logf("testing: %v", GetTorV3Hostname(torsionPubKey)) - if IsValidHostname( GetTorV3Hostname(torsionPubKey)) == true { + if IsValidHostname(GetTorV3Hostname(torsionPubKey)) == true { t.Fatalf("torsion onion should not validate as a valid hostname") } - torsionPubKey,_ = hex.DecodeString("f43e3a046db8749164c6e69b193f1e942c7452e7d888736f40b98093d814d5e7") + torsionPubKey, _ = hex.DecodeString("f43e3a046db8749164c6e69b193f1e942c7452e7d888736f40b98093d814d5e7") t.Logf("testing: %v", GetTorV3Hostname(torsionPubKey)) - if IsValidHostname( GetTorV3Hostname(torsionPubKey)) == true { + if IsValidHostname(GetTorV3Hostname(torsionPubKey)) == true { t.Fatalf("torsion onion should not should validate as a valid hostname") } - torsionPubKey,_ = hex.DecodeString("300ef2e64e588e1df55b48e4da0416ffb64cc85d5b00af6463d5cc6c2b1c185e") + torsionPubKey, _ = hex.DecodeString("300ef2e64e588e1df55b48e4da0416ffb64cc85d5b00af6463d5cc6c2b1c185e") t.Logf("testing: %v", GetTorV3Hostname(torsionPubKey)) - if IsValidHostname( GetTorV3Hostname(torsionPubKey)) == true { + if IsValidHostname(GetTorV3Hostname(torsionPubKey)) == true { t.Fatalf("torsion onion should not validate as a valid hostname") } - // this should pass // (also from https://lists.torproject.org/pipermail/tor-dev/2017-April/012230.html) - validPubKey,_ := hex.DecodeString("4ba2e44760dff4c559ef3c38768c1c14a8a54740c782c8d70803e9d6e3ad8794") + validPubKey, _ := hex.DecodeString("4ba2e44760dff4c559ef3c38768c1c14a8a54740c782c8d70803e9d6e3ad8794") t.Logf("testing: %v", GetTorV3Hostname(validPubKey)) - if IsValidHostname( GetTorV3Hostname(validPubKey)) == false { + if IsValidHostname(GetTorV3Hostname(validPubKey)) == false { t.Fatalf("valid onion should validate as a valid hostname") } - // Finally test a completely invalid key... - badPubKey,_ := hex.DecodeString("e19c65de75c68cf3b7643ea732ba9eb1a3d20d6d57ba223c2ece1df66feb5af0") + badPubKey, _ := hex.DecodeString("e19c65de75c68cf3b7643ea732ba9eb1a3d20d6d57ba223c2ece1df66feb5af0") t.Logf("testing: %v", GetTorV3Hostname(badPubKey)) - if IsValidHostname( GetTorV3Hostname(badPubKey)) == true { + if IsValidHostname(GetTorV3Hostname(badPubKey)) == true { t.Fatalf("invalid ed25519 point should not validate as a valid hostname") } }