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

Detect crypto/tls/fipsonly on Go 1.17 #21

Closed
wants to merge 1 commit into from
Closed

Conversation

maraino
Copy link

@maraino maraino commented Nov 19, 2021

Description

This PR changes the check for boring crypto and fipsonly symbols to be compatible with Go 1.17, as well as the previous versions.

Fixes #20

@zaichang
Copy link

zaichang commented Dec 9, 2022

We'd love to have this merged so that our FIPS checking code can continue to work now that we've updated to Go 1.19.4
Who are the maintainers here?

@linouk23
Copy link

linouk23 commented Mar 5, 2023

@maraino
Copy link
Author

maraino commented Mar 5, 2023

@linouk23 boring crypto integration changed last year a lot, I can take a look to see if this PR is still valid or if you need something else.

@maraino
Copy link
Author

maraino commented Mar 5, 2023

@linouk23, it looks that this PR still works on Go 1.20.1, in this version, the condition strings.HasPrefix(name, "crypto/internal/boring/sig.BoringCrypto") is unnecessary, but I believe it was for Go 1.17 when I created this PR.

But at the same time, as a comment in Stack Overflow says, goversion is not necessary and you can use go tool nm

$ go tool nm main  | grep -E 'sig.FIPSOnly|sig.BoringCrypto|sig.StandardCrypto'
  52ea60 t crypto/internal/boring/sig.StandardCrypto.abi0
$ go tool nm main.fips | grep -E 'sig.FIPSOnly|sig.BoringCrypto|sig.StandardCrypto'
  532860 t crypto/internal/boring/sig.BoringCrypto.abi0
  532880 t crypto/internal/boring/sig.FIPSOnly.abi0

@rsc rsc closed this in 3a30cee Mar 9, 2023
@rsc
Copy link
Owner

rsc commented Mar 9, 2023

Fixed, thanks.

@maraino
Copy link
Author

maraino commented Mar 10, 2023

@rsc, Can you please tag a new release?

PS: I was pleasantly surprised that Go doesn't complain when you run go install rsc.io/goversion@master, the go.mod shows 1.21

@zaichang
Copy link

+1 on tagging a release. Thanks for merging this patch!

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

Successfully merging this pull request may close these issues.

With golang 1.17 the symbols are different
4 participants