From 63e0b93db47d43059f4e4bcf4361b94d1c215c5c Mon Sep 17 00:00:00 2001 From: "Lawrence, Rendall" Date: Sun, 19 Mar 2023 20:13:44 +0300 Subject: [PATCH 1/3] remove `randseed` package --- cmd/mochi/config.go | 3 --- frontend/udp/connection_id.go | 17 ++++++++++---- frontend/udp/connection_id_test.go | 1 - frontend/udp/frontend_test.go | 1 - middleware/jwt/jwt_test.go | 1 - middleware/varinterval/varinterval_test.go | 2 -- pkg/randseed/rand_seed.go | 27 ---------------------- storage/test/storage_bench.go | 2 -- 8 files changed, 12 insertions(+), 42 deletions(-) delete mode 100644 pkg/randseed/rand_seed.go diff --git a/cmd/mochi/config.go b/cmd/mochi/config.go index d51ebaf..aac39bd 100644 --- a/cmd/mochi/config.go +++ b/cmd/mochi/config.go @@ -11,9 +11,6 @@ import ( fu "github.com/sot-tech/mochi/frontend/udp" "github.com/sot-tech/mochi/pkg/conf" - // Seed math random - _ "github.com/sot-tech/mochi/pkg/randseed" - // Imports to register middleware hooks. _ "github.com/sot-tech/mochi/middleware/clientapproval" _ "github.com/sot-tech/mochi/middleware/jwt" diff --git a/frontend/udp/connection_id.go b/frontend/udp/connection_id.go index 6892477..672fc1d 100644 --- a/frontend/udp/connection_id.go +++ b/frontend/udp/connection_id.go @@ -2,9 +2,9 @@ package udp import ( "crypto/hmac" + cr "crypto/rand" "encoding/binary" "hash" - "math/rand" "net/netip" "time" @@ -70,18 +70,25 @@ func NewConnectionIDGenerator(key []byte, maxClockSkew time.Duration) *Connectio buff: make([]byte, buffLen), scratch: make([]byte, scratchLen), maxClockSkew: int64(maxClockSkew), - s: rand.Uint64(), } } // reset resets the generator. // This is called by other methods of the generator, it's not necessary to call // it after getting a generator from a pool. -func (g *ConnectionIDGenerator) reset() { +func (g *ConnectionIDGenerator) reset(init bool) { g.mac.Reset() g.connID = g.connID[:connIDLen] g.buff = g.buff[:buffLen] g.scratch = g.scratch[:0] + if init { + r := make([]byte, 8) + if _, err := cr.Read(r); err == nil { + g.s = binary.BigEndian.Uint64(r) + } else { + g.s = uint64(time.Now().UnixNano()) + } + } } // Generate generates an 8-byte connection ID as described in BEP 15 for the @@ -102,7 +109,7 @@ func (g *ConnectionIDGenerator) reset() { // will be reused, so it must not be referenced after returning the generator // to a pool and will be overwritten be subsequent calls to Generate! func (g *ConnectionIDGenerator) Generate(ip netip.Addr, now time.Time) (out []byte) { - g.reset() + g.reset(true) var r uint64 r, g.s = xorshift.XorShift64S(g.s) g.buff[0] = byte(r) @@ -123,7 +130,7 @@ func (g *ConnectionIDGenerator) Generate(ip netip.Addr, now time.Time) (out []by // Validate validates the given connection ID for an IP and the current time. func (g *ConnectionIDGenerator) Validate(connectionID []byte, ip netip.Addr, now time.Time) bool { - g.reset() + g.reset(false) nowTS := now.Unix() g.buff[0] = connectionID[0] // connectionID contains only 2 bytes of timestamp, so we clean little 16 bits to place it and rehash. diff --git a/frontend/udp/connection_id_test.go b/frontend/udp/connection_id_test.go index 8b38b05..dae1ef9 100644 --- a/frontend/udp/connection_id_test.go +++ b/frontend/udp/connection_id_test.go @@ -13,7 +13,6 @@ import ( "github.com/cespare/xxhash/v2" "github.com/sot-tech/mochi/pkg/log" - _ "github.com/sot-tech/mochi/pkg/randseed" "github.com/stretchr/testify/require" ) diff --git a/frontend/udp/frontend_test.go b/frontend/udp/frontend_test.go index 0313e9b..dda2955 100644 --- a/frontend/udp/frontend_test.go +++ b/frontend/udp/frontend_test.go @@ -7,7 +7,6 @@ import ( "github.com/sot-tech/mochi/middleware" "github.com/sot-tech/mochi/pkg/conf" "github.com/sot-tech/mochi/pkg/log" - _ "github.com/sot-tech/mochi/pkg/randseed" "github.com/sot-tech/mochi/storage" _ "github.com/sot-tech/mochi/storage/memory" ) diff --git a/middleware/jwt/jwt_test.go b/middleware/jwt/jwt_test.go index f54a94d..35ce4ee 100644 --- a/middleware/jwt/jwt_test.go +++ b/middleware/jwt/jwt_test.go @@ -22,7 +22,6 @@ import ( "github.com/sot-tech/mochi/bittorrent" "github.com/sot-tech/mochi/pkg/conf" "github.com/sot-tech/mochi/pkg/log" - _ "github.com/sot-tech/mochi/pkg/randseed" ) const ( diff --git a/middleware/varinterval/varinterval_test.go b/middleware/varinterval/varinterval_test.go index c792485..f8c298e 100644 --- a/middleware/varinterval/varinterval_test.go +++ b/middleware/varinterval/varinterval_test.go @@ -9,8 +9,6 @@ import ( "github.com/sot-tech/mochi/bittorrent" "github.com/sot-tech/mochi/pkg/conf" - - _ "github.com/sot-tech/mochi/pkg/randseed" ) var configTests = []struct { diff --git a/pkg/randseed/rand_seed.go b/pkg/randseed/rand_seed.go deleted file mode 100644 index 8807832..0000000 --- a/pkg/randseed/rand_seed.go +++ /dev/null @@ -1,27 +0,0 @@ -// Package randseed just seeds (math) rand.Rand -package randseed - -import ( - cr "crypto/rand" - "math/rand" - "time" -) - -func init() { - // Seeding global math random - //nolint:staticcheck - rand.Seed(GenSeed()) -} - -// GenSeed returns 64bit seed from crypto/rand source or -// from current time, if crypto random read error occurred -func GenSeed() (seed int64) { - r := make([]byte, 8) - if _, err := cr.Read(r); err == nil { - seed = int64(r[0])<<56 | int64(r[1])<<48 | int64(r[2])<<40 | int64(r[3])<<32 | - int64(r[4])<<24 | int64(r[5])<<16 | int64(r[6])<<8 | int64(r[7]) - } else { - seed = time.Now().UnixNano() - } - return -} diff --git a/storage/test/storage_bench.go b/storage/test/storage_bench.go index b86c6f7..edded0e 100644 --- a/storage/test/storage_bench.go +++ b/storage/test/storage_bench.go @@ -13,8 +13,6 @@ import ( "testing" "github.com/sot-tech/mochi/bittorrent" - // used for seeding global math.Rand - _ "github.com/sot-tech/mochi/pkg/randseed" "github.com/sot-tech/mochi/storage" ) From 21c600e92179063ef60b4eaac1e54c6e7e15a640 Mon Sep 17 00:00:00 2001 From: "Lawrence, Rendall" Date: Sun, 19 Mar 2023 20:19:55 +0300 Subject: [PATCH 2/3] add verbose to linter --- .github/workflows/lint.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 1f077d2..3e8893b 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -31,6 +31,7 @@ jobs: - uses: "golangci/golangci-lint-action@v3" with: version: "latest" + args: -v codeql: name: "Analyze with CodeQL" From 869a594aa386a95d81acafc1b20632f8c6550368 Mon Sep 17 00:00:00 2001 From: "Lawrence, Rendall" Date: Sun, 19 Mar 2023 20:33:07 +0300 Subject: [PATCH 3/3] fix lint warnings --- .github/workflows/lint.yaml | 1 - frontend/http/params_test.go | 24 +++++++----------------- frontend/udp/params_test.go | 4 +++- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 3e8893b..1f077d2 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -31,7 +31,6 @@ jobs: - uses: "golangci/golangci-lint-action@v3" with: version: "latest" - args: -v codeql: name: "Analyze with CodeQL" diff --git a/frontend/http/params_test.go b/frontend/http/params_test.go index 9830fb7..d99df78 100644 --- a/frontend/http/params_test.go +++ b/frontend/http/params_test.go @@ -68,21 +68,18 @@ func mapArrayEqual(boxed map[string][]string, unboxed *queryParams) (bool, strin // Also note that any error that is encountered during parsing is returned as a // ClientError, as this method is expected to be used to parse client-provided // data. -func parseURLData(urlData []byte) (*queryParams, error) { +func parseURLData(urlData []byte) *queryParams { i := bytes.IndexByte(urlData, '?') if i >= 0 { urlData = urlData[i+1:] } q := &queryParams{new(fasthttp.Args)} q.ParseBytes(urlData) - return q, nil + return q } func TestParseEmptyURLData(t *testing.T) { - parsedQuery, err := parseURLData(nil) - if err != nil { - t.Fatal(err) - } + parsedQuery := parseURLData(nil) if parsedQuery == nil { t.Fatal("Parsed query must not be nil") } @@ -90,10 +87,7 @@ func TestParseEmptyURLData(t *testing.T) { func TestParseValidURLData(t *testing.T) { for parseIndex, parseVal := range ValidAnnounceArguments { - parsedQueryObj, err := parseURLData([]byte("/announce?" + parseVal.Encode())) - if err != nil { - t.Fatal(err) - } + parsedQueryObj := parseURLData([]byte("/announce?" + parseVal.Encode())) if eq, exp := mapArrayEqual(parseVal, parsedQueryObj); !eq { t.Fatalf("Incorrect parse at item %d.\n Expected=%v\n Received=%v\n", parseIndex, parseVal, exp) @@ -101,9 +95,9 @@ func TestParseValidURLData(t *testing.T) { } } -func TestParseShouldNotPanicURLData(t *testing.T) { +func TestParseShouldNotPanicURLData(_ *testing.T) { for _, parseStr := range shouldNotPanicQueries { - _, _ = parseURLData(parseStr) + _ = parseURLData(parseStr) } } @@ -115,11 +109,7 @@ func BenchmarkParseQuery(b *testing.B) { b.ResetTimer() for bCount := 0; bCount < b.N; bCount++ { i := bCount % len(announceStrings) - parsedQueryObj, err := parseURLData(announceStrings[i]) - if err != nil { - b.Error(err, i) - b.Log(parsedQueryObj) - } + _ = parseURLData(announceStrings[i]) } } diff --git a/frontend/udp/params_test.go b/frontend/udp/params_test.go index 266d6f5..89e9a93 100644 --- a/frontend/udp/params_test.go +++ b/frontend/udp/params_test.go @@ -88,7 +88,9 @@ func TestParseInvalidURLData(t *testing.T) { func TestParseShouldNotPanicURLData(t *testing.T) { for _, parseStr := range shouldNotPanicQueries { - _, _ = parseQuery(parseStr) + if _, err := parseQuery(parseStr); err != nil { + t.Error(err) + } } }