How to handle OSSL errors for functions that return values (not error codes)

[feliperodri]

Security issue notifications

N/A.

Problem:

In the following case, we want to guard this code using GUARD_OSSL:

    GUARD_OSSL(shared_key_size = DH_compute_key(shared_key->data, pub_key, server_dh_params->dh), S2N_ERR_DH_SHARED_SECRET);

    shared_key->size = shared_key_size;

But we also need to keep the return value of this function, which leads to an error.

[2020-12-02 20:33:41] [build-err] /opt/src/crypto/s2n_dhe.c:286:5: note: in expansion of macro ‘GUARD_OSSL’
[2020-12-02 20:33:41] [build-err]   286 |     GUARD_OSSL(shared_key_size = DH_compute_key(shared_key->data, pub_key, server_dh_params->dh), S2N_ERR_DH_SHARED_SECRET);
[2020-12-02 20:33:41] [build-err]       |     ^~~~~~~~~~
[2020-12-02 20:33:41] [build-err] In file included from /opt/src/crypto/s2n_dhe.h:18,
[2020-12-02 20:33:41] [build-err]                  from /opt/src/crypto/s2n_dhe.c:16:
[2020-12-02 20:33:41] [build-err] /usr/include/openssl/dh.h:152:54: note: expected ‘const BIGNUM *’ {aka ‘const struct bignum_st *’} but argument is of type ‘int’
[2020-12-02 20:33:41] [build-err]   152 | int DH_compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh);
[2020-12-02 20:33:41] [build-err]       |                                        ~~~~~~~~~~~~~~^~~~~~~
[2020-12-02 20:33:41] [build-err] /opt/src/crypto/s2n_dhe.c:290:13: warning: passing argument 1 of ‘BN_free’ makes pointer from integer without a cast [-Wint-conversion]
[2020-12-02 20:33:41] [build-err]   290 |     BN_free(pub_key);
[2020-12-02 20:33:41] [build-err]       |             ^~~~~~~
[2020-12-02 20:33:41] [build-err]       |             |
[2020-12-02 20:33:41] [build-err]       |             int
[2020-12-02 20:33:41] [build-err] In file included from /usr/include/openssl/asn1.h:23,
[2020-12-02 20:33:41] [build-err]                  from /usr/include/openssl/dh.h:18,
[2020-12-02 20:33:41] [build-err]                  from /opt/src/crypto/s2n_dhe.h:18,
[2020-12-02 20:33:41] [build-err]                  from /opt/src/crypto/s2n_dhe.c:16:
[2020-12-02 20:33:41] [build-err] /usr/include/openssl/bn.h:275:22: note: expected ‘BIGNUM *’ {aka ‘struct bignum_st *’} but argument is of type ‘int’
[2020-12-02 20:33:41] [build-err]   275 | void BN_free(BIGNUM *a);
[2020-12-02 20:33:41] [build-err]       |              ~~~~~~~~^

How can we handle such cases?

Solution:

N/A.

• Does this change what S2N sends over the wire? No.
• Does this change any public APIs? No.
• Which versions of TLS will this impact? N/A.

Requirements / Acceptance Criteria:

Update GUARD_OSSL to support functions with return values (not only error codes). Also update s2n_dhe.c where appropriate.

• RFC links: N/A.
• Related Issues: N/A.
• Will the Usage Guide or other documentation need to be updated? No.
• Testing: N/A.
  • Will this change trigger SAW changes? N/A.
  • Should this change be fuzz tested? N/A.

Out of scope:

N/A.

[SalusaSecondus]

I believe that #2385 fixes this by introducing new macros to handle these cases.