Attention is currently required from: flichtenheld, plaisthos.
Hello plaisthos, flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/774?usp=email
to review the following change.
Change subject: Improve data channel crypto error messages
......................................................................
Improve data channel crypto error messages
* Make decryption error messages better understandable.
* Increase verbosity level for authentication errors, because those can
be expected on bad connections.
Change-Id: I0fd48191babe4fe5c56f10eb3ba88182ffb075d1
Signed-off-by: Steffan Karger <[email protected]>
---
M src/openvpn/crypto.c
M src/openvpn/crypto.h
2 files changed, 12 insertions(+), 9 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/74/774/1
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 12ad0b9..064e59e 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -459,14 +459,14 @@
if (!cipher_ctx_update(ctx->cipher, BPTR(&work), &outlen, BPTR(buf),
data_len))
{
- CRYPT_ERROR("cipher update failed");
+ CRYPT_ERROR("packet decryption failed");
}
ASSERT(buf_inc_len(&work, outlen));
if (!cipher_ctx_final_check_tag(ctx->cipher, BPTR(&work) + outlen,
&outlen, tag_ptr, tag_size))
{
- CRYPT_ERROR("cipher final failed");
+ CRYPT_DROP("packet tag authentication failed");
}
ASSERT(buf_inc_len(&work, outlen));
@@ -538,7 +538,7 @@
/* Compare locally computed HMAC with packet HMAC */
if (memcmp_constant_time(local_hmac, BPTR(buf), hmac_len))
{
- CRYPT_ERROR("packet HMAC authentication failed");
+ CRYPT_DROP("packet HMAC authentication failed");
}
ASSERT(buf_advance(buf, hmac_len));
@@ -572,26 +572,26 @@
/* ctx->cipher was already initialized with key & keylen */
if (!cipher_ctx_reset(ctx->cipher, iv_buf))
{
- CRYPT_ERROR("cipher init failed");
+ CRYPT_ERROR("decrypt initialization failed");
}
/* Buffer overflow check (should never happen) */
if (!buf_safe(&work, buf->len +
cipher_ctx_block_size(ctx->cipher)))
{
- CRYPT_ERROR("potential buffer overflow");
+ CRYPT_ERROR("packet too big to decrypt");
}
/* Decrypt packet ID, payload */
if (!cipher_ctx_update(ctx->cipher, BPTR(&work), &outlen,
BPTR(buf), BLEN(buf)))
{
- CRYPT_ERROR("cipher update failed");
+ CRYPT_ERROR("packet decryption failed");
}
ASSERT(buf_inc_len(&work, outlen));
/* Flush the decryption buffer */
if (!cipher_ctx_final(ctx->cipher, BPTR(&work) + outlen, &outlen))
{
- CRYPT_ERROR("cipher final failed");
+ CRYPT_DROP("packet authentication failed, dropping.");
}
ASSERT(buf_inc_len(&work, outlen));
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 61184bc..d91de74 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -288,8 +288,11 @@
* security operation functions. */
};
-#define CRYPT_ERROR(format) \
- do { msg(D_CRYPT_ERRORS, "%s: " format, error_prefix); goto error_exit; }
while (false)
+#define CRYPT_ERROR_EXIT(flags, format) \
+ do { msg(flags, "%s: " format, error_prefix); goto error_exit; } while
(false)
+
+#define CRYPT_ERROR(format) CRYPT_ERROR_EXIT(D_CRYPT_ERRORS, format)
+#define CRYPT_DROP(format) CRYPT_ERROR_EXIT(D_MULTI_DROPPED, format)
/**
* Minimal IV length for AEAD mode ciphers (in bytes):
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/774?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0fd48191babe4fe5c56f10eb3ba88182ffb075d1
Gerrit-Change-Number: 774
Gerrit-PatchSet: 1
Gerrit-Owner: syzzer <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
