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 SSL_CTX_set_msg_callback/SSL_set_msg_callback for ssl/tls record tracing #283

Merged
merged 9 commits into from
Aug 19, 2021

Conversation

CowboyTim
Copy link
Contributor

This adds support for the msg callback functionality to do some debugging for e.g. tls handshake.

  • lib/Net/SSLeay.pod: update manpage
  • updated Changes
  • updated MANIFEST
  • t/local/46_msg_callback.t added

…ecord tracing

- lib/Net/SSLeay.pod: update manpage
- updated Changes
- updated MANIFEST
- t/local/46_msg_callback.t added
Not all openssl versions have the same "version" for the tls record protocol
version.
The 3 things this sub needs to do:
  1. not die
  2. no memory leak (difficult to test)
  3. provide information

The validness of the buffer can be checked, so we use this as a validation
instead. This selftest is not here to validate the protocol and the intricacies
of the possible implementation or version (ssl3 vs tls1 etc).
A different openssl/protocol can't assume the same nr of records either. For
now, all buffer length checks need to be ok and at least 2 records logged via
SSL_CTX_msg_callback/SSL_msg_callback.
@CowboyTim CowboyTim closed this Aug 12, 2021
@CowboyTim CowboyTim reopened this Aug 12, 2021
Changes Outdated Show resolved Hide resolved
lib/Net/SSLeay.pod Outdated Show resolved Hide resolved
lib/Net/SSLeay.pod Outdated Show resolved Hide resolved
- point url to manmaster/man3 instead of just /ssl/
- fix SSL_CTX_set_msg_callback instead of CTX_SSL_set_msg_callback
lib/Net/SSLeay.pod Outdated Show resolved Hide resolved
lib/Net/SSLeay.pod Outdated Show resolved Hide resolved
SSLeay.xs Outdated Show resolved Hide resolved
…g_callback

- use the same argument order as the doc in OpenSSL for the callback function
    sub { my ($write_p,$version,$content_type,$buffer,$len,$ssl,$data) = @_; ... }
  instead of:
    sub { my ($ssl,$write_p,$version,$content_type,$buffer,$len,$data) = @_; ... }
- change the test so that it uses the callback data as a test
- add a (verify simple) test for undef/unset the callback
- add documentation that SSL_CTX_set_msg_callback_arg/SSL_set_msg_callback_arg
  aren't provided as the $arg/$data callback SV is handled automatically
@h-vn h-vn merged commit af3b45c into radiator-software:master Aug 19, 2021
@h-vn
Copy link
Contributor

h-vn commented Aug 19, 2021

To help with observing the messages in the callback function, future work could include:

  • adding constants listed on the manual page https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_msg_callback.html
  • providing helper functions within SSLeay.xs to convert record content type (Handshake, Alert, etc.) to strings
  • similar helpers for converting Handshake message types (ClientHello, Finished, etc.) to strings
  • likewise for Alerts for stringifying Level and Description

Here's an example from Curl:

* TLSv1.2 (OUT), TLS handshake, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (IN), TLS handshake, Finished (20):

@CowboyTim
Copy link
Contributor Author

Thx!
Yes, those constants etc,.. are indeed next on my todo list - but it will depend on time and availability. They are of less importance but it would be nice to have these constants and functions implemented as it makes this feature more complete and matching openssl/curl functionality.

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.

2 participants