Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
guidovranken opened this issue Nov 6, 2020 · 2 comments
Closed
Labels
bug Something isn't working

Comments

@guidovranken
Copy link

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
Copy link
Contributor

NielsFerguson commented Nov 7, 2020 via email

@mlindgren mlindgren added the investigate Needs further investigation label Mar 2, 2021
@mlindgren mlindgren added bug Something isn't working and removed investigate Needs further investigation labels Aug 31, 2022
@mlindgren
Copy link
Member

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) )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants