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

Improve the security of SUCI to SUPI conversion #47

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

linouxis9
Copy link

@linouxis9 linouxis9 commented Feb 17, 2025

Hi!

This PR improves:

  • The security of SUCI parsing using regex
  • Use built-in function to compress/uncompress Profile-B keys instead of rolling out the crypto ourselves :-)
  • Use constant time equality
  • Add much needed returns on errors as well
  • Stop the usage of logf.Printf
  • Added some units tests about compressed and uncompressed Profile B Ephemeral and Home Keys (using test data from TS 33.501 C.4.4)
  • Use crypto/ecdh (introduced in Go 1.20) instead of rolling out crypto, and factor what's possible between Profile A and Profile B

This work is part of an going project I'm working on to considerably up the code quality and security of free5gc!
This work is sponsored by Free Mobile!

Hope you find it useful.

CC: @ianchen0119

Thanks and cheers,
Valentin

@linouxis9 linouxis9 changed the title Improve the security of SUCI parsing, and use built-in function to compress/uncompress Profile-B keys instead of rolling out the crypto ourselves Improve the security of SUCI to SUPI conversion Feb 17, 2025
@linouxis9
Copy link
Author

Will fix the checks, thanks!

@linouxis9
Copy link
Author

For the following golangci-lint warnings:

Error: pkg/suci/suci.go:231:19: SA1019: elliptic.Marshal has been deprecated since Go 1.21: for ECDH, use the crypto/ecdh package. This function returns an encoding equivalent to that of PublicKey.Bytes in crypto/ecdh. (staticcheck)
  		pubKeyForECDH = elliptic.Marshal(elliptic.P256(), x, y)
  		                ^
Error: pkg/suci/suci.go:239:11: SA1019: elliptic.Unmarshal has been deprecated since Go 1.21: for ECDH, use the crypto/ecdh package. This function accepts an encoding equivalent to that of the NewPublicKey methods in crypto/ecdh. (staticcheck)
  		x, y := elliptic.Unmarshal(elliptic.P256(), transmittedPubKey)

Sadly, we don't have the choice to not use these deprecated functions. It's either we use these deprecated functions, or roll out our own functions, which should be avoided in crypto. This PR at least reduces the number of usage of deprecated unsafe functions such as elliptic.ScalarMult.

I had opened a Go issue about it yesterday: golang/go#71807

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.

1 participant