Skip to content

Conversation

@ccutrer
Copy link

@ccutrer ccutrer commented Nov 17, 2025

Closes #800

Note that the callback code is mostly based on apps/lib/s_cb.c in openssl.

@ccutrer
Copy link
Author

ccutrer commented Nov 17, 2025

This is working, but I still want to improve string allocation in the callbacks.

args.version = rb_sprintf("Not TLS data or unknown version (version=%d, content_type=%d)", version, content_type);
}

args.buf = rb_str_new(bp, len);
Copy link
Member

Choose a reason for hiding this comment

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

Any Ruby object allocation that isn't an immediate value can potentially fail, so these rb_str_new*() calls need to be moved to inside rb_protect().

Comment on lines +416 to +417
rb_set_errinfo(Qnil);
rb_warn("exception in msg_callback is ignored");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
rb_set_errinfo(Qnil);
rb_warn("exception in msg_callback is ignored");
VALUE ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx);
rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state));

Comment on lines +233 to +306
static STRINT_PAIR ssl_versions[] = {
{"SSL 3.0", SSL3_VERSION},
{"TLS 1.0", TLS1_VERSION},
{"TLS 1.1", TLS1_1_VERSION},
{"TLS 1.2", TLS1_2_VERSION},
{"TLS 1.3", TLS1_3_VERSION},
{"DTLS 1.0", DTLS1_VERSION},
{"DTLS 1.0 (bad)", DTLS1_BAD_VER},
{NULL}
};

static STRINT_PAIR alert_types[] = {
{"close_notify", 0},
{"end_of_early_data", 1},
{"unexpected_message", 10},
{"bad_record_mac", 20},
{"decryption_failed", 21},
{"record_overflow", 22},
{"decompression_failure", 30},
{"handshake_failure", 40},
{"bad_certificate", 42},
{"unsupported_certificate", 43},
{"certificate_revoked", 44},
{"certificate_expired", 45},
{"certificate_unknown", 46},
{"illegal_parameter", 47},
{"unknown_ca", 48},
{"access_denied", 49},
{"decode_error", 50},
{"decrypt_error", 51},
{"export_restriction", 60},
{"protocol_version", 70},
{"insufficient_security", 71},
{"internal_error", 80},
{"inappropriate_fallback", 86},
{"user_canceled", 90},
{"no_renegotiation", 100},
{"missing_extension", 109},
{"unsupported_extension", 110},
{"certificate_unobtainable", 111},
{"unrecognized_name", 112},
{"bad_certificate_status_response", 113},
{"bad_certificate_hash_value", 114},
{"unknown_psk_identity", 115},
{"certificate_required", 116},
{NULL}
};

static STRINT_PAIR handshakes[] = {
{"HelloRequest", SSL3_MT_HELLO_REQUEST},
{"ClientHello", SSL3_MT_CLIENT_HELLO},
{"ServerHello", SSL3_MT_SERVER_HELLO},
{"HelloVerifyRequest", DTLS1_MT_HELLO_VERIFY_REQUEST},
{"NewSessionTicket", SSL3_MT_NEWSESSION_TICKET},
{"EndOfEarlyData", SSL3_MT_END_OF_EARLY_DATA},
{"EncryptedExtensions", SSL3_MT_ENCRYPTED_EXTENSIONS},
{"Certificate", SSL3_MT_CERTIFICATE},
{"ServerKeyExchange", SSL3_MT_SERVER_KEY_EXCHANGE},
{"CertificateRequest", SSL3_MT_CERTIFICATE_REQUEST},
{"ServerHelloDone", SSL3_MT_SERVER_DONE},
{"CertificateVerify", SSL3_MT_CERTIFICATE_VERIFY},
{"ClientKeyExchange", SSL3_MT_CLIENT_KEY_EXCHANGE},
{"Finished", SSL3_MT_FINISHED},
{"CertificateUrl", SSL3_MT_CERTIFICATE_URL},
{"CertificateStatus", SSL3_MT_CERTIFICATE_STATUS},
{"SupplementalData", SSL3_MT_SUPPLEMENTAL_DATA},
{"KeyUpdate", SSL3_MT_KEY_UPDATE},
{"CompressedCertificate", SSL3_MT_COMPRESSED_CERTIFICATE},
#ifndef OPENSSL_NO_NEXTPROTONEG
{"NextProto", SSL3_MT_NEXT_PROTO},
#endif
{"MessageHash", SSL3_MT_MESSAGE_HASH},
{NULL}
};
Copy link
Member

Choose a reason for hiding this comment

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

Is there any way to get human-readable strings from the OpenSSL library? I'd prefer not to maintain these string lists within ruby/openssl, since they're likely to fall out of sync quickly.

cb = rb_attr_get(v_ctx, id_i_msg_callback);
if (!NIL_P(cb)) {
SSL_set_msg_callback(ssl, imp_ssl_msg_callback);
SSL_set_msg_callback_arg(ssl, (void *)cb);
Copy link
Member

Choose a reason for hiding this comment

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

A GC compaction could make the cb reference stale. We need to either prevent compaction by calling rb_gc_mark(the_proc_object) in ossl_ssl_mark(), or implement a "dcompact" handler to update the stored reference after a compaction.

Alternatively, since imp_ssl_msg_callback() receives the SSL *, it can look up id_i_msg_callback there. This is probably easier and is what existing callbacks currently do.

Copy link
Member

Choose a reason for hiding this comment

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

We probably should set the callback to SSL_CTX, in ossl_sslctx_setup(). All SSL objects created with the SSL_CTX will inherit the settings.

id_i_alpn_select_cb, id_i_alpn_protocols, id_i_servername_cb,
id_i_verify_hostname, id_i_keylog_cb, id_i_tmp_dh_callback;
id_i_verify_hostname, id_i_keylog_cb, id_i_tmp_dh_callback,
id_i_msg_callback;
Copy link
Member

Choose a reason for hiding this comment

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

The indentation is inconsistent throughout the patch. You might need to configure your text editor to use a tab size of 8.

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.

Can we get DEBUG loggin from the OpenSSL library?

2 participants