Skip to content

BIO_read_ex does not distinguish EOF from failure #8208

@davidben

Description

@davidben

OpenSSL 1.1.1 added BIO_read_ex and BIO_write_ex functions, along with corresponding functions at the SSL layer, so that things can be size_t-clean.

BIO_read_ex is not a suitable replacement for BIO_read because it loses the ability to distinguish EOF from failure. BIO_read did so via a 0 vs -1 return.

See this logic where a file BIO's BIO_read implementation uses ferror to decide whether to return 0 or -1.
https://github.com/openssl/openssl/blob/master/crypto/bio/bss_file.c#L145-L154

Or this logic where fd BIOs preserve whether read returned 0 or -1.
https://github.com/openssl/openssl/blob/master/crypto/bio/bss_fd.c#L121

For BIO_read_ex, one answer would be to return a "successful" read of zero bytes. However, BIO_read_ex's documentation does not call this out (necessary so a caller knows to expect it), and indeed the implementation does not do this. Both 0 and -1 from an old-style BIO_read get mapped to the same thing.
https://github.com/openssl/openssl/blob/master/crypto/bio/bio_meth.c#L123-L128

This means that any BIO implementation which distinguishes these, such as fd or file BIOs above, cannot be ported to BIO_meth_set_read_ex because there is no way to retain that signal for existing BIO_read callers. Conversely, any BIO consumer that needs to distinguish EOF from error cannot be ported to BIO_read_ex. This includes many of the BIO_read calls within OpenSSL itself:
https://github.com/openssl/openssl/blob/master/apps/apps.c#L1704
https://github.com/openssl/openssl/blob/master/apps/dgst.c#L425
https://github.com/openssl/openssl/blob/master/apps/rsautl.c#L222
https://github.com/openssl/openssl/blob/master/crypto/asn1/a_d2i_fp.c#L123
https://github.com/openssl/openssl/blob/master/demos/bio/saccept.c#L97
https://github.com/openssl/openssl/blob/master/demos/bio/sconnect.c#L101
https://github.com/openssl/openssl/blob/master/demos/bio/server-arg.c#L120
https://github.com/openssl/openssl/blob/master/demos/bio/server-conf.c#L112

The distinction is messier for SSL_read_ex, since it has SSL_get_error, but I'll note that SSL_ERROR_ZERO_RETURN is really just a clean EOF. Though EOFs for TLS are a little messy because there's a clean close_notify and there's unclear transport EOFs which, while technically an unclean shutdown, are in practice treated as "clean" in many uses of TLS due to the ecosystem failing to send close_notify. In SSL_read, this is signaled via some messy combination of SSL_get_error and 0 vs -1.

Metadata

Metadata

Assignees

Labels

backlog fixThe issue was closed as part of the backlog reduction initiative.branch: masterApplies to master branchtriaged: featureThe issue/pr requests/adds a feature

Projects

Status

Done

Relationships

None yet

Development

No branches or pull requests

Issue actions