[Asterisk-code-review] res pjsip: Add existence and readablity checks for tls rela... (asterisk[master])
Joshua Colp
asteriskteam at digium.com
Thu Dec 10 07:13:40 CST 2015
Joshua Colp has submitted this change and it was merged.
Change subject: res_pjsip: Add existence and readablity checks for tls related files
......................................................................
res_pjsip: Add existence and readablity checks for tls related files
Both transport and endpoint now check for the existence and readability
of tls certificate and key files before passing them on to pjproject.
This will cause the object to not load rather than waiting for pjproject
to discover that there's a problem when a session is attempted.
NOTE: chan_sip also uses ast_rtp_dtls_cfg_parse but it's located
in build_peer which is gigantic and I didn't want to disturb it.
Error messages will emit but it won't interrupt chan_sip loading.
ASTERISK-25618 #close
Change-Id: Ie43f2c1d653ac1fda6a6f6faecb7c2ebadaf47c9
Reported-by: George Joseph
Tested-by: George Joseph
---
M include/asterisk/utils.h
M main/rtp_engine.c
M main/utils.c
M res/res_pjsip/config_transport.c
M res/res_pjsip/pjsip_configuration.c
5 files changed, 73 insertions(+), 1 deletion(-)
Approvals:
Anonymous Coward #1000019: Verified
Joshua Colp: Looks good to me, approved
diff --git a/include/asterisk/utils.h b/include/asterisk/utils.h
index 664e347..832500c 100644
--- a/include/asterisk/utils.h
+++ b/include/asterisk/utils.h
@@ -1089,4 +1089,14 @@
*/
int ast_crypt_validate(const char *key, const char *expected);
+/*
+ * \brief Test that a file exists and is readable by the effective user.
+ * \since 13.7.0
+ *
+ * \param filename File to test.
+ * \return True (non-zero) if the file exists and is readable.
+ * \return False (zero) if the file either doesn't exists or is not readable.
+ */
+int ast_file_is_readable(const char *filename);
+
#endif /* _ASTERISK_UTILS_H */
diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 3290909..24e56b4 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -2118,18 +2118,34 @@
}
} else if (!strcasecmp(name, "dtlscertfile")) {
ast_free(dtls_cfg->certfile);
+ if (!ast_file_is_readable(value)) {
+ ast_log(LOG_ERROR, "%s file %s does not exist or is not readable\n", name, value);
+ return -1;
+ }
dtls_cfg->certfile = ast_strdup(value);
} else if (!strcasecmp(name, "dtlsprivatekey")) {
ast_free(dtls_cfg->pvtfile);
+ if (!ast_file_is_readable(value)) {
+ ast_log(LOG_ERROR, "%s file %s does not exist or is not readable\n", name, value);
+ return -1;
+ }
dtls_cfg->pvtfile = ast_strdup(value);
} else if (!strcasecmp(name, "dtlscipher")) {
ast_free(dtls_cfg->cipher);
dtls_cfg->cipher = ast_strdup(value);
} else if (!strcasecmp(name, "dtlscafile")) {
ast_free(dtls_cfg->cafile);
+ if (!ast_file_is_readable(value)) {
+ ast_log(LOG_ERROR, "%s file %s does not exist or is not readable\n", name, value);
+ return -1;
+ }
dtls_cfg->cafile = ast_strdup(value);
} else if (!strcasecmp(name, "dtlscapath") || !strcasecmp(name, "dtlscadir")) {
ast_free(dtls_cfg->capath);
+ if (!ast_file_is_readable(value)) {
+ ast_log(LOG_ERROR, "%s file %s does not exist or is not readable\n", name, value);
+ return -1;
+ }
dtls_cfg->capath = ast_strdup(value);
} else if (!strcasecmp(name, "dtlssetup")) {
if (!strcasecmp(value, "active")) {
diff --git a/main/utils.c b/main/utils.c
index ba1a07c..74932b8 100644
--- a/main/utils.c
+++ b/main/utils.c
@@ -2927,3 +2927,20 @@
{
return memcmp(eid1, eid2, sizeof(*eid1));
}
+
+int ast_file_is_readable(const char *filename)
+{
+#if defined(HAVE_EACCESS) || defined(HAVE_EUIDACCESS)
+#if defined(HAVE_EUIDACCESS) && !defined(HAVE_EACCESS)
+#define eaccess euidaccess
+#endif
+ return eaccess(filename, R_OK) == 0;
+#else
+ int fd = open(filename, O_RDONLY | O_NONBLOCK);
+ if (fd < 0) {
+ return 0;
+ }
+ close(fd);
+ return 1;
+#endif
+}
diff --git a/res/res_pjsip/config_transport.c b/res/res_pjsip/config_transport.c
index e2f0c7f..d8ece15 100644
--- a/res/res_pjsip/config_transport.c
+++ b/res/res_pjsip/config_transport.c
@@ -27,6 +27,7 @@
#include "asterisk/astobj2.h"
#include "asterisk/sorcery.h"
#include "asterisk/acl.h"
+#include "asterisk/utils.h"
#include "include/res_pjsip_private.h"
#include "asterisk/http_websocket.h"
@@ -224,8 +225,22 @@
ast_sorcery_object_get_id(obj));
return -1;
}
+ if (!ast_strlen_zero(transport->ca_list_file)) {
+ if (!ast_file_is_readable(transport->ca_list_file)) {
+ ast_log(LOG_ERROR, "Transport: %s: ca_list_file %s is either missing or not readable\n",
+ ast_sorcery_object_get_id(obj), transport->ca_list_file);
+ return -1;
+ }
+ }
transport->tls.ca_list_file = pj_str((char*)transport->ca_list_file);
#ifdef HAVE_PJ_SSL_CERT_LOAD_FROM_FILES2
+ if (!ast_strlen_zero(transport->ca_list_path)) {
+ if (!ast_file_is_readable(transport->ca_list_path)) {
+ ast_log(LOG_ERROR, "Transport: %s: ca_list_path %s is either missing or not readable\n",
+ ast_sorcery_object_get_id(obj), transport->ca_list_path);
+ return -1;
+ }
+ }
transport->tls.ca_list_path = pj_str((char*)transport->ca_list_path);
#else
if (!ast_strlen_zero(transport->ca_list_path)) {
@@ -233,7 +248,21 @@
"support the 'ca_list_path' option. Please upgrade to version 2.4 or later.\n");
}
#endif
+ if (!ast_strlen_zero(transport->cert_file)) {
+ if (!ast_file_is_readable(transport->cert_file)) {
+ ast_log(LOG_ERROR, "Transport: %s: cert_file %s is either missing or not readable\n",
+ ast_sorcery_object_get_id(obj), transport->cert_file);
+ return -1;
+ }
+ }
transport->tls.cert_file = pj_str((char*)transport->cert_file);
+ if (!ast_strlen_zero(transport->privkey_file)) {
+ if (!ast_file_is_readable(transport->privkey_file)) {
+ ast_log(LOG_ERROR, "Transport: %s: privkey_file %s is either missing or not readable\n",
+ ast_sorcery_object_get_id(obj), transport->privkey_file);
+ return -1;
+ }
+ }
transport->tls.privkey_file = pj_str((char*)transport->privkey_file);
transport->tls.password = pj_str((char*)transport->password);
set_qos(transport, &transport->tls.qos_params);
diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index 3bb0869..72f896a 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -680,7 +680,7 @@
endpoint->media.rtp.encryption = AST_SIP_MEDIA_ENCRYPT_SDES;
} else if (!strcasecmp("dtls", var->value)) {
endpoint->media.rtp.encryption = AST_SIP_MEDIA_ENCRYPT_DTLS;
- ast_rtp_dtls_cfg_parse(&endpoint->media.rtp.dtls_cfg, "dtlsenable", "yes");
+ return ast_rtp_dtls_cfg_parse(&endpoint->media.rtp.dtls_cfg, "dtlsenable", "yes");
} else {
return -1;
}
--
To view, visit https://gerrit.asterisk.org/1789
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie43f2c1d653ac1fda6a6f6faecb7c2ebadaf47c9
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
More information about the asterisk-code-review
mailing list