Skip to content

Refactor: dust off OpenSSL::BIO [fixup #16480] #16640

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.

Sign up for GitHub

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
ysbaddaden merged 1 commit into from
Feb 10, 2026
Merged

Refactor: dust off OpenSSL::BIO [fixup #16480] #16640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 79 additions & 104 deletions src/openssl/bio.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,116 +2,91 @@ require "./lib_crypto"

# :nodoc:
struct OpenSSL::BIO
def self.get_data(bio) : Void*
{% if LibCrypto.has_method?(:BIO_get_data) %}
LibCrypto.BIO_get_data(bio)
CRYSTAL_BIO = begin
biom = LibCrypto.BIO_meth_new(Int32::MAX, "Crystal BIO")

{% if LibCrypto.has_method?(:BIO_meth_set_read_ex) %}
LibCrypto.BIO_meth_set_read_ex(biom, ->read_ex)
{% else %}
bio.value.ptr
LibCrypto.BIO_meth_set_read(biom, ->read)
{% end %}
end

def self.set_data(bio, data : Void*)
{% if LibCrypto.has_method?(:BIO_set_data) %}
LibCrypto.BIO_set_data(bio, data)
{% if LibCrypto.has_method?(:BIO_meth_set_write_ex) %}
LibCrypto.BIO_meth_set_write_ex(biom, ->write_ex)
{% else %}
bio.value.ptr = data
LibCrypto.BIO_meth_set_write(biom, ->write)
{% end %}

LibCrypto.BIO_meth_set_ctrl(biom, ->ctrl)
LibCrypto.BIO_meth_set_create(biom, ->create)
LibCrypto.BIO_meth_set_destroy(biom, ->destroy)

biom
end

CRYSTAL_BIO = begin
bwrite = LibCrypto::BioMethodWriteOld.new do |bio, data, len|
io = Box(IO).unbox(BIO.get_data(bio))
io.write Slice.new(data, len)
len
end

bwrite_ex = LibCrypto::BioMethodWrite.new do |bio, data, len, writep|
count = len > Int32::MAX ? Int32::MAX : len.to_i
io = Box(IO).unbox(BIO.get_data(bio))
io.write Slice.new(data, count)
writep.value = LibC::SizeT.new(count)
1
end

bread = LibCrypto::BioMethodReadOld.new do |bio, buffer, len|
io = Box(IO).unbox(BIO.get_data(bio))
io.flush
io.read(Slice.new(buffer, len)).to_i
end

bread_ex = LibCrypto::BioMethodWrite.new do |bio, buffer, len, readp|
count = len > Int32::MAX ? Int32::MAX : len.to_i
io = Box(IO).unbox(BIO.get_data(bio))
io.flush
ret = io.read Slice.new(buffer, count)
readp.value = LibC::SizeT.new(ret)
1
end

ctrl = LibCrypto::BioMethodCtrl.new do |bio, cmd, num, ptr|
io = Box(IO).unbox(BIO.get_data(bio))

val = case cmd
when LibCrypto::CTRL_FLUSH
io.flush
1
when LibCrypto::CTRL_PUSH, LibCrypto::CTRL_POP, LibCrypto::CTRL_EOF
0
when LibCrypto::CTRL_SET_KTLS_SEND
0
when LibCrypto::CTRL_GET_KTLS_SEND, LibCrypto::CTRL_GET_KTLS_RECV
0
else
STDERR.puts "WARNING: Unsupported BIO ctrl call (#{cmd})"
0
end
LibCrypto::Long.new(val)
end

create = LibCrypto::BioMethodCreate.new do |bio|
{% if LibCrypto.has_method?(:BIO_set_shutdown) %}
LibCrypto.BIO_set_shutdown(bio, 1)
LibCrypto.BIO_set_init(bio, 1)
# bio.value.num = -1
{% else %}
bio.value.shutdown = 1
bio.value.init = 1
bio.value.num = -1
{% end %}
1
end

destroy = LibCrypto::BioMethodDestroy.new do |bio|
BIO.set_data(bio, Pointer(Void).null)
1
end

{% if LibCrypto.has_method?(:BIO_meth_new) %}
biom = LibCrypto.BIO_meth_new(Int32::MAX, "Crystal BIO")

{% if LibCrypto.has_method?(:BIO_meth_set_write_ex) %}
LibCrypto.BIO_meth_set_write_ex(biom, bwrite_ex)
LibCrypto.BIO_meth_set_read_ex(biom, bread_ex)
{% else %}
LibCrypto.BIO_meth_set_write(biom, bwrite)
LibCrypto.BIO_meth_set_read(biom, bread)
{% end %}

LibCrypto.BIO_meth_set_ctrl(biom, ctrl)
LibCrypto.BIO_meth_set_create(biom, create)
LibCrypto.BIO_meth_set_destroy(biom, destroy)
biom
{% else %}
biom = Pointer(LibCrypto::BioMethod).malloc(1)
biom.value.type_id = Int32::MAX
biom.value.name = "Crystal BIO"
biom.value.bwrite = bwrite
biom.value.bread = bread
biom.value.ctrl = ctrl
biom.value.create = create
biom.value.destroy = destroy
biom
{% end %}
def self.write_ex(bio, data, len, writep)
count = len > Int32::MAX ? Int32::MAX : len.to_i
io = Box(IO).unbox(LibCrypto.BIO_get_data(bio))
io.write Slice.new(data, count)
writep.value = LibC::SizeT.new(count)
1
end

def self.write(bio, data, len)
io = Box(IO).unbox(LibCrypto.BIO_get_data(bio))
io.write Slice.new(data, len)
len
Copy link
Member

@straight-shoota straight-shoota Feb 6, 2026

Choose a reason for hiding this comment

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

thought: Not directly related to this change: What happens when these methods raise?
Do we need to rescue exceptions and return an error value?

Copy link
Collaborator Author

@ysbaddaden ysbaddaden Feb 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Exactly my thoughts when I saw that. It looks like it's working in practice, but we might want to investigate.

We should review the write and write_ex methods (and read counterparts) to see if we shall rescue and return -errno or -1 and set errno, or set the SSL error, or something else entirely.

(in a follow up)

Copy link
Collaborator Author

@ysbaddaden ysbaddaden Feb 6, 2026

Choose a reason for hiding this comment

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

Let's review the whole BIO and compare it with https://github.com/openssl/openssl/blob/baf4156f7052cf5fa08aaf5187dc1f5d25e49664/crypto/bio/bss_sock.c

There's likely a bunch of details to fix (and KTLS support to hack in).

Copy link
Collaborator Author

@ysbaddaden ysbaddaden Feb 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Example: BIO_meth_new(Int32::MAX, "crystal") should be (omitting LibCrypto for readability):

id = BIO_get_new_index
methods = BIO_meth_new(id | BIO_TYPE_SOURCE_SINK | BIO_TYPE_DESCRIPTOR, "crystal")

Copy link
Collaborator Author

@ysbaddaden ysbaddaden Feb 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Reading the OpenSSL source code, I believe it expects the BIO methods to return -1 with the system error set (Errno, WinError, ...). In practice the nested C calls are expected to each fail immediately and return -1 with the system error already set by the libc functions.

We'd probably want to do that. Exceptions going from Crystal -> C -> Crystal, thus bypassing the C land, might be a bad idea. Though it might not be an issue in practice: read or write fails and the C functions might just return ret if ret <= 0 (bubble the error up with no cleanup).

An issue is that we'd raise twice. If we could assume that io is a Socket, then maybe we could bypass IO (i.e. call Crystal::EventLoop#read(Socket) directly), and rework the Crystal::EventLoop interface to return system errors (e.g. #read : Errno | WinError | SizeT) instead of raising 🤔

Copy link
Member

@straight-shoota straight-shoota Feb 9, 2026

Choose a reason for hiding this comment

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

Interacting directly with Socket on the event loop would align well with #16642.

Copy link
Collaborator Author

@ysbaddaden ysbaddaden Feb 9, 2026

Choose a reason for hiding this comment

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

We actually need it to be a general IO. There's one usage in stdlib where we use an IO::Memory for getting the value out of x509 certificate.

Now, nothing prevents us from having two custom BIOs: one for general IO and another dedicated for Socket.

end

def self.read_ex(bio, buffer, len, readp)
count = len > Int32::MAX ? Int32::MAX : len.to_i
io = Box(IO).unbox(LibCrypto.BIO_get_data(bio))

# FIXME: why flush (write) before reading?!
io.flush

ret = io.read Slice.new(buffer, count)
readp.value = LibC::SizeT.new(ret)
1
end

def self.read(bio, buffer, len)
io = Box(IO).unbox(LibCrypto.BIO_get_data(bio))

# FIXME: why flush (write) before reading?!
io.flush

io.read(Slice.new(buffer, len)).to_i
end

def self.ctrl(bio, cmd, num, ptr)
io = Box(IO).unbox(LibCrypto.BIO_get_data(bio))
val = case cmd
when LibCrypto::CTRL_FLUSH
io.flush
1
when LibCrypto::CTRL_PUSH, LibCrypto::CTRL_POP, LibCrypto::CTRL_EOF
0
when LibCrypto::CTRL_SET_KTLS_SEND
0
when LibCrypto::CTRL_GET_KTLS_SEND, LibCrypto::CTRL_GET_KTLS_RECV
0
else
STDERR.puts "WARNING: Unsupported BIO ctrl call (#{cmd})"
0
end
LibCrypto::Long.new(val)
end

def self.create(bio)
LibCrypto.BIO_set_shutdown(bio, 1)
LibCrypto.BIO_set_init(bio, 1)
1
end

def self.destroy(bio)
LibCrypto.BIO_set_data(bio, Pointer(Void).null)
1
end

@boxed_io : Void*
Expand All @@ -124,7 +99,7 @@ struct OpenSSL::BIO
# not in Crystal-land.
@boxed_io = Box(IO).box(io)

BIO.set_data(@bio, @boxed_io)
LibCrypto.BIO_set_data(@bio, @boxed_io)
end

getter io
Expand Down
34 changes: 15 additions & 19 deletions src/openssl/lib_crypto.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,6 @@ lib LibCrypto
CTRL_GET_KTLS_SEND = 73
CTRL_GET_KTLS_RECV = 76

alias BioMethodWrite = (Bio*, Char*, SizeT, SizeT*) -> Int
alias BioMethodWriteOld = (Bio*, Char*, Int) -> Int
alias BioMethodRead = (Bio*, Char*, SizeT, SizeT*) -> Int
alias BioMethodReadOld = (Bio*, Char*, Int) -> Int
alias BioMethodPuts = (Bio*, Char*) -> Int
alias BioMethodGets = (Bio*, Char*, Int) -> Int
alias BioMethodCtrl = (Bio*, Int, Long, Void*) -> Long
alias BioMethodCreate = Bio* -> Int
alias BioMethodDestroy = Bio* -> Int
alias BioMethodCallbackCtrl = (Bio*, Int, Void*) -> Long

type BioMethod = Void

fun BIO_new(BioMethod*) : Bio*
Expand All @@ -120,14 +109,21 @@ lib LibCrypto
fun BIO_set_shutdown(Bio*, Int)

fun BIO_meth_new(Int, Char*) : BioMethod*
fun BIO_meth_set_read(BioMethod*, BioMethodReadOld)
fun BIO_meth_set_write(BioMethod*, BioMethodWriteOld)
fun BIO_meth_set_puts(BioMethod*, BioMethodPuts)
fun BIO_meth_set_gets(BioMethod*, BioMethodGets)
fun BIO_meth_set_ctrl(BioMethod*, BioMethodCtrl)
fun BIO_meth_set_create(BioMethod*, BioMethodCreate)
fun BIO_meth_set_destroy(BioMethod*, BioMethodDestroy)
fun BIO_meth_set_callback_ctrl(BioMethod*, BioMethodCallbackCtrl)
fun BIO_meth_set_read(BioMethod*, (Bio*, Char*, Int) -> Int)
fun BIO_meth_set_write(BioMethod*, (Bio*, Char*, Int) -> Int)

{% unless compare_versions(LIBRESSL_VERSION, "0.0.0") > 0 %}
# LibreSSL doesn't support the _ex functions
fun BIO_meth_set_read_ex(BioMethod*, (Bio*, Char*, SizeT, SizeT*) -> Int)
fun BIO_meth_set_write_ex(BioMethod*, (Bio*, Char*, SizeT, SizeT*) -> Int)
{% end %}

fun BIO_meth_set_puts(BioMethod*, (Bio*, Char*) -> Int)
fun BIO_meth_set_gets(BioMethod*, (Bio*, Char*, Int) -> Int)
fun BIO_meth_set_ctrl(BioMethod*, (Bio*, Int, Long, Void*) -> Long)
fun BIO_meth_set_create(BioMethod*, (Bio*) -> Int)
fun BIO_meth_set_destroy(BioMethod*, (Bio*) -> Int)
fun BIO_meth_set_callback_ctrl(BioMethod*, (Bio*, Int, Void*) -> Long)

fun sha1 = SHA1(data : Char*, length : SizeT, md : Char*) : Char*

Expand Down
Loading