microsoft / go-crypto-openssl

Go crypto backend for Linux using OpenSSL
MIT License
55 stars 14 forks source link

Add API support for SHA3-384 and friends #57

Closed xnox closed 1 year ago

xnox commented 1 year ago

openssl provides sha3-384 implementation.

when available please expose sha3-384 implementation.

This is to gain access to a FIPS certified SHA3 implementation instead of golang.org/x/crypto/sha3

ideally as a drop in replacement for golang.org/x/crypto/sha3

dagood commented 1 year ago

Mind if I ask how you're using this library to create a FIPS-compliant Go application? With the way this library is currently implemented, it doesn't seem like it would necessarily be straightforward to use the new API in practice. go-crypto-openssl is designed to be used by forks of Go, where the fork internally loads OpenSSL--it would be difficult in this situation for a non-built-in crypto package to get access to the same loaded OpenSSL.

If you're writing an application from scratch and using github.com/microsoft/go-crypto-openssl instead of Go crypto, this would make sense.

This repo is not designed as a drop-in replacement for crypto, so I don't think we would want to maintain a drop-in replacement for sha3 unless the API happens to match up with what we'd naturally add to support sha3. (Either way, a package could be created that wraps this library in APIs more like crypto and sha3's.)

xnox commented 1 year ago

Mind if I ask how you're using this library to create a FIPS-compliant Go application?

Yes, that's the intention. github.com/snapcore/snapd uses crypto, and _ golang.org/x/crypto/sha3 and would like to use Canonical Ubuntu OpenSSL FIPS which has certified RSA, SHA-2 and SHA-3.

With the way this library is currently implemented, it doesn't seem like it would necessarily be straightforward to use the new API in practice. go-crypto-openssl is designed to be used by forks of Go, where the fork internally loads OpenSSL--it would be difficult in this situation for a non-built-in crypto package to get access to the same loaded OpenSSL.

However, that's what built-in crypto package requires one to do to use crypto.SHA3_384.

import "crypto"
import _ "golang.org/x/crypto/sha3"
....
crypto.SHA3_384() becomes usable

So sha3 is half-way in the crypto package, it works as long as side-effect init of crypto/sha3 has been imported. Poking this module, it seems like the easiest way to bring over sha3, albeit with duplicate import of this whole module.

If you're writing an application from scratch and using github.com/microsoft/go-crypto-openssl instead of Go crypto, this would make sense.

In snapd source, and also elsewhere on the github there are many users of both direct users of golang.org/x/crypto/sha3 package and those that rely on the side-effect hash registration.

This repo is not designed as a drop-in replacement for crypto, so I don't think we would want to maintain a drop-in replacement for sha3 unless the API happens to match up with what we'd naturally add to support sha3. (Either way, a package could be created that wraps this library in APIs more like crypto and sha3's.)

To be more specific I understand that this module is not replacement for "crypto", but I hope it can offer replacement for import _ "golang.org/x/crypto/sha3"which integrates with the "crypto" module of the patched FIPS toolchain.

qmuntal commented 1 year ago

Thanks for the detailing your use-case and for sending out a working PR @xnox. This repo is going to be archived once the Microsoft Go fork migrates to https://github.com/golang-fips/openssl (see https://github.com/golang-fips/openssl/issues/83). Therefore, I suggest you to fill an issue in that repo.

Following @dagood comments, the openssl package follows the API defined in crypto/internal/boring so facilitate integrating OpenSSL into Go standard library, making it unsuitable as a drop-in replacement for the public crypto API.

Having said this, I'm ok adding drop-in replacements for the x/crypto packages, in fact Red Hat already add support for hkdf to https://github.com/golang-fips/openssl. I'm still not sure about how to support both use-cases without adding too much maintenance burden, though.

xnox commented 1 year ago

Thanks for the detailing your use-case and for sending out a working PR @xnox. This repo is going to be archived once the Microsoft Go fork migrates to https://github.com/golang-fips/openssl (see https://github.com/golang-fips/openssl/issues/83). Therefore, I suggest you to fill an issue in that repo.

Ack. I found this repository by following what Microsoft Go fork current versions used. I can absolutely make a similar issue and PR there.

Following @dagood comments, the openssl package follows the API defined in crypto/internal/boring so facilitate integrating OpenSSL into Go standard library, making it unsuitable as a drop-in replacement for the public crypto API.

Having said this, I'm ok adding drop-in replacements for the x/crypto packages, in fact Red Hat already add support for hkdf to https://github.com/golang-fips/openssl. I'm still not sure about how to support both use-cases without adding too much maintenance burden, though.

I mean, everything would be much easier if sha3 graduated to be part of the standard library, as then everything would be in the compiler provided packages.

I will ask upstream about graduating sha3 into standard library.

dagood commented 1 year ago

Thanks for the info about the standard library crypto.SHA3_384! I read through https://pkg.go.dev/golang.org/x/crypto/sha3 and didn't realize there was any API to use it other than the direct calls listed there. Now I see the comments on the various crypto.Hash consts about what needs to be imported for which hash.

For the sake of discussion... the way we're designing the fork, I believe we would want SHA3384 to be usable through OpenSSL even without removing a `import "golang.org/x/crypto/sha3"line. I think that means we need to override the mechanism that registers the x/crypto implementation in favor of one implemented ingithub.com/microsoft/go-crypto-openssl`. For our use case, that would mean it's not so much a drop-in implementation but more of an implementation that the forked Go toolset swaps in for you.

That's not to say that a drop-in module can't be made, I just don't think that it's the ideal approach when a Go fork is in the mix that can be used to do this in a more source-compatible way.

(For the sake of ensuring "new" code written with our fork is still compatible with official Go, we should likely still make sure import _ "golang.org/x/crypto/sha3" is present by making sure the registration happened, and only then ignore it in favor of OpenSSL.)

xnox commented 1 year ago

I think that means we need to override the mechanism that registers the x/crypto implementation

yes yes yes. Ideally somehow internal openssl/sha3 should be registered with crypto api, and exposed instead of x/crypto/sha3 whenever that is imported for direct usage or via crypto api.

I will open that as a separate request on the compiler fork, once sha3 is available in the openssl module.

qmuntal commented 1 year ago

Closing this issue in favor of that https://github.com/golang-fips/openssl/issues/87.