Commit a17755c6 authored by Guillaume Roguez's avatar Guillaume Roguez Committed by Andreas Traczyk

Fix DhParams copy-assignment and copy-constructor

DhParams copy-assignement was implemented as non-op operator.
This is weird for a copy to do nothing!
No intends are indicated to help.
Seems only done to help MSVC compiler, but this is WRONG
and cause a SECURITY FAILURE as the DhParam is not set
as supposed to be.
Consequence observed on UWP daemon: the anonymous connection
cannot be done and the certificate is always send in cleartext.

The fix consists in using the dedicated gnutls API
to copy correctly the internal dh_params C structure.
This is used to implement copy-assignement and copy-constructor
methods
Reviewed-by: Andreas Traczyk's avatarAndreas Traczyk <andreas.traczyk@savoirfairelinux.com>
Change-Id: I49d3a3fd2c0a2ed5fde0bd251efbad589c830c70
parent 5b5fe54b
......@@ -82,6 +82,25 @@ DhParams::DhParams(const std::vector<uint8_t>& data)
throw std::runtime_error(std::string("Error importing DH params: ") + gnutls_strerror(ret_pem) + " " + gnutls_strerror(ret_der));
}
DhParams&
DhParams::operator=(const DhParams& other)
{
if (not params_) {
// We need a valid DH params pointer for the copy
gnutls_dh_params_t new_params_;
auto err = gnutls_dh_params_init(&new_params_);
if (err != GNUTLS_E_SUCCESS)
throw std::runtime_error(std::string("Error initializing DH params: ") + gnutls_strerror(err));
params_.reset(new_params_);
}
auto err = gnutls_dh_params_cpy(params_.get(), other.get());
if (err != GNUTLS_E_SUCCESS)
throw std::runtime_error(std::string("Error copying DH params: ") + gnutls_strerror(err));
return *this;
}
std::vector<uint8_t>
DhParams::serialize() const
{
......
......@@ -22,6 +22,7 @@
#pragma once
#include "threadloop.h"
#include "noncopyable.h"
#include <gnutls/gnutls.h>
#include <gnutls/dtls.h>
......@@ -41,6 +42,7 @@
#include <atomic>
#include <iterator>
#include <array>
#include <stdexcept>
namespace ring {
class IceTransport;
......@@ -71,13 +73,18 @@ class DhParams {
public:
DhParams() = default;
DhParams(DhParams&&) = default;
DhParams& operator =(const DhParams& other){
return *this;
DhParams(const DhParams& other) {
*this = other;
}
/** Take ownership of gnutls_dh_params */
explicit DhParams(gnutls_dh_params_t p) : params_(p, gnutls_dh_params_deinit) {};
DhParams& operator=(DhParams&& other) = default;
DhParams& operator=(const DhParams& other);
/// \brief Construct by taking ownership of given gnutls DH params
///
/// User should not call gnutls_dh_params_deinit on given \a raw_params.
/// The object is stolen and its live is manager by our object.
explicit DhParams(gnutls_dh_params_t p) : params_ {p, gnutls_dh_params_deinit} {}
/** Deserialize DER or PEM encoded DH-params */
DhParams(const std::vector<uint8_t>& data);
......@@ -99,7 +106,7 @@ public:
static DhParams generate();
private:
std::unique_ptr<gnutls_dh_params_int, decltype(gnutls_dh_params_deinit)&> params_ {nullptr, gnutls_dh_params_deinit};
std::unique_ptr<gnutls_dh_params_int, decltype(gnutls_dh_params_deinit)*> params_ {nullptr, gnutls_dh_params_deinit};
};
struct TlsParams {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment