Page MenuHomeFreeBSD

opencrypto: Relax checks for zero-length payloads
AbandonedPublic

Authored by markj on Sep 24 2021, 6:26 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 13, 6:40 AM
Unknown Object (File)
Sun, Nov 10, 9:00 PM
Unknown Object (File)
Sun, Nov 10, 1:29 PM
Unknown Object (File)
Oct 6 2024, 5:52 AM
Unknown Object (File)
Oct 6 2024, 2:13 AM
Unknown Object (File)
Oct 5 2024, 7:06 PM
Unknown Object (File)
Oct 4 2024, 2:01 AM
Unknown Object (File)
Oct 3 2024, 11:47 PM
Subscribers

Details

Reviewers
jhb
Summary

In general it is permitted to submit crypto ops with zero-length
payloads. However, our handling of them is inconsistent. In
particular, crypto_op() checks for an input length of zero and returns
EINVAL in that case, but one can bypass this check by passing an inline
IV, resulting in a cryptop with payload_start != 0 && payload_length ==
0. cryptoaead_op() does not check for a length of zero at all.

In the aforementioned case where a crypto op has length zero but an
inline IV, the operation will trigger a KASSERT in crp_sanity() since
crp_payload_start != 0 && crp_payload_len == 0. Relax the assertion to
permit this. Remove the check in crypto_op(), reverting commit
e6d944d7c3eae828d8353abd414f634067ec6f7d.

Reported by: syzbot+d4b94fbd9a44b032f428@syzkaller.appspotmail.com

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 41725
Build 38614: arc lint + arc unit

Event Timeline

markj requested review of this revision.Sep 24 2021, 6:26 PM
sys/opencrypto/crypto.c
1299

Hmmm, I do think it's kind of odd to give a payload_start if payload_length is 0 fwiw. See for example the assertion about aad at 1266-1268. My intention was to require start fields to be 0 if the associated length was 0. Of course, that would require a special case in cryptodev for an empty payload to leave the start as 0. Hmmm.

sys/opencrypto/cryptodev.c
756

Hmm, I do think a payload of 0 should be an error if there isn't an associated MAC. Perhaps as a followup to my pending change we could change the == 0 case I moved into the CSP_CIPHER case further below to take into account the implicit IV?

sys/opencrypto/cryptodev.c
756

Isn't the problem broader than just the CSP_CIPHER case though?

sys/opencrypto/cryptodev.c
756

An empty payload is permitted for ETA and AEAD modes (you still generate a MAC/tag output even with an empty payload). DIGEST doesn't have a txform, so it's really just CIPHER mode for which this is relevant.