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

implement SSLSocket#export_keying_material for doing RFC 5705 operations #530

Merged
merged 1 commit into from
Aug 31, 2022
Merged

implement SSLSocket#export_keying_material for doing RFC 5705 operations #530

merged 1 commit into from
Aug 31, 2022

Conversation

madblobfish
Copy link

@madblobfish madblobfish commented Aug 3, 2022

I need to generate shared ttls secrets from TLS sessions using this API.
Note this implementation is incomplete! as it does not allow using the context.
See the first commit how that could look. It did not work for me so I removed it.

Should I write a test for this?

Refs:
https://datatracker.ietf.org/doc/html/rfc5705
https://datatracker.ietf.org/doc/html/rfc8446#section-7.5
https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#exporter-labels
https://man.openbsd.org/SSL_export_keying_material.3

@madblobfish
Copy link
Author

madblobfish commented Aug 3, 2022

A super hacky radius implementation as a "real live" example can be found here: https://gist.github.com/madblobfish/9f1e89a3b5847ab80dcef16c56a4c0f9

@rhenium
Copy link
Member

rhenium commented Aug 6, 2022

See the first commit how that could look. It did not work for me so I removed it.

What is blocking it from working? Check for rb_scan_args() for how to implement optional parameters in C extensions.

It also needs a StringValue() to ensure that the argument is actually an instance of String.

Should I write a test for this?

Yes. :)

ext/openssl/ossl_ssl.c Outdated Show resolved Hide resolved
ext/openssl/ossl_ssl.c Outdated Show resolved Hide resolved
ext/openssl/ossl_ssl.c Outdated Show resolved Hide resolved
@rhenium rhenium linked an issue Aug 11, 2022 that may be closed by this pull request
@madblobfish
Copy link
Author

madblobfish commented Aug 12, 2022

This should fix everything, also squashed everything together (also did it wrong the first time, sorry for that noise).
Thanks for the rb_scan_args hint, that really helped :)
I've added a simple test, not sure how to test it deeper, but it seems to work (I also verified it with my application).

I got no example application code for the optional context functionality though.

Edit: removed useless comments from the test by another fix and squash

Copy link
Member

@rhenium rhenium left a comment

Choose a reason for hiding this comment

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

Sorry that it took long to respond. I added a few code comments.

ext/openssl/ossl_ssl.c Show resolved Hide resolved
test/openssl/test_ssl.rb Show resolved Hide resolved
ext/openssl/ossl_ssl.c Show resolved Hide resolved
@madblobfish
Copy link
Author

Don't worry about taking your time. Thanks for the review

@madblobfish
Copy link
Author

madblobfish commented Aug 21, 2022

new push should fix all comments, lets see if the CI agrees :)

Add OpenSSL::SSL::SSLSocket#export_keying_material to support RFC 5705
@madblobfish
Copy link
Author

Fixed failed tests (should have checked locally first 🤦) lets hope there are no compiler warnings left now

@rhenium rhenium merged commit ed83759 into ruby:master Aug 31, 2022
@rhenium
Copy link
Member

rhenium commented Aug 31, 2022

Thank you so much!

@Neustradamus
Copy link

@madblobfish, @rhenium: Thanks a lot!

It is not possible to add text about RFC 9266 support in code?

@madblobfish madblobfish deleted the openssl-add-rfc5705 branch September 3, 2022 16:06
@madblobfish
Copy link
Author

Hi @Neustradamus
The mechanism is defined in https://datatracker.ietf.org/doc/html/rfc8446#section-7.5 and formerly in https://datatracker.ietf.org/doc/html/rfc5705.
I do not think it makes sense to collect all implementing RFCs for that in this openssl library. The information would quickly be outdated and annoying to keep up to date.

An official and proper overview of them may be found in IANA's registry: https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#exporter-labels. Maybe this could be linked.
But in the end you can already find this registry linked in the openssl documentation: https://www.openssl.org/docs/man3.0/man3/SSL_export_keying_material.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

RFC 9266: Channel Bindings for TLS 1.3 support
3 participants