Skip to content

Add support for the TLS13-KDF algorithm #272

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

Merged
merged 10 commits into from
Apr 24, 2025

Conversation

nicholasberlin
Copy link
Contributor

@nicholasberlin nicholasberlin commented Apr 14, 2025

We are attempting to run a go binary on Ubuntu with FIPS enabled. But, the openssl shared object is rejecting our binary's usage of TLS1.3 because the iv length.

microsoft/go#1626

This PR makes use of the KDF specialization carve out for TLS1.3 when the code detects it.
This PR adds functions for external users to make use of TLS13-KDF

@nicholasberlin nicholasberlin force-pushed the nberlin/add_tls13_kdf_support branch from 6ea7791 to 1ede3fc Compare April 14, 2025 19:17
Copy link
Collaborator

@qmuntal qmuntal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting this. I would prefer to have a dedicated on-shot API for TLS 1.3 KDF. Something like func SupportsTLS13KDF() and func ExpandTLS13KDF(fh func() hash.Hash, secret []byte, label string, context []byte, keyLength int) []byte. The caller can decide whether to use this or not based on SupportsTLS13KDF.

Copy link
Collaborator

@dagood dagood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly localized style comments, please feel free to wait for @qmuntal's next review to see if they're applicable in the big picture. 🙂

- Don't panic if there's no support
- Don't export the parse function
  - Move unit tests to hkdf_interal_test.go which use the same package
- Rework the parse function to return a boolean at the end
@nicholasberlin
Copy link
Contributor Author

We don't need this function if newHKDFCtx3_with_TLS13_KDF_Support accepts a label and a context as byte slices. I don't see the benefit on passing them as a tlf1.3 info. This way, when this KDF is available, callers can avoid encoding the info and just rely on the the kdf to return the correctly encoded data.

I believe I've addressed everything up to this comment. Back to it tomorrow.

@nicholasberlin
Copy link
Contributor Author

Oh, and this one:

Also, HKDF should not appear in the name of the function.

Reduce to only an Expand function based on TLS13-KDF
Remove associated cruft
Add new unit tests
@nicholasberlin
Copy link
Contributor Author

Ok, I think this is ready for another round

nicholasberlin and others added 5 commits April 23, 2025 09:01
Co-authored-by: Quim Muntal <quimmuntal@gmail.com>
Co-authored-by: Quim Muntal <quimmuntal@gmail.com>
Copy link
Collaborator

@qmuntal qmuntal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@qmuntal qmuntal merged commit b488d85 into golang-fips:v2 Apr 24, 2025
25 checks passed
@nicholasberlin nicholasberlin deleted the nberlin/add_tls13_kdf_support branch April 24, 2025 16:18
nicholasberlin added a commit to nicholasberlin/openssl that referenced this pull request May 1, 2025
* Add support for the TLS13-KDF algorithm

* Change to opt-in

* Code review suggestions

- Don't panic if there's no support
- Don't export the parse function
  - Move unit tests to hkdf_interal_test.go which use the same package
- Rework the parse function to return a boolean at the end

* Purge parsing

Reduce to only an Expand function based on TLS13-KDF
Remove associated cruft
Add new unit tests

* Update hkdf.go

Co-authored-by: Quim Muntal <quimmuntal@gmail.com>

* Update hkdf.go

Co-authored-by: Quim Muntal <quimmuntal@gmail.com>

* Be explicit, not clever.

* Do not ignore testdata/

* panic for unknown openssl versions

---------

Co-authored-by: Quim Muntal <quimmuntal@gmail.com>
nicholasberlin added a commit to nicholasberlin/openssl that referenced this pull request May 5, 2025
* Add support for the TLS13-KDF algorithm

* Change to opt-in

* Code review suggestions

- Don't panic if there's no support
- Don't export the parse function
  - Move unit tests to hkdf_interal_test.go which use the same package
- Rework the parse function to return a boolean at the end

* Purge parsing

Reduce to only an Expand function based on TLS13-KDF
Remove associated cruft
Add new unit tests

* Update hkdf.go

Co-authored-by: Quim Muntal <quimmuntal@gmail.com>

* Update hkdf.go

Co-authored-by: Quim Muntal <quimmuntal@gmail.com>

* Be explicit, not clever.

* Do not ignore testdata/

* panic for unknown openssl versions

---------

Co-authored-by: Quim Muntal <quimmuntal@gmail.com>
nicholasberlin added a commit to nicholasberlin/openssl that referenced this pull request May 5, 2025
* Add support for the TLS13-KDF algorithm

* Change to opt-in

* Code review suggestions

- Don't panic if there's no support
- Don't export the parse function
  - Move unit tests to hkdf_interal_test.go which use the same package
- Rework the parse function to return a boolean at the end

* Purge parsing

Reduce to only an Expand function based on TLS13-KDF
Remove associated cruft
Add new unit tests

* Update hkdf.go

Co-authored-by: Quim Muntal <quimmuntal@gmail.com>

* Update hkdf.go

Co-authored-by: Quim Muntal <quimmuntal@gmail.com>

* Be explicit, not clever.

* Do not ignore testdata/

* panic for unknown openssl versions

---------

Co-authored-by: Quim Muntal <quimmuntal@gmail.com>
qmuntal added a commit that referenced this pull request May 5, 2025
* Add support for the TLS13-KDF algorithm (#272)

* Add support for the TLS13-KDF algorithm

* Change to opt-in

* Code review suggestions

- Don't panic if there's no support
- Don't export the parse function
  - Move unit tests to hkdf_interal_test.go which use the same package
- Rework the parse function to return a boolean at the end

* Purge parsing

Reduce to only an Expand function based on TLS13-KDF
Remove associated cruft
Add new unit tests

* Update hkdf.go

Co-authored-by: Quim Muntal <quimmuntal@gmail.com>

* Update hkdf.go

Co-authored-by: Quim Muntal <quimmuntal@gmail.com>

* Be explicit, not clever.

* Do not ignore testdata/

* panic for unknown openssl versions

---------

Co-authored-by: Quim Muntal <quimmuntal@gmail.com>

* Fix up cherry-pick

* hkdf: Replace nil salt with a slice of a preallocated all zeros buffer (#260)

* hkdf: Replace nil salt with a slice of a preallocated all zeros buffer. This fixes HKDF when using KeyPair FIPS Provider for OpenSSL 3

* hkdf: fixed PR review comments

* hkdf: second round of PR comment fixes

* hkdf: third round of PR comment fixes

---------

Co-authored-by: Quim Muntal <quimmuntal@gmail.com>
Co-authored-by: samiponkanenssh <112563010+samiponkanenssh@users.noreply.github.com>
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.

3 participants