herumi / mcl

a portable and fast pairing-based cryptography library
BSD 3-Clause "New" or "Revised" License
450 stars 151 forks source link

Hash to G assumes data is in montgomery form #186

Closed darcys22 closed 10 months ago

darcys22 commented 10 months ago

This code spans both the bls and mcl libraries but in bls/src/bls_c_impl.hpp this function that takes the binary of a hash and maps it to a point:

#ifndef BLS_MINIMUM_API
template<class G>
inline bool toG(G& Hm, const void *h, mclSize size)
{
    if (g_irtfHashAndMap) {
        hashAndMapToG(Hm, h, size);
        return true;
    }
    // backward compatibility
    Fp t;
    std::cout << "t is Mont: " << t.isMont() << '\n'; // Debugging line added here
    t.setArrayMask((const uint8_t *)h, size);
...
}

The debugging line returns this:

t is Mont: 1 

The effects of this being that when setArrayMask is called:

    template<class S>
    void setArrayMask(const S *x, size_t n)
    {
        const size_t dstByte = sizeof(Unit) * op_.N;
        if (sizeof(S) * n > dstByte) {
            n = dstByte / sizeof(S);
        }
        bool b = fp::convertArrayAsLE(v_, op_.N, x, n);
        assert(b);
        (void)b;
        bint::maskN(v_, op_.N, op_.bitSize);
        if (bint::cmpGeN(v_, op_.p, op_.N)) {
            bint::maskN(v_, op_.N, op_.bitSize - 1);
        }
        toMont();
    }

The data going into v_ will be assumed to be already in montgomery form and the toMont() function will be a noop.

This might be a non-issue because the point that the data gets mapped to is somewhat arbitrary and as long as its consistent I don't think it cause issues. But when trying to match this behaviour in another non c++ library where we haven't got montgomery functions this has caused some difficulty.

Is there a way to set Fp to be not in montgomery form before this setArrayMask happens? (Calling fromMont() on the uninitialized t variable perhaps?) and will this cause issues in other places?

herumi commented 10 months ago

The data going into v_ will be assumed to be already in montgomery form and the toMont() function will be a noop.

No, Fp::isMont() == true means that v_ must be in Montgomery form, so setArrayMask takes a given array and must convert it into a Montgomery form by calling toMont(). toMont() does nothing unless isMont() is true. Then setArrayMask can set the same value whether isMont() is true or not.

darcys22 commented 10 months ago

My mistake! thanks for the clarification