Skip to content

Commit

Permalink
Allocation optimization (golang-jwt#33)
Browse files Browse the repository at this point in the history
* Test to ensure ECDSA signature is valid

Add assertions to ensure ECDSA signing methods return valid signatures.

This is probably covered elsewhere as well, but putting it in
ecdsa_test.go makes it more obvious and easier to find.

* Benchmark ECDSA signing methods

Add benchmark coverage of ECDSA signing methods.

Benchmarks are run using the existing helper for comparability with
existing benchmarks.

Sign method is also tested directly, to avoid the overhead of *Token.

Report allocations for all benchmarks.

Allocation count for ES384 and ES512 fluctuate across test runs,
other signing methods consistently report the same number of allocations.

Sample output:
```
$ go test -bench=Bench -run=NONE .
2021/02/26 18:18:30 Listening...
goos: darwin
goarch: amd64
pkg: github.com/dgrijalva/jwt-go
BenchmarkECDSASigning/Basic_ES256-8                     190572      6702 ns/op    4249 B/op      65 allocs/op
BenchmarkECDSASigning/Basic_ES256/sign-only-8            47383     24650 ns/op    3329 B/op      43 allocs/op
BenchmarkECDSASigning/Basic_ES384-8                       1113   1252975 ns/op 1750744 B/op   14474 allocs/op
BenchmarkECDSASigning/Basic_ES384/sign-only-8              286   3937773 ns/op 1746175 B/op   14423 allocs/op
BenchmarkECDSASigning/Basic_ES512-8                        662   1949937 ns/op 3028386 B/op   19608 allocs/op
BenchmarkECDSASigning/Basic_ES512/sign-only-8              170   6856189 ns/op 3025471 B/op   19571 allocs/op
BenchmarkECDSASigning/basic_ES256_invalid:_foo_=>_bar-8 190638      6665 ns/op    4249 B/op      65 allocs/op
BenchmarkHS256Signing-8                                1000000      1024 ns/op    1584 B/op      32 allocs/op
BenchmarkHS384Signing-8                                 917286      1447 ns/op    1969 B/op      32 allocs/op
BenchmarkHS512Signing-8                                 827744      1470 ns/op    2065 B/op      32 allocs/op
BenchmarkRS256Signing-8                                   3037    390077 ns/op   32576 B/op     136 allocs/op
BenchmarkRS384Signing-8                                   2976    379155 ns/op   32684 B/op     136 allocs/op
BenchmarkRS512Signing-8                                   3205    388628 ns/op   32704 B/op     136 allocs/op
```

* Reduce allocations during ECDSA signing

Reduce the number of byte arrays allocated by using big.Int.FillBytes
when calculating ECDSA signature.

After this change, Benchmarks of ES256 signing method consistently
report 4 fewer allocations.

Before:
```
BenchmarkECDSASigning/Basic_ES256-8              190572         6702 ns/op       4249 B/op         65 allocs/op
BenchmarkECDSASigning/Basic_ES256/sign-only-8     47383        24650 ns/op       3329 B/op         43 allocs/op
```

After:
```
BenchmarkECDSASigning/Basic_ES256-8              187682         6725 ns/op       4121 B/op         61 allocs/op
BenchmarkECDSASigning/Basic_ES256/sign-only-8     48656        24446 ns/op       3201 B/op         39 allocs/op
```

* Use base64.RawURLEncoding to avoid padding

JWT uses a non-padded base64 encoding.

Current code uses base64.URLEncoding to generate a padded string and
then removes the padding.
Likewise, current code adds padding before decoding.

Instead, use base64.RawURLEncoding which does not add or require the
padding in the first place.

In addition to making the code cleaner, this reduces memory allocations
as reported by benchmarks.

Before:
```
BenchmarkECDSASigning/Basic_ES256-8                     191396         6917 ns/op       4121 B/op         61 allocs/op
BenchmarkECDSASigning/Basic_ES256/sign-only-8            49347        25039 ns/op       3201 B/op         39 allocs/op
BenchmarkECDSASigning/basic_ES256_invalid:_foo_=>_bar-8 190668         6586 ns/op       4121 B/op         61 allocs/op
BenchmarkHS256Signing-8                                1260060         1131 ns/op       1585 B/op         32 allocs/op
BenchmarkHS384Signing-8                                 861378         1387 ns/op       1969 B/op         32 allocs/op
BenchmarkHS512Signing-8                                 896745         1463 ns/op       2065 B/op         32 allocs/op
BenchmarkRS256Signing-8                                   3086       355769 ns/op      32576 B/op        136 allocs/op
BenchmarkRS384Signing-8                                   3414       353570 ns/op      32694 B/op        136 allocs/op
BenchmarkRS512Signing-8                                   3235       349394 ns/op      32706 B/op        136 allocs/op
```

After:
```
BenchmarkECDSASigning/Basic_ES256-8                     176617         6827 ns/op       4021 B/op         58 allocs/op
BenchmarkECDSASigning/Basic_ES256/sign-only-8            48038        24213 ns/op       3169 B/op         38 allocs/op
BenchmarkECDSASigning/basic_ES256_invalid:_foo_=>_bar-8 194352         6928 ns/op       4021 B/op         58 allocs/op
BenchmarkHS256Signing-8                                1000000         1127 ns/op       1488 B/op         29 allocs/op
BenchmarkHS384Signing-8                                 972552         1369 ns/op       1873 B/op         29 allocs/op
BenchmarkHS512Signing-8                                 780751         1368 ns/op       1969 B/op         29 allocs/op
BenchmarkRS256Signing-8                                   3014       387326 ns/op      32475 B/op        133 allocs/op
BenchmarkRS384Signing-8                                   3044       361411 ns/op      32591 B/op        133 allocs/op
BenchmarkRS512Signing-8                                   3273       355504 ns/op      32607 B/op        133 allocs/op
```

Benchmarks of signing methods ES384 and ES512 are omitted because their
allocations are not consistent.
  • Loading branch information
jkanywhere authored Jul 13, 2021
1 parent 3008b2b commit 860640e
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 19 deletions.
18 changes: 6 additions & 12 deletions ecdsa.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,18 +128,12 @@ func (m *SigningMethodECDSA) Sign(signingString string, key interface{}) (string
keyBytes += 1
}

// We serialize the outpus (r and s) into big-endian byte arrays and pad
// them with zeros on the left to make sure the sizes work out. Both arrays
// must be keyBytes long, and the output must be 2*keyBytes long.
rBytes := r.Bytes()
rBytesPadded := make([]byte, keyBytes)
copy(rBytesPadded[keyBytes-len(rBytes):], rBytes)

sBytes := s.Bytes()
sBytesPadded := make([]byte, keyBytes)
copy(sBytesPadded[keyBytes-len(sBytes):], sBytes)

out := append(rBytesPadded, sBytesPadded...)
// We serialize the outputs (r and s) into big-endian byte arrays
// padded with zeros on the left to make sure the sizes work out.
// Output must be 2*keyBytes long.
out := make([]byte, 2*keyBytes)
r.FillBytes(out[0:keyBytes]) // r is assigned to the first half of output.
s.FillBytes(out[keyBytes:]) // s is assigned to the second half of output.

return EncodeSegment(out), nil
} else {
Expand Down
48 changes: 47 additions & 1 deletion ecdsa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,60 @@ func TestECDSASign(t *testing.T) {

if data.valid {
parts := strings.Split(data.tokenString, ".")
toSign := strings.Join(parts[0:2], ".")
method := jwt.GetSigningMethod(data.alg)
sig, err := method.Sign(strings.Join(parts[0:2], "."), ecdsaKey)
sig, err := method.Sign(toSign, ecdsaKey)

if err != nil {
t.Errorf("[%v] Error signing token: %v", data.name, err)
}
if sig == parts[2] {
t.Errorf("[%v] Identical signatures\nbefore:\n%v\nafter:\n%v", data.name, parts[2], sig)
}

err = method.Verify(toSign, sig, ecdsaKey.Public())
if err != nil {
t.Errorf("[%v] Sign produced an invalid signature: %v", data.name, err)
}
}
}
}

func BenchmarkECDSASigning(b *testing.B) {
for _, data := range ecdsaTestData {
key, _ := ioutil.ReadFile(data.keys["private"])

ecdsaKey, err := jwt.ParseECPrivateKeyFromPEM(key)
if err != nil {
b.Fatalf("Unable to parse ECDSA private key: %v", err)
}

method := jwt.GetSigningMethod(data.alg)

b.Run(data.name, func(b *testing.B) {
benchmarkSigning(b, method, ecdsaKey)
})

// Directly call method.Sign without the decoration of *Token.
b.Run(data.name+"/sign-only", func(b *testing.B) {
if !data.valid {
b.Skipf("Skipping because data is not valid")
}

parts := strings.Split(data.tokenString, ".")
toSign := strings.Join(parts[0:2], ".")

b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
sig, err := method.Sign(toSign, ecdsaKey)
if err != nil {
b.Fatalf("[%v] Error signing token: %v", data.name, err)
}
if sig == parts[2] {
b.Fatalf("[%v] Identical signatures\nbefore:\n%v\nafter:\n%v", data.name, parts[2], sig)
}
}
})
}
}
2 changes: 2 additions & 0 deletions parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,8 @@ func TestParser_ParseUnverified(t *testing.T) {
// Helper method for benchmarking various methods
func benchmarkSigning(b *testing.B, method jwt.SigningMethod, key interface{}) {
t := jwt.New(method)
b.ReportAllocs()
b.ResetTimer()
b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
if _, err := t.SignedString(key); err != nil {
Expand Down
8 changes: 2 additions & 6 deletions token.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,10 @@ func ParseWithClaims(tokenString string, claims Claims, keyFunc Keyfunc) (*Token

// Encode JWT specific base64url encoding with padding stripped
func EncodeSegment(seg []byte) string {
return strings.TrimRight(base64.URLEncoding.EncodeToString(seg), "=")
return base64.RawURLEncoding.EncodeToString(seg)
}

// Decode JWT specific base64url encoding with padding stripped
func DecodeSegment(seg string) ([]byte, error) {
if l := len(seg) % 4; l > 0 {
seg += strings.Repeat("=", 4-l)
}

return base64.URLEncoding.DecodeString(seg)
return base64.RawURLEncoding.DecodeString(seg)
}

0 comments on commit 860640e

Please sign in to comment.