Handle invalid inputs consistently with respect to constant-time #1621

1. 
   jonasnick commented at 12:27 pm on October 21, 2024: contributor

   Right now, we have two different ways of handling invalid inputs in constant-time functions:

   1. The function is constant-time, even for invalid inputs. For example, secp256k1_schnorrsig_sign_internal continues with the signing procedure even if keypair_load fails (which happens when the keypair is unitialized):

      0ret &= secp256k1_keypair_load(ctx, &sk, &pk, keypair);
      1...
      2return ret;
      

      This works because even if secp256k1_keypair_load fails, valid values (sk=1, pk=G) are returned.
   2. The function is constant-time only for valid inputs. For example, in secp256k1_musig_partial_sign, we have

      0if (!secp256k1_keypair_load(ctx, &sk, &keypair_pk, keypair)) {
      1    secp256k1_musig_partial_sign_clear(&sk, k);
      2    return 0;
      3}
      

      This works because the return value of keypair_load is declassified.

   I think we should make functions only constant-time with respect to valid inputs. This leads to more readable and maintainable code (due to fewer ret &=). Calling functions with invalid inputs (such as an unitialized keypair) should never happen outside of development.

   Whatever version we’re choosing, we should document it in CONTRIBUTING.md.

2. jonasnick added the label refactor/smell on Oct 21, 2024
3. jonasnick added the label meta/development on Oct 21, 2024

Contributors
jonasnick

Labels
refactor/smell meta/development