Improve the security of SUCI to SUPI conversion

[linouxis9]

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]

Will fix the checks, thanks!

[linouxis9]

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

[linouxis9]

Hi!

As per new free5gc policy, I've rebased my code upon next branch.

Thanks and cheers,
Valentin

[ianchen0119]

Hi @linouxis9

Thanks for your contribution.
I'm planning to release the v4.0.0 next week, So I'll review the PR after new version released.

[Alonza0314]

Hi, @linouxis9

I have question about why this part is removed. I think this part can make log message be clear.

[linouxis9]

Hi @Alonza0314!

I thought that if the SUCI was able to be resolved, it would be better to use it in the log.
If we had an invalid SUCI, it would already had been logged earlier.

But you are right, it's probably clearer for the log flow to show from which SUCI this SUPI is coming from if auth failed! I'll add it back alongside SUPI.

Thanks a ton,
Valentin