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: boringcrypto NewGCMTLS no longer accessible #56326

Open
wadey opened this issue Oct 19, 2022 · 3 comments
Open

crypto: boringcrypto NewGCMTLS no longer accessible #56326

wadey opened this issue Oct 19, 2022 · 3 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@wadey
Copy link
Contributor

wadey commented Oct 19, 2022

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

go1.19.2

Does this issue reproduce with the latest release?

Yes

What did you do?

Before go1.19, the dev.boringcrypto branch let you access the NewGCMTLS function because it was exposed on the AES cipher struct.

You could do the following:

type gcmtls interface {
	NewGCMTLS() (cipher.AEAD, error)
}

if aesTLS, ok := aesCipher.(gcmtls); ok {
	gcm, err = aesTLS.NewGCMTLS()
} else {
	gcm, err = cipher.NewGCM(c)
}

But this is no longer possible as the method is no longer exposed on the internal/boring AES struct. It was removed by this CL: https://go-review.googlesource.com/c/go/+/403976 as part of #51940.

It is useful to be able to use this GCM mode directly from an application using the boringcrypto experiment. The way it was exposed on the aes struct previously was ideal since it was hidden unless boringcrypto was active, but any other way of exposing it would work as well.

@wadey
Copy link
Contributor Author

wadey commented Oct 19, 2022

As an extra note, the future to-be approved version of BoringCrypto adds a "Random Nonce" (EVP_aead_aes_256_gcm_randnonce) mode for GCM that will be even more useful for folks using boringcrypto, so finding a good way to expose both of these would be ideal.

https://boringssl-review.googlesource.com/c/boringssl/+/43686

@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 19, 2022
@seankhliao
Copy link
Member

cc @golang/security

@wadey
Copy link
Contributor Author

wadey commented Oct 27, 2022

If anyone finds this, there is another workaround that is possible until this issue is resolved:

//go:linkname newGCMTLS crypto/internal/boring.NewGCMTLS
func newGCMTLS(c cipher.Block) (cipher.AEAD, error)

@seankhliao seankhliao added this to the Unplanned milestone Nov 19, 2022
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

2 participants