From 07b3e6fd748e05a40d6e53b0c92d0b74f97406b2 Mon Sep 17 00:00:00 2001 From: Simon Wilkinson Date: Tue, 18 Sep 2018 15:43:28 +0100 Subject: [PATCH] Allow zero-length encrypt IOVs in _krb5_evp_encrypt_iov_cts() The iovec encryption code doesn't handle 0 length iovecs correctly. Instead of just skipping them, _krb5_evp_encrypt_iov_cts() will spin on the 0 length iovec. Modify the _krb5_evp_iov_cursor_expand helper so that iovec expansion simply skips 0 length iovecs, and make _krb5_evp_iov_cursor_nextcrypt do the same. Original bug report and tests from Andrew Bartlett --- lib/gssapi/test_context.c | 41 ++++++++++++++++++++++++++++++++++++++- lib/krb5/crypto-evp.c | 8 +++++--- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/lib/gssapi/test_context.c b/lib/gssapi/test_context.c index 452d50b52..535461899 100644 --- a/lib/gssapi/test_context.c +++ b/lib/gssapi/test_context.c @@ -311,6 +311,7 @@ wrapunwrap(gss_ctx_id_t cctx, gss_ctx_id_t sctx, int flags, gss_OID mechoid) #define USE_HEADER_ONLY 2 #define USE_SIGN_ONLY 4 #define FORCE_IOV 8 +#define NO_DATA 16 static void wrapunwrap_iov(gss_ctx_id_t cctx, gss_ctx_id_t sctx, int flags, gss_OID mechoid) @@ -360,7 +361,11 @@ wrapunwrap_iov(gss_ctx_id_t cctx, gss_ctx_id_t sctx, int flags, gss_OID mechoid) iov[1].buffer.value = NULL; } iov[2].type = GSS_IOV_BUFFER_TYPE_DATA; - iov[2].buffer.length = token.length; + if (flags & NO_DATA) { + iov[2].buffer.length = 0; + } else { + iov[2].buffer.length = token.length; + } iov[2].buffer.value = token.data; if (trailer.length != 0) { iov[3].type = GSS_IOV_BUFFER_TYPE_SIGN_ONLY; @@ -1007,6 +1012,40 @@ main(int argc, char **argv) wrapunwrap_iov(cctx, sctx, USE_CONF|USE_HEADER_ONLY, actual_mech); wrapunwrap_iov(cctx, sctx, USE_CONF|USE_HEADER_ONLY|FORCE_IOV, actual_mech); + + wrapunwrap_iov(cctx, sctx, NO_DATA, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_HEADER_ONLY|FORCE_IOV, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_HEADER_ONLY, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_CONF, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_CONF|USE_HEADER_ONLY, actual_mech); + + wrapunwrap_iov(cctx, sctx, NO_DATA|FORCE_IOV, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_CONF|FORCE_IOV, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_HEADER_ONLY|FORCE_IOV, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_CONF|USE_HEADER_ONLY|FORCE_IOV, actual_mech); + + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_SIGN_ONLY|FORCE_IOV, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_CONF|USE_SIGN_ONLY|FORCE_IOV, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_CONF|USE_HEADER_ONLY|USE_SIGN_ONLY|FORCE_IOV, actual_mech); + + /* works */ + wrapunwrap_iov(cctx, sctx, NO_DATA, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|FORCE_IOV, actual_mech); + + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_CONF, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_CONF|FORCE_IOV, actual_mech); + + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_SIGN_ONLY, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_SIGN_ONLY|FORCE_IOV, actual_mech); + + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_CONF|USE_SIGN_ONLY, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_CONF|USE_SIGN_ONLY|FORCE_IOV, actual_mech); + + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_HEADER_ONLY, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_HEADER_ONLY|FORCE_IOV, actual_mech); + + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_CONF|USE_HEADER_ONLY, actual_mech); + wrapunwrap_iov(cctx, sctx, NO_DATA|USE_CONF|USE_HEADER_ONLY|FORCE_IOV, actual_mech); } if (aead_flag) { diff --git a/lib/krb5/crypto-evp.c b/lib/krb5/crypto-evp.c index 2cbb6abbb..a16b83cb0 100644 --- a/lib/krb5/crypto-evp.c +++ b/lib/krb5/crypto-evp.c @@ -220,8 +220,9 @@ _krb5_evp_iov_cursor_expand(struct _krb5_evp_iov_cursor *cursor) return; while (_krb5_evp_iov_should_encrypt(&cursor->iov[cursor->nextidx])) { - if ((char *)cursor->current.data + cursor->current.length - != cursor->iov[cursor->nextidx].data.data) { + if (cursor->iov[cursor->nextidx].data.length != 0 && + ((char *)cursor->current.data + cursor->current.length + != cursor->iov[cursor->nextidx].data.data)) { return; } cursor->current.length += cursor->iov[cursor->nextidx].data.length; @@ -237,7 +238,8 @@ static inline void _krb5_evp_iov_cursor_nextcrypt(struct _krb5_evp_iov_cursor *cursor) { for (; cursor->nextidx < cursor->niov; cursor->nextidx++) { - if (_krb5_evp_iov_should_encrypt(&cursor->iov[cursor->nextidx])) { + if (_krb5_evp_iov_should_encrypt(&cursor->iov[cursor->nextidx]) + && cursor->iov[cursor->nextidx].data.length != 0) { cursor->current = cursor->iov[cursor->nextidx].data; cursor->nextidx++; _krb5_evp_iov_cursor_expand(cursor);