minio-go icon indicating copy to clipboard operation
minio-go copied to clipboard

[Enhancement Request] Is it possible to have a way to disable sha256-simd via an env var or some other method?

Open tesshuflower opened this issue 3 years ago • 4 comments

We are building restic (https://github.com/restic/restic) which uses minio-go - we're building to be FIPS-compatible, so that means it's using a version of goboringcrypto.

What we find is when running in FIPS enabled environments, when the CPU supports it, minio-go is using sha256-simd as a replacement for crypto/sha256. This ends up creating a hash with sha256.New() which goboringcrypto does not recognize.

        "panic: goboringcrypto: hmac hash not recognized",
        "",
        "goroutine 21 [running]:",
        "crypto/hmac.New(0x8?, {0xc0001f0a80?, 0x0?, 0x8?})",
        "    crypto/hmac/hmac.go:136 +0x229",
        "github.com/minio/minio-go/v7/pkg/signer.sumHMAC({0xc0001f0a80?, 0xc0001f0a78?, 0x8?}, {0xc0001f0a90, 0x8, 0x8})",
        "    github.com/minio/minio-go/[email protected]/pkg/signer/utils.go:40 +0x4a",
        "github.com/minio/minio-go/v7/pkg/signer.getSigningKey({0xc00003a016?, 0x3?}, {0xe9ec5b, 0x9}, {0x9?, 0xc00019a480?, 0x0?}, {0xe98422, 0x2})",
        "    github.com/minio/minio-go/[email protected]/pkg/signer/request-signature-v4.go:68 +0xfc",
        "github.com/minio/minio-go/v7/pkg/signer.signV4({{0xe986bb, 0x3}, 0xc000284480, {0xe9d11d, 0x8}, 0x1, 0x1, 0xc0001f9e00, {0x0, 0x0}, ...}, ...)",
        "    github.com/minio/minio-go/[email protected]/pkg/signer/request-signature-v4.go:289 +0x55d",
        "github.com/minio/minio-go/v7/pkg/signer.SignV4(...)",
        "    github.com/minio/minio-go/[email protected]/pkg/signer/request-signature-v4.go:317",
        "github.com/minio/minio-go/v7.Client.getBucketLocationRequest({0xc000284240, 0xc000122cc0, 0x0, {{0x0, 0x0}, {0x0, 0x0}}, 0x0, 0xc0001f9c80, 0xc000124be0, ...}, ...)",
        "    github.com/minio/minio-go/[email protected]/bucket-cache.go:251 +0x770",
        "github.com/minio/minio-go/v7.Client.getBucketLocation({0xc000284240, 0xc000122cc0, 0x0, {{0x0, 0x0}, {0x0, 0x0}}, 0x0, 0xc0001f9c80, 0xc000124be0, ...}, ...)",
        "    github.com/minio/minio-go/[email protected]/bucket-cache.go:100 +0x118",
        "github.com/minio/minio-go/v7.Client.newRequest({0xc000284240, 0xc000122cc0, 0x0, {{0x0, 0x0}, {0x0, 0x0}}, 0x0, 0xc0001f9c80, 0xc000124be0, ...}, ...)",
        "    github.com/minio/minio-go/[email protected]/api.go:766 +0xde",
        "github.com/minio/minio-go/v7.Client.executeMethod({0xc000284240, 0xc000122cc0, 0x0, {{0x0, 0x0}, {0x0, 0x0}}, 0x0, 0xc0001f9c80, 0xc000124be0, ...}, ...)",
        "    github.com/minio/minio-go/[email protected]/api.go:637 +0x458",
        "github.com/minio/minio-go/v7.Client.listObjectsV2Query({0xc000284240, 0xc000122cc0, 0x0, {{0x0, 0x0}, {0x0, 0x0}}, 0x0, 0xc0001f9c80, 0xc000124be0, ...}, ...)",
        "    github.com/minio/minio-go/[email protected]/api-list.go:206 +0x7b4",
        "github.com/minio/minio-go/v7.Client.listObjectsV2.func1(0xc000122d20)",
        "    github.com/minio/minio-go/[email protected]/api-list.go:98 +0x24e",
        "created by github.com/minio/minio-go/v7.Client.listObjectsV2",
        "    github.com/minio/minio-go/[email protected]/api-list.go:92 +0x2ba",
        "ERROR: failure checking existence of repository"

Is there a way to allow an env var or something equivalent to allow us to disable the usage of the sha256-simd library (i.e. fallback to crypto/sha256 for certain envs?

For reference, we've seen that syncthing has implemented a workaround to disable the library via an env var: https://github.com/syncthing/syncthing/pull/3617

tesshuflower avatar Sep 21 '22 16:09 tesshuflower

We should not fail in this scenario, we should check why boring crypto is resulting in unexpected output.

harshavardhana avatar Sep 21 '22 16:09 harshavardhana

I think this is the place:

goboring tries to recognize the specific HMAC that is being used: https://github.com/golang/go/blob/dev.boringcrypto.go1.18/src/crypto/internal/boring/hmac.go (hashToMD)

We're seeing that hashToMD is returning nil.

tesshuflower avatar Sep 21 '22 17:09 tesshuflower

Okay that is quite terrible :-)

harshavardhana avatar Sep 21 '22 18:09 harshavardhana

goboring tries to recognize the specific HMAC that is being used: https://github.com/golang/go/blob/dev.boringcrypto.go1.18/src/crypto/internal/boring/hmac.go (hashToMD)

@aead @klauspost ^^ FYI when FIPS is compiled in.

harshavardhana avatar Sep 21 '22 18:09 harshavardhana

@tesshuflower The CustomSHA256 in Client options allows you to specify which sha256 hash to use.

The interface is easy to satisfy:

type Hasher interface {
	hash.Hash
	Close()
}

klauspost avatar Sep 26 '22 10:09 klauspost

@klauspost I think even if I try to set CustomSHA256 in the the client options, it still ends up calling utilities in pkg/signer that use the sha256-simd?

https://github.com/minio/minio-go/blob/master/pkg/signer/utils.go#L40

This seems to be called from bucket_cache: https://github.com/minio/minio-go/blob/master/bucket-cache.go#L257

and I don't see where it will use the customsha256 from options in this case - Possibly I'm missing something here though.

tesshuflower avatar Sep 26 '22 15:09 tesshuflower

I see. That is a mistake.

I may add add fips build tag as well.

klauspost avatar Sep 26 '22 16:09 klauspost

Yes, a fips build tag is needed. FIPS / boringcrypto requires that a specific implementation of SHA-256 is used. A "custom" SHA-256 implementation (like SHA256-simd) will not work.

aead avatar Sep 26 '22 17:09 aead

With #1700 You can

  1. Use build tag fips

or

  1. Replace CustomSHA256. The custom sha256 will not be used for signatures.

klauspost avatar Sep 27 '22 13:09 klauspost

Wow this is great! Thanks very much for the quick turnaround!

tesshuflower avatar Sep 27 '22 17:09 tesshuflower