Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto/rsa: can generate digital signature when hash algorithm (digest method) is set to MD5 in FIPS mode #45565

Open
ijajmulani opened this issue Apr 14, 2021 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@ijajmulani
Copy link

ijajmulani commented Apr 14, 2021

I am able to generate signature with md5 hash algorithm in FIPS mode. According to FIPS 140-2 md5 should not be use for digital signature.

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

$ go version
go version go1.15.4 linux/amd64


Does this issue reproduce with the latest release?

Not checked

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

GOARCH="amd64"
GOHOSTOS="linux"
OS=redhat 7.5

What did you do?

below code I'm using to generate digital signature

data := []byte("Checking fips mode")
hash := md5.New()
hash.Write(data)
bytesData := hash.Sum(nil)

signData, err := rsa.SignPKCS1v15(nil, privKeyObj, crypto.MD5, bytesData[:])
if err != nil {
        return "", err
}

I have build this code with go-toolset
GOOS=linux GOARCH=amd64 scl enable go-toolset-1.14 'go build -v -o fips-compliance-check'

When I run generated go binary in FIPS enabled host it should fail but unfortunately code is generating signature

I don't know whether is this issue or not.
Or am I lacking some understanding here?

@seankhliao seankhliao changed the title Can able generate digital signature when hash algorithm (digest method) is set to MD5 in FIPS mode crypto/rsa: can generate digital signature when hash algorithm (digest method) is set to MD5 in FIPS mode Apr 14, 2021
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 14, 2021
@seankhliao
Copy link
Member

cc @FiloSottile

@ijajmulani
Copy link
Author

ijajmulani commented Apr 16, 2021

I checked, my binary uses boringcrypto instead of native boringcrypto

`
go tool nm fips-compliance-check | grep Cfunc__goboringcrypto

4016b0 T _cgo_18935346a3e2_Cfunc__goboringcrypto_BN_bin2bn
401730 T _cgo_18935346a3e2_Cfunc__goboringcrypto_BN_bn2bin
401840 T _cgo_18935346a3e2_Cfunc__goboringcrypto_DLOPEN_OPENSSL
401ab0 T _cgo_18935346a3e2_Cfunc__goboringcrypto_ECDSA_sig
`

also I executed my binary with below command.
./fips-compliance-check -fipsMode=true

Still digital signature is generated with MD5 digest

Note --
The container where I'm building my code is not FIPS compliant. But machine where I'm executing binary is FIPS mode enabled.

@elagergren-spideroak
Copy link

elagergren-spideroak commented Apr 16, 2021

It's true that the boringcrypto branch allows MD5:

md := cryptoHashToMD(h)
if md == nil {
return nil, errors.New("crypto/rsa: unsupported hash function: " + strconv.Itoa(int(h)))
}
nid := C._goboringcrypto_EVP_MD_type(md)
var out []byte
var outLen C.uint
if priv.withKey(func(key *C.GO_RSA) C.int {
out = make([]byte, C._goboringcrypto_RSA_size(key))
return C._goboringcrypto_RSA_sign(nid, base(hashed), C.uint(len(hashed)),
func cryptoHashToMD(ch crypto.Hash) *C.GO_EVP_MD {
switch ch {
case crypto.MD5:
return C._goboringcrypto_EVP_md5()

@elagergren-spideroak
Copy link

elagergren-spideroak commented Apr 16, 2021

Also, the only FIPS 140-2 approved[1,2,3] hash functions are SHA-1, SHA-224, SHA-256, SHA-384 SHA-512, SHA-512/224, and SHA-512/256.

@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
@FiloSottile
Copy link
Contributor

AFAIK, Go+BoringCrypto will not actively stop you from using unapproved algorithms, and the Security Policy mentions it. It's up to the application to operate within the SP requirements. @agl can you confirm this is working as intended?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants