Commit 7c00b364 authored by johan's avatar johan

Fix OPk id generation

parent c90b270a
......@@ -76,8 +76,8 @@ class bctbx_RNG : public RNG {
uint32_t randomize() override {
std::array<uint8_t, 4> buffer;
bctbx_rng_get(m_context, buffer.data(), buffer.size());
buffer[0] = 0; // just to be sure it's properly set to 0
bctbx_rng_get(m_context, buffer.data(), buffer.size());
// buffer[0] is shifted by 23 instead of 24 to keep the MSb to 0.
// as we must (see RNG interface definition) keep the uint32_t MSb set to 0
return (static_cast<uint32_t>(buffer[0])<<23 | static_cast<uint32_t>(buffer[1])<<16 | static_cast<uint32_t>(buffer[2])<<8 | static_cast<uint32_t>(buffer[3]));
......
......@@ -19,6 +19,7 @@
#include <bctoolbox/exception.hh>
#include <soci/soci.h>
#include <set>
#include "lime_log.hpp"
#include "lime/lime.hpp"
......@@ -897,9 +898,32 @@ template <typename Curve>
void Lime<Curve>::X3DH_generate_OPks(std::vector<X<Curve, lime::Xtype::publicKey>> &publicOPks, std::vector<uint32_t> &OPk_ids, const uint16_t OPk_number) {
// make room for OPk and OPk ids
OPk_ids.clear();
publicOPks.clear();
publicOPks.reserve(OPk_number);
OPk_ids.reserve(OPk_number);
// OPkIds must be random but unique, prepare them before insertion
std::set<uint32_t> activeOPkIds{};
// fetch existing OPk ids from DB (OPKid is unique on all users, so really get them all, do not restrict with m_db_Uid)
rowset<row> rs = (m_localStorage->sql.prepare << "SELECT OPKid FROM X3DH_OPK");
for (const auto &r : rs) {
auto OPk_id = static_cast<uint32_t>(r.get<int>(0));
activeOPkIds.insert(OPk_id);
}
// we must create OPk_number new Ids
while (OPk_ids.size() < OPk_number){
// Generate a random OPk Id
// Sqlite doesn't really support unsigned value, the randomize function makes sure that the MSbit is set to 0 to not fall into strange bugs with that
uint32_t OPk_id = m_RNG->randomize();
if (activeOPkIds.insert(OPk_id).second) { // if this one wasn't in the set
OPk_ids.push_back(OPk_id);
}
}
// Prepare DB statement
transaction tr(m_localStorage->sql);
blob OPk(m_localStorage->sql);
......@@ -910,24 +934,22 @@ void Lime<Curve>::X3DH_generate_OPks(std::vector<X<Curve, lime::Xtype::publicKey
auto DH = make_keyExchange<Curve>();
try {
for (uint16_t i=0; i<OPk_number; i++) {
for (auto id : OPk_ids) { // loop on all ids
// Generate a new ECDH Key pair
DH->createKeyPair(m_RNG);
// Generate a random OPk Id
// Sqlite doesn't really support unsigned value, the randomize function makes sure that the MSbit is set to 0 to not fall into strange bugs with that
OPk_id = m_RNG->randomize();
// Insert in DB: store Public Key || Private Key
OPk.write(0, (const char *)(DH->get_selfPublic().data()), X<Curve, lime::Xtype::publicKey>::ssize());
OPk.write(X<Curve, lime::Xtype::publicKey>::ssize(), (const char *)(DH->get_secret().data()), X<Curve, lime::Xtype::privateKey>::ssize());
OPk_id = id; // store also the key id
st.execute(true);
// set in output vectors
// set in output vector
publicOPks.emplace_back(DH->get_selfPublic());
OPk_ids.push_back(OPk_id);
}
} catch (exception &e) {
OPk_ids.clear();
publicOPks.clear();
tr.rollback();
throw BCTBX_EXCEPTION << "OPK insertion in DB failed. DB backend says : "<<e.what();
}
......
......@@ -317,8 +317,6 @@ static void signAndVerify(void) {
static void hashMac_KDF_bench(uint64_t runTime_ms, size_t IKMsize) {
size_t batch_size = 500;
/* Generate random input and info */
auto rng_source = make_RNG();
/* input lenght is the same used by X3DH */
std::vector<uint8_t> IKM(IKMsize, 0);
lime_tester::randomize(IKM.data(), IKM.size());
......@@ -611,13 +609,64 @@ static void AEAD(void) {
BC_ASSERT_TRUE(plain==pattern_plain);
}
/**
* @brief Test the Random Number Generator used to generate keys Id
* The Id generation gives a 31 bits unsigned integer, is used when RNG->randomize function returns an uint32_t value
* This test doesn't really test the quality of the RNG, just to point obvious mistakes in the implementation
* It computes the mean and standard deviation of the generated number
* Being a discrete uniform distribution, standard deviation is supposed to be
* (max-min)/sqrt(12), in our case (0x7FFFFFFF - 0)/sqrt(12).
* To get an more readable perspective, mean and sqrt are divided by 0x7FFFFFFF
* and result are tested agains 0.5 and 1/sqrt(12)
*
*/
static void RNG_test(void) {
constexpr size_t NB_INT31_TESTED=10000;
/* init the RNG */
auto rng_source = make_RNG();
uint32_t random_uint31 = rng_source->randomize();
long double m0=static_cast<long double>(random_uint31),
s0=0.0,
m1=0.0;
size_t i=2;
for (; i<=NB_INT31_TESTED; i++) {
random_uint31 = rng_source->randomize();
auto random_double = static_cast<long double>(random_uint31);
m1 = m0 + (random_double - m0)/static_cast<long double>(i);
s0 += (random_double - m0)*(random_double - m1);
m0 = m1;
}
/* mean/0x7FFFFFFF shall be around 0.5 */
m0 = m0/(long double)0x7FFFFFFF;
if (m0>0.49 && m0<0.51) {
BC_PASS("RNG mean value ok");
} else {
LIME_LOGE << NB_INT31_TESTED << " 31 bits unsigned integers generated Mean " << m0 <<" expected around 0.5" <<std::endl;
BC_FAIL("RNG mean value incorrect");
}
/* sigma value/0x7FFFFFFF shall be around 1/sqrt(12)[0.288675]*/
s0 = std::sqrt(s0/i-1)/(long double)0x7FFFFFFF;
if (s0>0.278 && s0<0.299) {
BC_PASS("RNG sigma value ok");
} else {
LIME_LOGE << NB_INT31_TESTED << " 31 bits unsigned integers generated Sigma " << s0 <<" expected around 0.288675"<<std::endl;
BC_FAIL("RNG sigma value incorrect");
}
LIME_LOGD << NB_INT31_TESTED << " 31 bits unsigned integers generated Mean " << m0 << " Sigma "<<s0<<std::endl;
}
static test_t tests[] = {
TEST_NO_TAG("Key Exchange", exchange),
TEST_NO_TAG("Signature", signAndVerify),
TEST_NO_TAG("HKDF", hashMac_KDF),
TEST_NO_TAG("AEAD", AEAD),
TEST_NO_TAG("RNG", RNG_test),
};
test_suite_t lime_crypto_test_suite = {
......
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