Skip to content

Commit

Permalink
Enable strict HTTPS validation by default
Browse files Browse the repository at this point in the history
If a user enables 'ssl = true' for a provider strict HTTPS certificate
validation is now enabled by default.  With 'secure-ssl = false' this
can be disabled.

Signed-off-by: Joachim Nilsson <troglobit@gmail.com>
  • Loading branch information
troglobit committed Jun 5, 2016
1 parent d3faa40 commit 52b256b
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 10 deletions.
2 changes: 2 additions & 0 deletions ChangeLog.md
Expand Up @@ -34,6 +34,8 @@ All notable changes to the project are documented in this file.
- Removed old compatibility symlinks and other required GNU specific
files, we now distribute and install README.md and ChangeLog.md
- Reorganized SSL code, split `ssl.c` into `openssl.c` and `gnutls.c`
- Strict HTTPS certificate validation is now default. To disable this
use `strict-ssl = false` in the .conf file.

### Fixes
- Fix issue #61: Add HTTPS certificate validation for OpenSSL/LibreSSL
Expand Down
2 changes: 2 additions & 0 deletions include/http.h
Expand Up @@ -74,6 +74,8 @@ typedef struct {
char status_desc[256];
} http_trans_t;

extern int secure_ssl;

int http_construct (http_t *client);
int http_destruct (http_t *client, int num);

Expand Down
10 changes: 10 additions & 0 deletions man/inadyn.conf.5
Expand Up @@ -93,6 +93,16 @@ How often the IP is checked, in seconds. Default: apx 1 minute. Max: 10 days.
.It Cm forced-update = SEC
How often the IP should be updated even if it is not changed. The time
should be given in seconds. Default is equal to 30 days.
.It Cm secure-ssl = < true | false >
If the HTTPS certificate validation fails for a provider
.Nm inadyn
aborts the DDNS update before sending any credentials. When this
setting is disabled, i.e.
.Ar false ,
then
.Nm inadyn
will only issue a warning. By default this setting is enabled, because
security matters.
.It Cm custom randomUserID {}
.It Cm provider email@ddns-service.tld[:ID] {}
The custom{} and provider{} sections are very similar, except that the
Expand Down
2 changes: 2 additions & 0 deletions src/conf.c
Expand Up @@ -411,6 +411,7 @@ cfg_t *conf_parse_file(char *file, ddns_t *ctx)
cfg_opt_t opts[] = {
CFG_BOOL("fake-address", cfg_false, CFGF_NONE),
CFG_BOOL("allow-ipv6", cfg_false, CFGF_NONE),
CFG_BOOL("secure-ssl", cfg_true, CFGF_NONE),
CFG_STR ("cache-dir", DEFAULT_CACHE_DIR, CFGF_NONE),
CFG_INT ("period", DDNS_DEFAULT_PERIOD, CFGF_NONE),
CFG_INT ("iterations", DDNS_DEFAULT_ITERATIONS, CFGF_NONE),
Expand Down Expand Up @@ -465,6 +466,7 @@ cfg_t *conf_parse_file(char *file, ddns_t *ctx)
if (!iface)
iface = cfg_getstr(cfg, "iface");
allow_ipv6 = cfg_getbool(cfg, "allow-ipv6");
secure_ssl = cfg_getbool(cfg, "secure-ssl");

for (i = 0; i < cfg_size(cfg, "provider"); i++)
ret |= create_provider(cfg_getnsec(cfg, "provider", i), 0);
Expand Down
19 changes: 12 additions & 7 deletions src/gnutls.c
Expand Up @@ -47,7 +47,7 @@ static int verify_certificate_callback(gnutls_session_t session)
ret = gnutls_certificate_verify_peers2(session, &status);
if (ret < 0) {
logit(LOG_ERR, "Failed verifying certificate peers.");
return GNUTLS_E_CERTIFICATE_ERROR;
goto error;
}

if (status & GNUTLS_CERT_SIGNER_NOT_FOUND)
Expand All @@ -64,7 +64,7 @@ static int verify_certificate_callback(gnutls_session_t session)

if (status & GNUTLS_CERT_INVALID) {
logit(LOG_ERR, "The certificate is not trusted.");
return GNUTLS_E_CERTIFICATE_ERROR;
goto error;
}

/* Up to here the process is the same for X.509 certificates and
Expand All @@ -73,35 +73,40 @@ static int verify_certificate_callback(gnutls_session_t session)
*/
if (gnutls_certificate_type_get(session) != GNUTLS_CRT_X509) {
logit(LOG_ERR, "Not a valid X.509 certificate");
return GNUTLS_E_CERTIFICATE_ERROR;
goto error;
}

if (gnutls_x509_crt_init(&cert) < 0) {
logit(LOG_ERR, "Failed init of X.509 cert engine");
return GNUTLS_E_CERTIFICATE_ERROR;
goto error;
}

cert_list = gnutls_certificate_get_peers(session, &cert_list_size);
if (cert_list == NULL) {
logit(LOG_ERR, "No certificate was found!");
return GNUTLS_E_CERTIFICATE_ERROR;
goto error;
}

if (gnutls_x509_crt_import(cert, &cert_list[0], GNUTLS_X509_FMT_DER) < 0) {
logit(LOG_ERR, "Error while parsing certificate.");
return GNUTLS_E_CERTIFICATE_ERROR;
goto error;
}


if (!gnutls_x509_crt_check_hostname(cert, hostname)) {
logit(LOG_ERR, "The certificate's owner does not match the hostname '%s'", hostname);
return GNUTLS_E_CERTIFICATE_ERROR;
goto error;
}

gnutls_x509_crt_deinit(cert);

/* notify gnutls to continue handshake normally */
logit(LOG_DEBUG, "Certificate OK");
return 0;

error:
if (secure_ssl)
return GNUTLS_E_CERTIFICATE_ERROR;

return 0;
}
Expand Down
1 change: 1 addition & 0 deletions src/main.c
Expand Up @@ -38,6 +38,7 @@ int ignore_errors = 0;
int startup_delay = DDNS_DEFAULT_STARTUP_SLEEP;
int use_syslog = 1;
int allow_ipv6 = 0;
int secure_ssl = 1; /* Strict cert validation by default */
char *iface = NULL;
char *config = NULL;
char *cache_dir = NULL;
Expand Down
6 changes: 3 additions & 3 deletions src/openssl.c
Expand Up @@ -75,10 +75,10 @@ static int verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
logit(LOG_ERR, "issuer= %s", buf);
}

if (1) //always_continue)
return 1;
if (secure_ssl)
return preverify_ok;

return preverify_ok;
return 1;
}

int ssl_open(http_t *client, char *msg)
Expand Down

0 comments on commit 52b256b

Please sign in to comment.