Skip to content

Commit

Permalink
Fix: libcrmcommon: get DH prime bit length from GnuTLS API
Browse files Browse the repository at this point in the history
Previously, Pacemaker hard-coded a prime length of 1024 bits when generating
Diffie-Hellman parameters for a TLS server. This value was chosen in 2007,
but the ideal value increases over time.

The current best practice is to allow the client and server to negotiate
Diffie-Hellman parameters using a TLS extension (RFC 7919). However, we have to
support both older versions of GnuTLS that don't support the extension on our
side, and older Pacemaker versions that don't support the extension on the
other side.

We can improve the situation by querying the GnuTLS library for an appropriate
prime length, when the library supports that.

This also refactors the DH initialization code into a new library function,
and handles errors by logging and failing, rather than continuing with
insufficiently initialized parameters.
  • Loading branch information
kgaillot committed Sep 11, 2018
1 parent ef5e209 commit 875e2d1
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 5 deletions.
1 change: 1 addition & 0 deletions configure.ac
Expand Up @@ -1360,6 +1360,7 @@ dnl gnutls_priority_set_direct available since 2.1.7 (released 2007-11-29)
AC_CHECK_LIB(gnutls, gnutls_priority_set_direct)
if test "$ac_cv_lib_gnutls_gnutls_priority_set_direct" != ""; then
AC_CHECK_HEADERS(gnutls/gnutls.h)
AC_CHECK_FUNCS([gnutls_sec_param_to_pk_bits]) dnl since 2.12.0 (2011-03-24)
fi

dnl ========================================================================
Expand Down
6 changes: 3 additions & 3 deletions daemons/based/based_remote.c
Expand Up @@ -58,7 +58,6 @@ int init_remote_listener(int port, gboolean encrypted);
void cib_remote_connection_destroy(gpointer user_data);

#ifdef HAVE_GNUTLS_GNUTLS_H
# define DH_BITS 1024
gnutls_dh_params_t dh_params;
gnutls_anon_server_credentials_t anon_cred_s;
static void
Expand Down Expand Up @@ -109,8 +108,9 @@ init_remote_listener(int port, gboolean encrypted)
crm_gnutls_global_init();
/* gnutls_global_set_log_level (10); */
gnutls_global_set_log_function(debug_log);
gnutls_dh_params_init(&dh_params);
gnutls_dh_params_generate2(dh_params, DH_BITS);
if (pcmk__init_tls_dh(&dh_params) != GNUTLS_E_SUCCESS) {
return -1;
}
gnutls_anon_allocate_server_credentials(&anon_cred_s);
gnutls_anon_set_server_dh_params(anon_cred_s, dh_params);
#endif
Expand Down
5 changes: 3 additions & 2 deletions daemons/execd/remoted_tls.c
Expand Up @@ -319,8 +319,9 @@ lrmd_init_remote_tls_server()
crm_gnutls_global_init();
gnutls_global_set_log_function(debug_log);

gnutls_dh_params_init(&dh_params);
gnutls_dh_params_generate2(dh_params, 1024);
if (pcmk__init_tls_dh(&dh_params) != GNUTLS_E_SUCCESS) {
return -1;
}
gnutls_psk_allocate_server_credentials(&psk_cred_s);
gnutls_psk_set_server_credentials_function(psk_cred_s, lrmd_tls_server_key_cb);
gnutls_psk_set_server_dh_params(psk_cred_s, dh_params);
Expand Down
2 changes: 2 additions & 0 deletions include/crm/common/remote_internal.h
Expand Up @@ -31,6 +31,8 @@ void crm_sockaddr2str(void *sa, char *s);
gnutls_session_t *pcmk__new_tls_session(int csock, unsigned int conn_type,
gnutls_credentials_type_t cred_type,
void *credentials);
int pcmk__init_tls_dh(gnutls_dh_params_t *dh_params);

/*!
* \internal
* \brief Initiate the client handshake after establishing the tcp socket
Expand Down
53 changes: 53 additions & 0 deletions lib/common/remote.c
Expand Up @@ -223,6 +223,59 @@ pcmk__new_tls_session(int csock, unsigned int conn_type,
return NULL;
}

/*!
* \internal
* \brief Initialize Diffie-Hellman parameters for a TLS server
*
* \param[out] dh_params Parameter object to initialize
*
* \return GNUTLS_E_SUCCESS on success, GnuTLS error code on error
* \todo The current best practice is to allow the client and server to
* negotiate the Diffie-Hellman parameters via a TLS extension (RFC 7919).
* However, we have to support both older versions of GnuTLS (<3.6) that
* don't support the extension on our side, and older Pacemaker versions
* that don't support the extension on the other side. The next best
* practice would be to use a known good prime (see RFC 5114 section 2.2),
* possibly stored in a file distributed with Pacemaker.
*/
int
pcmk__init_tls_dh(gnutls_dh_params_t *dh_params)
{
int rc = GNUTLS_E_SUCCESS;
unsigned int dh_bits = 0;

rc = gnutls_dh_params_init(dh_params);
if (rc != GNUTLS_E_SUCCESS) {
goto error;
}

#ifdef HAVE_GNUTLS_SEC_PARAM_TO_PK_BITS
dh_bits = gnutls_sec_param_to_pk_bits(GNUTLS_PK_DH,
GNUTLS_SEC_PARAM_NORMAL);
if (dh_bits == 0) {
rc = GNUTLS_E_DH_PRIME_UNACCEPTABLE;
goto error;
}
#else
dh_bits = 1024;
#endif

crm_info("Generating Diffie-Hellman parameters with %u-bit prime for TLS",
dh_bits);
rc = gnutls_dh_params_generate2(*dh_params, dh_bits);
if (rc != GNUTLS_E_SUCCESS) {
goto error;
}

return rc;

error:
crm_err("Could not initialize Diffie-Hellman parameters for TLS: %s "
CRM_XS " rc=%d", gnutls_strerror(rc), rc);
CRM_ASSERT(rc == GNUTLS_E_SUCCESS);
return rc;
}

static int
crm_send_tls(gnutls_session_t * session, const char *buf, size_t len)
{
Expand Down

0 comments on commit 875e2d1

Please sign in to comment.