x/crypto/bcrypt: base64 decode - incorrect padding calculation

[BenJoParadise]

What version of Go are you using (go version)?

$ go version
go version go1.13.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/bparadise/Library/Caches/go-build"
GOENV="/Users/bparadise/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/bparadise/dev/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/bparadise/dev/go/src/github.com/demisto/server/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/dw/42b9g2ds6_552shlyt5kym0jqhh9d5/T/go-build594486857=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Test bcrypt base64 decoding function with inputs of different lengths, generated using the corresponding encoding function from the same file

What did you expect to see?

Successful decoding of all valid base64-encoded inputs

What did you see instead?

Decoding errors

Additional Details

Description

When calculating padding =s, the calculation returns 4 when it should return 0.
This edge case is never encountered in bcrypt, since the inputs are always the same length (which is not divisible by 4).

Reference

This is the existing code (https://github.com/golang/crypto/blob/ac88ee75c92c889b97e05591e9a39b6480c538b3/bcrypt/base64.go#L24)

	numOfEquals := 4 - (len(src) % 4)

Suggested fix

	numOfEquals := (4 - (len(src) % 4)) % 4

[toothrot]

/cc @FiloSottile @katiehockman

[FiloSottile]

This only affects the internal API, correct? No external inputs can cause the issue?

[BenJoParadise]

Correct.

[gopherbot]

Change https://golang.org/cl/356469 mentions this issue: bcrypt: use base64.NoPadding

[AlexanderYastrebov]

@FiloSottile https://golang.org/cl/356469 is ready for review