API transitions in Mbed TLS about cryptofuzz HOT 3 CLOSED

Sorry, didn't see this issue yet.

• Bignum: we're rewriting the bignum stack into 4 layers. The highest layer is the current one, and does its own memory management. The new layers are implemented without malloc(). There's a core layer for operations in [0,2^n] and two layers for operations modulo P, one with sanity checks and one without (intended to allow ECC code to be better optimized).

Apart from testing bignums of arbitrary size, Cryptofuzz has support for testing fields (0..2^N, 0..P where P is a prime, or anything else). This assumes overflowing behavior (e.g. uint256_t(2**256-1) + 1 == 0 and the same for prime fields).

Are there two functions you can call, one with sanity checks and one without? In that case Cryptofuzz provides a modifier from which we can extract bools to choose different code paths, e.g.

    Datasource ds(op.modifier.GetPtr(), op.modifier.GetSize());
    bool with_sanity_checks = false;
    try {
        with_sanity_checks = ds.Get<bool>();
    } catch ( ... ) { }
    
    if ( with_sanity_checks ) {
    	bignumcalc_mod_p_with_sanity_checks(op);
    } else {
	bignumcalc_mod_p_no_sanity_checks(op);
    }

• Crypto: we've had a new API for crypto (psa_xxx()) for a while, and it's currently not getting fuzzed. The new interfaces are mostly functionally equivalent, although there are a few features of the legacy API that aren't present in the PSA API. One difference that matters to fuzzing is that the PSA API handles random generation entirely internally, so making the RNG deterministic works completely differently.

Cryptofuzz can work without being able to hook the RNG. Hooking the RNG is better because this can for instance find bugs in ECDSA signing where an invalid nonce is generated if the RNG generates a particular sequence of data. We could consider replacing the PSA RNG with a deterministic one in fuzzing mode only, but it's not strictly necessary.

Do you have any advice as to when we should start fuzzing the new interfaces, in terms of maturity? Should we have a transition period where both the old and the new interfaces are tested?

I'm fine with starting right away but if the new interface is still under heavy development and will frequently cause bugs, it might be better to move the Mbed TLS Cryptofuzz fuzzer to the OSS-Fuzz mbedtls project, instead of the cryptofuzz project, because the latter has a large CC list of project maintainers, and I try to reduce the amount of e-mails they get from OSS-Fuzz.

If you want that I can set that up. I don't need to be on the mbedtls CC list. If there are any build failures or other bugs originating on my part, you can simply forward these e-mails to me and I won't be privy to other mbedtls bugs.

If we have a transition period, how should we declare the different APIs to cryptofuzz and bignum-fuzzer? I guess they'd be different modules (since e.g. differential fuzzing of an ECDSA signature should be done via both psa_sign_hash and mbedtls_ecdsa_sign)? How would we arrange the build system to indicate that both modules are using the same library?

Are the old and the new API contained in the same static library? In that case we can just add support for the new to the existing module, choose between running the old and new API functions at runtime using the modifier as explained above, and build as usual.

If the old and new API require different build procedures and result in two different static libraries, then we need to build two different fuzzing targets. I guess in that case we can still add support for the new API to the same module, enclosed in #if defined(MBEDTLS_NEW_API).

For bignum, there's differential fuzzing in both bignum-fuzzer and cryptofuzz. What does bignum-fuzzer do that cryptofuzz doesn't?

bignum-fuzzer is essentially deprecated and it doesn't do anything that Cryptofuzz does not do. If the old API is removed from mbed TLS, I think we should just also remove support for mbed TLS from bignum-fuzzer.

Would you like to add PSA support the the Mbed TLS Cryptofuzz module? It doesn't need to be perfect, or even have to compile, once you submit a rough draft I can take it from there.

from cryptofuzz.

Thanks for the detailed reply, that was very helpful!

All the interface are available from a single build. I hadn't noticed the modifier possibility, that's exactly what we need.

The PSA API is stable, fuzzing it is overdue.

The bignum interfaces are brand new, and they're internal interfaces which we're liable to change without notice. So on the one hand we'd like to fuzz the bugs out of them, but on the other hand if we do that we'd take the responsibility of validating that our changes don't break the oss-fuzz cryptofuzz instance. The nice thing about cryptofuzz is that it does differential fuzzing: just fuzzing mbedtls on its own would catch e.g. a buffer overflow but not a wrong result. But I guess we could set up a cryptofuzz instance in the mbedtls fuzzer which just fuzzes against openssl?

Thanks for the advice. We're going to discuss what we do inside the Mbed TLS team. At a guess, the next step will probably be for us to submit an initial version of PSA crypto API support in cryptofuzz.

from cryptofuzz.

I'm closing this issue because at this stage, Mbed TLS is not expecting an action from Cryptofuzz — the action is on us to get started with PSA support in Cryptofuzz.

from cryptofuzz.