Go: crypto, x/crypto: use math.bits rotate functions instead of ad-hoc implementations

Created on 13 Apr 2019  ·  11Comments  ·  Source: golang/go

NeedsInvestigation

Most helpful comment

The following query finds a number of additional cases in /x/crypto when run with GOARCH=arm64 to scan the non-amd64-assembly versions.

gogrep -x       '$a >> $k | $a << (64 - $k)' ./...
gogrep -x       '($a >> $k) | ($a << (64 - $k))' ./...
gogrep -x       '$a << $k | $a >> (64 - $k)' ./...
gogrep -x       '($a << $k) | ($a >> (64 - $k))' ./...
gogrep -x       '$a >> (64 - $k) | $a << $k' ./...
gogrep -x       '($a >> (64 - $k)) | ($a<< $k)' ./...
gogrep -x       '$a << (64 - $k) | $a >> $k' ./...
gogrep -x       '($a << (64 - $k)) | ($a >> $k)' ./...
gogrep -x       '$a >> $k | $a << (32 - $k)' ./...
gogrep -x       '($a >> $k) | ($a << (32 - $k))' ./...
gogrep -x       '$a << $k | $a >> (32 - $k)' ./...
gogrep -x       '($a << $k) | ($a >> (32 - $k))' ./...
gogrep -x       '$a >> (32 - $k) | $a << $k' ./...
gogrep -x       '($a >> (32 - $k)) | ($a<< $k)' ./...
gogrep -x       '$a << (32 - $k) | $a >> $k' ./...
gogrep -x       '($a << (32 - $k)) | ($a >> $k)' ./...

Edit:
And a 16-bit one in my rc2 code :D

golang.org/x/crypto/pkcs12/internal/rc2/rc2.go:84:9: (x >> (16 - b)) | (x << b)

All 11 comments

Change https://golang.org/cl/171729 mentions this issue: crypto/sha1: use math/bits.RotateLeft32 instead of ad-hoc implementation.

Change https://golang.org/cl/171731 mentions this issue: crypto/sha256: Use bits.RotateLeft32 instead of ad-hoc implementation

Change https://golang.org/cl/171736 mentions this issue: crypto/sha512: use math/bits.RotateLeft64 instead of ad-hoc implementation.

@FiloSottile

The following query finds a number of additional cases in /x/crypto when run with GOARCH=arm64 to scan the non-amd64-assembly versions.

gogrep -x       '$a >> $k | $a << (64 - $k)' ./...
gogrep -x       '($a >> $k) | ($a << (64 - $k))' ./...
gogrep -x       '$a << $k | $a >> (64 - $k)' ./...
gogrep -x       '($a << $k) | ($a >> (64 - $k))' ./...
gogrep -x       '$a >> (64 - $k) | $a << $k' ./...
gogrep -x       '($a >> (64 - $k)) | ($a<< $k)' ./...
gogrep -x       '$a << (64 - $k) | $a >> $k' ./...
gogrep -x       '($a << (64 - $k)) | ($a >> $k)' ./...
gogrep -x       '$a >> $k | $a << (32 - $k)' ./...
gogrep -x       '($a >> $k) | ($a << (32 - $k))' ./...
gogrep -x       '$a << $k | $a >> (32 - $k)' ./...
gogrep -x       '($a << $k) | ($a >> (32 - $k))' ./...
gogrep -x       '$a >> (32 - $k) | $a << $k' ./...
gogrep -x       '($a >> (32 - $k)) | ($a<< $k)' ./...
gogrep -x       '$a << (32 - $k) | $a >> $k' ./...
gogrep -x       '($a << (32 - $k)) | ($a >> $k)' ./...

Edit:
And a 16-bit one in my rc2 code :D

golang.org/x/crypto/pkcs12/internal/rc2/rc2.go:84:9: (x >> (16 - b)) | (x << b)

Probably we can close this one as soon as the #171731 would be merged.

➜  src git:(master) find crypto -type f -name "*.go" -exec egrep ".+>>(\d)+ \| .+<<\((\d)+-(\d)+\)" {} \+
crypto/sha256/sha256block.go: t1 := (v1>>17 | v1<<(32-17)) ^ (v1>>19 | v1<<(32-19)) ^ (v1 >> 10)
crypto/sha256/sha256block.go: t2 := (v2>>7 | v2<<(32-7)) ^ (v2>>18 | v2<<(32-18)) ^ (v2 >> 3)
crypto/sha256/sha256block.go: t1 := h + ((e>>6 | e<<(32-6)) ^ (e>>11 | e<<(32-11)) ^ (e>>25 | e<<(32-25))) + ((e & f) ^ (^e & g)) + _K[i] + w[i]
crypto/sha256/sha256block.go: t2 := ((a>>2 | a<<(32-2)) ^ (a>>13 | a<<(32-13)) ^ (a>>22 | a<<(32-22))) + ((a & b) ^ (a & c) ^ (b & c))

@FiloSottile

Probably we can close this one as soon as the #171731 would be merged.

➜  src git:(master) find crypto -type f -name "*.go" -exec egrep ".+>>(\d)+ \| .+<<\((\d)+-(\d)+\)" {} \+
crypto/sha256/sha256block.go: t1 := (v1>>17 | v1<<(32-17)) ^ (v1>>19 | v1<<(32-19)) ^ (v1 >> 10)
crypto/sha256/sha256block.go: t2 := (v2>>7 | v2<<(32-7)) ^ (v2>>18 | v2<<(32-18)) ^ (v2 >> 3)
crypto/sha256/sha256block.go: t1 := h + ((e>>6 | e<<(32-6)) ^ (e>>11 | e<<(32-11)) ^ (e>>25 | e<<(32-25))) + ((e & f) ^ (^e & g)) + _K[i] + w[i]
crypto/sha256/sha256block.go: t2 := ((a>>2 | a<<(32-2)) ^ (a>>13 | a<<(32-13)) ^ (a>>22 | a<<(32-22))) + ((a & b) ^ (a & c) ^ (b & c))

@FiloSottile

There is code in x/crypto that contains bit rotation operations. Should it be changed in case of this particular issue?

I would be happy to take two global CLs, one for crypto/... and one for x/crypto/..., fixing all cases identified by gogrep according to https://github.com/golang/go/issues/31456#issuecomment-483104913.

Change https://golang.org/cl/173277 mentions this issue: blake2b: use math.bits rotate functions instead of ad-hoc implementations

Change https://golang.org/cl/173278 mentions this issue: blake2s: use math.bits rotate functions instead of ad-hoc implementation

Change https://golang.org/cl/174137 mentions this issue: scrypt: use math.bits rotate functions instead of ad-hoc implementation

Was this page helpful?
0 / 5 - 0 ratings