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

OpenSSL bindings for non standard crypto packages #100

Open
dbenoit17 opened this issue Aug 4, 2023 · 6 comments
Open

OpenSSL bindings for non standard crypto packages #100

dbenoit17 opened this issue Aug 4, 2023 · 6 comments

Comments

@dbenoit17
Copy link
Collaborator

dbenoit17 commented Aug 4, 2023

This project provides OpenSSL bindings for the Go standard crypto library, but there seems to be community interest in using non standard libraries such as x/crypto. Our approach so far with the project has been to provide effectively the same set of supported functionality as upstream Go, because the maintenance costs can be shared with the wider community via the upstream boringcrypto implementation.

Third party ecosystem packages do not have community implementations of boringcrypto, and without community agreement on a common implementation, we as a small community likely do not have the capacity to diverge and maintain our own custom implementation of non standard crypto packages. From time to time, the wider Go community does bring /x/ packages into the standard library, which is likely the best way to support those packages as it eliminates the risks associated with substantial divergence of community efforts (golang/go#61477) as an example of such a proposal). One approach could be to continue to advocate for more commonly used x/crypto packages to be adopted into the Go standard library upstream, or similarly, to advocate for a wider community implementation of boringcrypto bindings in the upstream x/crypto libraries.

Alternatively, for crypto which is not provided by Go standard library, it is technically possible for users to implement their own bindings on top of the internal golang-fips C API. An example in practice would be Grafana in CentOS Stream, which implements a patch to override the default implementation of x/crypto/pbkdf2 with its own calls into OpenSSL. While it's already demonstrably possible to do this, if there were a way to make the process more accessible (eg. API stability, documentation, boilerplate code generation), it might be a somewhat flexible way to accomplish roughly the same thing.

Does anyone have additional thoughts for users who would like to integrate non standard crypto libraries in their applications?

@qmuntal
Copy link
Collaborator

qmuntal commented Aug 7, 2023

We are also interested on supporting x/crypto packages (see microsoft/go#986 and microsoft/go#967).

The approach that we were currently pursuing is to fork x/crypto (we haven't done that yet) and patch it to use the OpenSSL/CNG backend wherever possible.

The problem with using the github.com/golang-fips/openssl module from outside the standard library is that the Go toolchain won't consider the same as the only linked with the standard library, due to how standard library vendoring works. This means that openssl.Init would have to be called independently, and that type checks like this won't work.

Luckily, using the go:linkname directive allows us to reference functions and variables from crypto/internal/backend, solving the aforementioned linker issue.

If we want to have a common x/crypto fork in golang-fips, these are some things to take into account:

  • Red Hat and Microsoft uses different experiment names to gate the openssl backend, we should support both, namely boringcrypto and systemcrypto.
  • The crypto/internal/backend API should be the same for all Go forks, else we would have problems importing it with go:linkname.
  • The x/crypto fork should be synced with upstream regularly, ideally automatically every X days. @dagood will probably have some insights here.

@xnox, from Canonical, is also interested on this.

@xnox
Copy link
Contributor

xnox commented Aug 7, 2023

I don't know about go:linkname - will read on that.

I do somehow wish for a nice way to consume x/crypto to the point I sort of wish for golang-fips toolchain to provide a built in x/crypto which would override any attempts to import the regular x/cryoto. And if a module is missing, the build should fail. But that is also tricky.

@dagood
Copy link
Collaborator

dagood commented Aug 7, 2023

I'll try to summarize what we (@qmuntal and I) wanted to support here and some problems we hit:

  • Replace the single x/crypto version used and vendored by the standard library.
    • This seems like the simple case. The Grafana patch seems to be doing this by sharing the C namespace, we tried using go:linkname. (We also want to make our non-cgo Windows CNG backend compatible here.)
  • Replace the use of x/crypto by an app, or by a module that the app depends on.
    • This requires something like automatic import replacement that directs the compiler at the vendored, modified x/crypto. Or, module replace... plus some similar magic to use the std-vendored module rather than the duplicate non-vendored package.
  • Support an app that uses a different version of x/crypto than the single version that's been vendored by the standard library.
    • New versions of x/crypto may add new APIs that we support, but the vendored x/crypto version doesn't include.
    • My understanding is that x/crypto may remove/rename APIs more freely than the standard library, so maintaining a single x/crypto version that is compatible with all desired x/crypto versions may not be feasible. (Maybe this isn't true? That would be nice.)
    • If we maintain, say, golang-fips/xcrypto so we have a version for each tag of x/crypto, and have our Go forks automatically use the corresponding fork version when a x/crypto tag is requested, how does that work mechanically in the build? Would the build try to download the module at that point? What if the app uses vendoring and doesn't allow downloading? We need a way around this that doesn't break the app's compatibility with upstream Go and doesn't break vendoring.
      • We could vendor every version of golang-fips/xcrypto in the Go toolset. This means we need to release a new Go version each time a new x/crypto version is released and apps would need to update their toolsets to get access to them.
      • Or: the app dev needs to add replace golang.org/x/crypto v0.10.0 => github.com/golang-fips/xcrypto v0.10.0 to the go.mod. This way, the fork will get downloaded during go mod vendor, etc. This seems like the simplest viable approach we've thought of. However, it means actually using the x/crypto fork all the time even for non-backend cases--even when it's just a passthrough to call the x/crypto implementation.
        • replace only affects the main module, so depending on how the app/project is being used, the "infectiousness" of this replace might be different. I just see the possibility that it could get contentious if added to a high-profile OSS project.
        • In many cases the project would be forked before the replace, but if the project maintainer is Microsoft/Red Hat/Canonical, it may end up being added directly to the main repo, and if there's any contentiousness, that's where I'd anticipate seeing it.

Please point out bad assumptions (or if I'm misremembering anything, @qmuntal), and sorry for the wall of text, but I'm hoping it'll be fruitful to share our thought process so far. 🙂

  • The x/crypto fork should be synced with upstream regularly, ideally automatically every X days. @dagood will probably have some insights here.

microsoft/go has some infra behind it in https://github.com/microsoft/go-infra that could potentially be reused, and I lean towards submodule-based forks rather than "true" forks for flexibility's sake but I don't know if everyone would be on board with that. For common ground I expect we'd use scheduled GitHub Actions and approve+merge PRs manually between the maintainers to make the process observable.

I think it's important here to figure out what we want to (and what we can) support. That might limit figuring out how to branch it and apply backend-interaction fixes across N branches. We can also figure out if we can handle that manually or go deeper into strategies that can simplify the process.

@xnox
Copy link
Contributor

xnox commented Sep 15, 2023

Experimenting with a toolchain that has openssl with custom things in place, it seems like we can contribute to x/crypto itself to do the right thing. Which might be the best thing to do anyway?

it appears that one can do stuff like

//go:build goexperiment.opensslcrypto
package sha3
import (
	_ "unsafe"
	"hash"
)
//go:linkname New384 crypto/internal/backend.NewSHA3_384
func New384() hash.Hash
//go:linkname Sum384 crypto/internal/backend.SHA3_384
func Sum384([]byte) [48]byte

Which basically gives access to crypto/internal symbols from external packages. if we add that inside x/crypto/sha3 itself, any external projects need to ensure they update their go.mod as normal and we are done? without need to maintain anything in our golang toolchains to imitate x/crypto?

Obviously this doesn't address things that are possible with openssl and do not even exist in neither x/crypto nor in go-lang toolchain but still doable no?

It will mean to potentially add more bindings in golang-fips/openssl and add more symbols in crypto/internal/backend as needed which other things will go:linkname to? Or does this all not good to you?

@xnox
Copy link
Contributor

xnox commented Sep 15, 2023

or do we need more things to expose things? i.e. goexperiment.opensslcrypto.provides-sha3? because i guess linkname will only be resolved at like runtime of the compiled app? or maybe x/crypto itself need extra build flags that people can turn on and off? aka buildflag to attempt to do sha3 via linkname.

@dagood
Copy link
Collaborator

dagood commented Sep 15, 2023

An update from the Microsoft end: we've been working on an automatic module replacement approach. One concern we had earlier was this:

  • My understanding is that x/crypto may remove/rename APIs more freely than the standard library, so maintaining a single x/crypto version that is compatible with all desired x/crypto versions may not be feasible. (Maybe this isn't true? That would be nice.)

But golang/go#56325 makes this much less concerning:

[FiloSottile]
[...] The [x/crypto] API is absolutely full of things I wish were not there, but it's been around for too long, and I don't think it would be tenable to make any backwards-incompatible changes without bumping the import path (which going from v0 to v1 doesn't do).

In practice, we've been upholding the Go Compatibility Promise in x/crypto for years, so we're effectively already at v1, we just need to call it that.

That means so far, we're comfortable with the idea of maintaining only one version in a fork (based on the latest x/crypto version) and forcing apps to use that.

The automatic replacement requires some patches to the toolset and I have a draft at microsoft/go#1043. (And it indeed uses go:linkname to reach into our internal APIs from the x/crypto fork. 🙂)


Experimenting with a toolchain that has openssl with custom things in place, it seems like we can contribute to x/crypto itself to do the right thing.

I'm initially skeptical that upstream would let us add things that are meaningless for "normal" Go, specific to our fork like //go:build goexperiment.opensslcrypto and crypto/internal/backend. Proposals for making it broadly easier to plug in new crypto APIs have tended not to go anywhere in the past. (AFAIK not even all Go-FIPS-focused forks are currently aligned on the crypto/internal/backend, let alone goexperiment.opensslcrypto.)

It would be fantastic to agree on backend API naming and make it into a de facto standard by pushing it to upstream, and I don't want to be dismissive of this approach because it would be much better if possible.

if we add that inside x/crypto/sha3 itself, any external projects need to ensure they update their go.mod as normal and we are done?

We have a perhaps extreme-sounding goal for Microsoft Go: backend swap-in should be possible without any source changes, not even go.mod/go.sum. It's not just an ease-of-use factor for us, but something we are explicitly targeting.

But: maybe other forks don't care about this and would prefer a simpler approach, even if it means their users need to patch OSS code that depends on x/crypto. We can share the x/crypto fork (or get it into upstream) and leave the automatic module replacement as a patch in only the Microsoft Go fork. I do think we can get our go:linkname comments to line up and work together, so we're still sharing effort.

Obviously this doesn't address things that are possible with openssl and do not even exist in neither x/crypto nor in go-lang toolchain but still doable no?

I think we do need to be capable of implementing x/crypto APIs with OpenSSL even if BoringCrypto doesn't implement them, to meet our FIPS goals. I think we could still try to upstream changes that make this happen, they're just more likely to be rejected (because they can't be tested with ordinary Go toolsets, etc.)

It will mean to potentially add more bindings in golang-fips/openssl and add more symbols in crypto/internal/backend as needed which other things will go:linkname to?

Yeah, I've already done some of this in the prototype/draft with SupportsHash (to determine if SHA3 is available), and I think we'll need more of these. And I think this is ok.

or do we need more things to expose things? i.e. goexperiment.opensslcrypto.provides-sha3?

So far I only think only one "should we replace x/crypto functionality?" build tag/experiment is needed. (I called it GOEXPERIMENT=xcryptobackendswap in my draft.) I think for specific functions it's better to add APIs like SupportsHash, though--it would be overwhelming IMO to force users of the Go toolset to set the right goexperiment.opensslcrypto.provides-sha3, goexperiment.opensslcrypto.provides-hkdf, etc. while building their apps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants