[Asterisk-code-review] tcptls: Log remote address on TLS setup failure (asterisk[13])
Sean Bright
asteriskteam at digium.com
Fri Sep 1 11:20:34 CDT 2017
Sean Bright has uploaded this change for review. ( https://gerrit.asterisk.org/6374
Change subject: tcptls: Log remote address on TLS setup failure
......................................................................
tcptls: Log remote address on TLS setup failure
Also send to the security log if it is configured.
ASTERISK-26006 #close
Reported by: Oleksandr Natalenko
Change-Id: Id2d3ed3436fc6f38a7165c83c84e85a15d6978be
---
M include/asterisk/event_defs.h
M include/asterisk/security_events_defs.h
M main/event.c
M main/security_events.c
M main/tcptls.c
5 files changed, 98 insertions(+), 15 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/74/6374/1
diff --git a/include/asterisk/event_defs.h b/include/asterisk/event_defs.h
index 2d5c75a..cc58074 100644
--- a/include/asterisk/event_defs.h
+++ b/include/asterisk/event_defs.h
@@ -311,8 +311,11 @@
* Payload type: UINT
*/
AST_EVENT_IE_NODE_ID = 0x003e,
+
+ AST_EVENT_IE_TLS_ERROR = 0x003f,
+
/*! \brief Must be the last IE value +1 */
- AST_EVENT_IE_TOTAL = 0x003f,
+ AST_EVENT_IE_TOTAL = 0x0040,
};
/*!
diff --git a/include/asterisk/security_events_defs.h b/include/asterisk/security_events_defs.h
index 30a7136..6e7d6ad 100644
--- a/include/asterisk/security_events_defs.h
+++ b/include/asterisk/security_events_defs.h
@@ -116,6 +116,10 @@
*/
AST_SECURITY_EVENT_INVAL_TRANSPORT,
/*!
+ * \brief A TLS error occurred.
+ */
+ AST_SECURITY_EVENT_TLS_FAILURE,
+ /*!
* \brief This _must_ stay at the end.
*/
AST_SECURITY_EVENT_NUM_TYPES
@@ -505,15 +509,15 @@
* \brief Attempt to contact peer on invalid transport
*/
struct ast_security_event_inval_transport {
- /*!
- * \brief Event descriptor version
- * \note This _must_ be changed if this event descriptor is changed.
- */
- #define AST_SECURITY_EVENT_INVAL_TRANSPORT_VERSION 1
- /*!
- * \brief Common security event descriptor elements
- * \note Account ID required
- */
+ /*!
+ * \brief Event descriptor version
+ * \note This _must_ be changed if this event descriptor is changed.
+ */
+ #define AST_SECURITY_EVENT_INVAL_TRANSPORT_VERSION 1
+ /*!
+ * \brief Common security event descriptor elements
+ * \note Account ID required
+ */
struct ast_security_event_common common;
/*!
* \brief Attempted transport
@@ -522,6 +526,26 @@
const char *transport;
};
+/*!
+ * \brief An error occurred during a TLS handshake
+ */
+struct ast_security_event_tls_failure {
+ /*!
+ * \brief Event descriptor version
+ * \note This _must_ be changed if this event descriptor is changed.
+ */
+ #define AST_SECURITY_EVENT_TLS_FAILURE_VERSION 1
+ /*!
+ * \brief Common security event descriptor elements
+ */
+ struct ast_security_event_common common;
+
+ /*!
+ * \brief The error message, if any
+ */
+ const char *error_message;
+};
+
#if defined(__cplusplus) || defined(c_plusplus)
}
#endif
diff --git a/main/event.c b/main/event.c
index 019dfca..505285e 100644
--- a/main/event.c
+++ b/main/event.c
@@ -191,6 +191,7 @@
[AST_EVENT_IE_PRESENCE_STATE] = { AST_EVENT_IE_PLTYPE_UINT, "PresenceState" },
[AST_EVENT_IE_PRESENCE_SUBTYPE] = { AST_EVENT_IE_PLTYPE_STR, "PresenceSubtype" },
[AST_EVENT_IE_PRESENCE_MESSAGE] = { AST_EVENT_IE_PLTYPE_STR, "PresenceMessage" },
+ [AST_EVENT_IE_TLS_ERROR] = { AST_EVENT_IE_PLTYPE_STR, "TLSError" },
};
const char *ast_event_get_type_name(const struct ast_event *event)
diff --git a/main/security_events.c b/main/security_events.c
index d549d62..74a884e 100644
--- a/main/security_events.c
+++ b/main/security_events.c
@@ -858,6 +858,26 @@
},
},
+[AST_SECURITY_EVENT_TLS_FAILURE] = {
+ .name = "TLSFailure",
+ .version = AST_SECURITY_EVENT_TLS_FAILURE_VERSION,
+ .severity = AST_SECURITY_EVENT_SEVERITY_ERROR,
+ .required_ies = {
+ { AST_EVENT_IE_EVENT_TV, 0 },
+ { AST_EVENT_IE_SEVERITY, 0 },
+ { AST_EVENT_IE_SERVICE, SEC_EVT_FIELD(common, service) },
+ { AST_EVENT_IE_EVENT_VERSION, SEC_EVT_FIELD(common, version) },
+ { AST_EVENT_IE_SESSION_ID, SEC_EVT_FIELD(common, session_id) },
+ { AST_EVENT_IE_LOCAL_ADDR, SEC_EVT_FIELD(common, local_addr) },
+ { AST_EVENT_IE_REMOTE_ADDR, SEC_EVT_FIELD(common, remote_addr) },
+ { AST_EVENT_IE_TLS_ERROR, SEC_EVT_FIELD(tls_failure, error_message) },
+ { AST_EVENT_IE_END, 0 }
+ },
+ .optional_ies = {
+ { AST_EVENT_IE_END, 0 }
+ },
+},
+
#undef SEC_EVT_FIELD
};
@@ -961,6 +981,7 @@
case AST_EVENT_IE_RECEIVED_CHALLENGE:
case AST_EVENT_IE_RECEIVED_HASH:
case AST_EVENT_IE_ATTEMPTED_TRANSPORT:
+ case AST_EVENT_IE_TLS_ERROR:
{
const char *str;
struct ast_json *json_string;
diff --git a/main/tcptls.c b/main/tcptls.c
index bc2d64b..0877b60 100644
--- a/main/tcptls.c
+++ b/main/tcptls.c
@@ -51,6 +51,7 @@
#include "asterisk/astobj2.h"
#include "asterisk/pbx.h"
#include "asterisk/app.h"
+#include "asterisk/security_events.h"
/*! ao2 object used for the FILE stream fopencookie()/funopen() cookie. */
struct ast_tcptls_stream {
@@ -639,6 +640,31 @@
#endif
+static void tcptls_report_ssl_error(const struct ast_tcptls_session_instance *session, const char *error_message)
+{
+ char session_id[32];
+
+ struct ast_security_event_tls_failure failed_tls_event = {
+ .common.event_type = AST_SECURITY_EVENT_TLS_FAILURE,
+ .common.version = AST_SECURITY_EVENT_TLS_FAILURE_VERSION,
+ .common.service = session->parent->name,
+ .common.local_addr = {
+ .addr = &session->parent->local_address,
+ .transport = AST_TRANSPORT_TLS
+ },
+ .common.remote_addr = {
+ .addr = &session->remote_address,
+ .transport = AST_TRANSPORT_TLS
+ },
+ .common.session_id = session_id,
+ .error_message = error_message,
+ };
+
+ snprintf(session_id, sizeof(session_id), "%p", session);
+
+ ast_security_event_report(AST_SEC_EVT(&failed_tls_event));
+}
+
/*! \brief
* creates a FILE * from the fd passed by the accept thread.
* This operation is potentially expensive (certificate verification),
@@ -686,14 +712,23 @@
}
}
#ifdef DO_SSL
- else if ( (tcptls_session->ssl = SSL_new(tcptls_session->parent->tls_cfg->ssl_ctx)) ) {
+ else if ((tcptls_session->ssl = SSL_new(tcptls_session->parent->tls_cfg->ssl_ctx))) {
SSL_set_fd(tcptls_session->ssl, tcptls_session->fd);
if ((ret = ssl_setup(tcptls_session->ssl)) <= 0) {
char err[256];
int sslerr = SSL_get_error(tcptls_session->ssl, ret);
+ const char *detailed = ssl_error_to_string(sslerr, ret);
- ast_log(LOG_ERROR, "Problem setting up ssl connection: %s, %s\n", ERR_error_string(sslerr, err),
- ssl_error_to_string(sslerr, ret));
+ ast_log(LOG_ERROR, "Problem setting up SSL connection for %s: %s, %s\n",
+ ast_sockaddr_stringify(&tcptls_session->remote_address),
+ ERR_error_string(sslerr, err),
+ detailed);
+
+ tcptls_report_ssl_error(tcptls_session, detailed);
+
+ ast_tcptls_close_session_file(tcptls_session);
+ ao2_ref(tcptls_session, -1);
+ return NULL;
} else if ((tcptls_session->f = tcptls_stream_fopen(tcptls_session->stream_cookie,
tcptls_session->ssl, tcptls_session->fd, -1))) {
if ((tcptls_session->client && !ast_test_flag(&tcptls_session->parent->tls_cfg->flags, AST_SSL_DONT_VERIFY_SERVER))
@@ -779,10 +814,9 @@
if (!tcptls_session->f) {
ast_tcptls_close_session_file(tcptls_session);
- ast_log(LOG_WARNING, "FILE * open failed!\n");
#ifndef DO_SSL
if (tcptls_session->parent->tls_cfg) {
- ast_log(LOG_ERROR, "Attempted a TLS connection without OpenSSL support. This will not work!\n");
+ ast_log(LOG_ERROR, "Attempted a TLS connection without OpenSSL support.\n");
}
#endif
ao2_ref(tcptls_session, -1);
--
To view, visit https://gerrit.asterisk.org/6374
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id2d3ed3436fc6f38a7165c83c84e85a15d6978be
Gerrit-Change-Number: 6374
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170901/80d74bc9/attachment-0001.html>
More information about the asterisk-code-review
mailing list