microsoft / SymCrypt

Cryptographic library
MIT License
660 stars 68 forks source link

Elliptic curve private-to-public incorrect result on Linux 32 bit #9

Closed guidovranken closed 2 years ago

guidovranken commented 3 years ago

Reproducer:

#include <cstdint>
#include <iostream>
#include <sstream>
#include <string>
#include <vector>
#include <boost/multiprecision/cpp_int.hpp>
#include <symcrypt.h>

#define CF_CHECK_EQ(expr, res) if ( (expr) != (res) ) { goto end; }
#define CF_CHECK_NE(expr, res) if ( (expr) == (res) ) { goto end; }

extern "C" {
    void SymCryptFatal(UINT32 fatalCode) {
        (void)fatalCode;

        abort();
    }
    void SymCryptInjectError( PBYTE pbData, SIZE_T cbData ) {
        (void)pbData;
        (void)cbData;
    }

    PVOID SymCryptCallbackAlloc( SIZE_T nBytes ) {
        return malloc(nBytes);
    }

    VOID SymCryptCallbackFree( VOID * pMem ) {
        free(pMem);
    }

    SYMCRYPT_ERROR SymCryptCallbackRandom(PBYTE   pbBuffer, SIZE_T  cbBuffer ) {
        abort();
    }

    SYMCRYPT_CPU_FEATURES
    SymCryptCpuFeaturesNeverPresent(void) {
        return 0;
    }
}

namespace SymCrypt_detail {
    static bool EncodeBignum(const std::string s, uint8_t* out, const size_t outSize) {
        std::vector<uint8_t> v;
        boost::multiprecision::cpp_int c(s);
        boost::multiprecision::export_bits(c, std::back_inserter(v), 8);
        if ( v.size() > outSize ) {
            return false;
        }
        const auto diff = outSize - v.size();

        memset(out, 0, outSize);
        memcpy(out + diff, v.data(), v.size());

        return true;
    }

    static std::string toString(const boost::multiprecision::cpp_int& i) {
        std::stringstream ss;
        ss << i;

        if ( ss.str().empty() ) {
            return "0";
        } else {
            return ss.str();
        }
    }
}

int main(void)
{
    SYMCRYPT_ECURVE* curve = nullptr;
    SYMCRYPT_ECKEY* key = nullptr;
    const SYMCRYPT_ECURVE_PARAMS* curveParams = SymCryptEcurveParamsNistP384;

    CF_CHECK_NE(curve = SymCryptEcurveAllocate(curveParams, 0), nullptr);
    CF_CHECK_NE(key = SymCryptEckeyAllocate(curve), nullptr);

    {
        const auto priv_size = SymCryptEckeySizeofPrivateKey(key);
        std::vector<uint8_t> priv_bytes(priv_size);

        CF_CHECK_EQ(SymCrypt_detail::EncodeBignum(
                    "15",
                    priv_bytes.data(),
                    priv_size), true);

        CF_CHECK_EQ(SymCryptEckeySetValue(
                priv_bytes.data(), priv_size,
                NULL, 0,
                SYMCRYPT_NUMBER_FORMAT_MSB_FIRST, SYMCRYPT_ECPOINT_FORMAT_XY,
                0, key), SYMCRYPT_NO_ERROR);
    }

    {
        const auto pub_size = SymCryptEckeySizeofPublicKey(key, SYMCRYPT_ECPOINT_FORMAT_XY);
        if ( (pub_size % 2) != 0 ) {
            abort();
        }
        std::vector<uint8_t> pub_bytes(pub_size);

        CF_CHECK_EQ(SymCryptEckeyGetValue(
                    key,
                    NULL, 0,
                    pub_bytes.data(), pub_size,
                    SYMCRYPT_NUMBER_FORMAT_MSB_FIRST, SYMCRYPT_ECPOINT_FORMAT_XY,
                    0), SYMCRYPT_NO_ERROR);

        {
            boost::multiprecision::cpp_int x, y;

            boost::multiprecision::import_bits(x, pub_bytes.begin(), pub_bytes.begin() + (pub_size/2));
            boost::multiprecision::import_bits(y, pub_bytes.begin() + (pub_size/2), pub_bytes.end());

            std::cout << SymCrypt_detail::toString(x) << std::endl;
            std::cout << SymCrypt_detail::toString(y) << std::endl;
        }
    }

end:
    if ( key ) {
        /* noret */ SymCryptEckeyFree(key);
    }
    if ( curve ) {
        /* noret */ SymCryptEcurveFree(curve);
    }

    return 0;
}

First, compile SymCrypt like this:

export CC=clang
export CXX=clang++
export CFLAGS="-m32"
export CXXFLAGS="-m32"

git clone --depth 1 https://github.com/microsoft/SymCrypt.git
cd SymCrypt/
# Unittests don't build with clang and are not needed anyway
sed -i "s/^add_subdirectory(unittest)$//g" CMakeLists.txt
mkdir b/
cd b/
setarch i386 cmake ../
make -j$(nproc)

Then compile and run the reproducer:

$ $CXX $CXXFLAGS -DSYMCRYPT_IGNORE_PLATFORM -I SymCrypt/inc/ p.cpp SymCrypt/b/lib/i686/Generic/libsymcrypt_common.a
$ ./a.out
3371410058567038036102658371067750911905410377350295952202630426945666718973957844373613366326295398452393
6490343607037185220769843464241451023660093550735871349269711288852702861970840804993910898129495774144414

But it should print:

27676427741886189792545170648212433086611990497738896912065553637549098719435212229527415490501936487430563654174219
3256906964515830166357393887851908050144111374947447699655351073133031632026642053647058697111201931251927081270626
NielsFerguson commented 3 years ago

Hi Guido,

I’ll reply in English so that I can CC some other folks.

Thanks for taking the time to test SymCrypt. We really appreciate the effort that goes into that.

Although we are working on the Linux port, this has not been finished yet, and we currently do not support SymCrypt on Linux 32 bits. Trying to track down a bug like this is really hard without porting and running the SymCrypt unit test; it could be a bug anywhere in the big-integer arithmetic, and we have lots of test for that. I’ve been out for a few months, so I don’t know the exact current status. Mitch, can you tell us what the SymCrypt-on-linux status is? We also might have to do another drop to Github with our latest changes soon.

Cheers!

Niels

mlindgren commented 2 years ago

This has been fixed internally; our next public release will include the changes. For anyone looking for a workaround in the meantime, the problem is this macro:

#if SYMCRYPT_MS_VC
#define SYMCRYPT_MUL32x32TO64( _a, _b )         UInt32x32To64( (_a), (_b) )
#elif SYMCRYPT_GNUC
#define SYMCRYPT_MUL32x32TO64( _a, _b )         ( (unsigned long)(_a)*(unsigned long)(_b) )
#else
    #error Unknown compiler
#endif

The SYMCRYPT_GNUC implementation should instead be ( (UINT64)(_a)*(UINT64)(_b) )