[Development] websockets (was RE: Qt 5.3 Feature freeze is coming quite soon...)

[Kurt Pattyn]

On 29 Jan 2014, at 09:45, Konrad Rosenbaum <konrad at silmor.de> wrote:
>  
> 1) create a fresh TCP connection for each Websocket, this way it is not possible to combine a previous HTTP request and a socket in an attack; consequently: if the handshake fails: terminate the TCP connection!
This is already happening in the implementation.
>  
> 2) when faced with an explicitly configured proxy: ALWAYS use the CONNECT method before sending the actual socket request - proxies do not care about the socket content after a CONNECT is through - additionally there are probably close to zero deployed proxies that even understand the semantics of Upgrade: websocket.
>  
> Unfortunately 2) does not help with broken transparent proxies, since by their nature you cannot detect them easily and it is not a good strategy to send a CONNECT just in case…
The implementation uses the QNetworkProxy implementation.
>  
> 3) implement masking properly.
>  
> The latter can be done in stages, I'd propose this plan:
>  
> First: implement websockets, get them into Qt. For the mask generation create a virtual protected method "QByteArray getNextMaskValue(int numbytes)const" and implement it using qrand()&0xff for each byte - please note that you have to initialize with qsrand in each thread.


If I use std::random as Thiago proposed (see http://en.cppreference.com/w/cpp/numeric/random), like:

    //one time initialisation
    Q_CONST_EXPR std::size_t numSeeds = 13;	//arbitrary number
    quint32 seeds[numSeeds] = { 0 };
    bool success = false;

    try {
        std::random_device seeder;		//supported since MSVS 2008, since GCC 4.5
        if (seeder.entropy() >= 0.5) {
            std::generate_n(seeds, numSeeds, seeder);
            success = true;
        }
    } catch (const std::exception &) {
        //fall through
    }
#ifdef Q_OS_WIN  //the MinGW implementation of GCC 4.8 has a known bug in std::random_device
    if (!success) {
        HCRYPTPROV hp = 0;
        BYTE rb[4];
        if ((success = CryptAcquireContext(&hp, 0, 0, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT)) == TRUE)
            for (std::size_t i = 0; i < numSeeds; ++i) {
                if ((success = CryptGenRandom(hp, sizeof(quint32), seeds + i) != TRUE)
                    break;
        }
        CryptReleaseContext(hp, 0);
    }
#endif
    if (!success) {
        qsrand(static_cast<uint>(QDateTime::currentMSecsSinceEpoch()));
        for (std::size_t i = 0; i < numSeeds; ++i) {
            const qreal multiplier = qreal(std::numeric_limits<quint8>::max()) / qreal(RAND_MAX);
            const quint32 byte1 = quint32(qrand() * multiplier) & 0xFF;
            const quint32 byte2 = quint32(qrand() * multiplier) & 0xFF;
            const quint32 byte3 = quint32(qrand() * multiplier) & 0xFF;
            const quint32 byte4 = quint32(qrand() * multiplier) & 0xFF;
            seeds[i] = byte1 | byte2 << 8 | byte3 << 16 | byte4 << 24;
        }
    }

    std::seek_seq seedSequence(seeds, seeds + numSeeds);
    std::mt19937 randomiser(seedSequence);
    //std::uniform_int_distribution<std::uint32_t> dist;	//range from 0 to UINT_MAX


    //somewhere else
    return randomiser();	//no need to use a distribution function as mt19937 outputs 2^32


Would this already be more acceptable?
Of course, this implementation requires C++11 support.
An interesting presentation about std::random can be found here: http://channel9.msdn.com/Events/GoingNative/2013/rand-Considered-Harmful

If the above implementation suffices, then a virtual method would not be needed anymore.

Should I fall back to the ordinary qrand() when the other methods fail?

> Document the shortcoming with something like this:
>  
> "This implementation of WebSockets uses cryptographically weak random numbers during communication. If you allow user generated or downloaded scripts to access WebSockets, then malicious scripts could abuse this implementation to attack some vulnerable web proxy servers (e.g. for cache poisoning). In this case it is recommended that you reimplement getNextMaskValue to use cryptographically strong random numbers."
> (maybe add a link to the paper)



Cheers,

Kurt

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20140209/eba53c21/attachment.html>