Skip to content

Commit ade6c66

Browse files
feliperodrifeliperodri
authored
Adds macros to check readable/mutable references (#1991)
* Adds ENSURE macros to check readable and mutable references Signed-off-by: Felipe R. Monteiro <felisous@amazon.com> * Replaces ENSURE_NONNULL by ENSURE_REF (and variants) Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
1 parent fa161e9 commit ade6c66

File tree

7 files changed

+122
-112
lines changed

7 files changed

+122
-112
lines changed

‎crypto/s2n_certificate.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,6 @@ s2n_pkey_type s2n_cert_chain_and_key_get_pkey_type(struct s2n_cert_chain_and_key
512512

513513
s2n_cert_private_key *s2n_cert_chain_and_key_get_private_key(struct s2n_cert_chain_and_key *chain_and_key)
514514
{
515-
ENSURE_PTR_NONNULL(chain_and_key);
515+
ENSURE_REF_PTR(chain_and_key);
516516
return chain_and_key->private_key;
517517
}

‎crypto/s2n_ecc_evp.c‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,17 @@ const struct s2n_ecc_named_curve s2n_ecc_curve_secp384r1 =
5555

5656
#if EVP_APIS_SUPPORTED
5757
const struct s2n_ecc_named_curve s2n_ecc_curve_x25519 = {
58-
.iana_id = TLS_EC_CURVE_ECDH_X25519,
59-
.libcrypto_nid = NID_X25519,
60-
.name = "x25519",
58+
.iana_id = TLS_EC_CURVE_ECDH_X25519,
59+
.libcrypto_nid = NID_X25519,
60+
.name = "x25519",
6161
.share_size = 32
6262
};
63-
#else
63+
#else
6464
const struct s2n_ecc_named_curve s2n_ecc_curve_x25519 = {0};
6565
#endif
6666

67-
/* All curves that s2n supports. New curves MUST be added here.
68-
* This list is a super set of all the curves present in s2n_ecc_preferences list.
67+
/* All curves that s2n supports. New curves MUST be added here.
68+
* This list is a super set of all the curves present in s2n_ecc_preferences list.
6969
*/
7070
const struct s2n_ecc_named_curve *const s2n_all_supported_curves_list[] = {
7171
&s2n_ecc_curve_secp256r1,
@@ -158,7 +158,7 @@ static int s2n_ecc_evp_compute_shared_secret(EVP_PKEY *own_key, EVP_PKEY *peer_p
158158
}
159159

160160
size_t shared_secret_size;
161-
161+
162162
DEFER_CLEANUP(EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new(own_key, NULL), EVP_PKEY_CTX_free_pointer);
163163
S2N_ERROR_IF(ctx == NULL, S2N_ERR_ECDHE_SHARED_SECRET);
164164

@@ -177,7 +177,7 @@ static int s2n_ecc_evp_compute_shared_secret(EVP_PKEY *own_key, EVP_PKEY *peer_p
177177

178178
int s2n_ecc_evp_generate_ephemeral_key(struct s2n_ecc_evp_params *ecc_evp_params) {
179179
notnull_check(ecc_evp_params->negotiated_curve);
180-
S2N_ERROR_IF(ecc_evp_params->evp_pkey != NULL, S2N_ERR_ECDHE_GEN_KEY);
180+
S2N_ERROR_IF(ecc_evp_params->evp_pkey != NULL, S2N_ERR_ECDHE_GEN_KEY);
181181
S2N_ERROR_IF(s2n_ecc_evp_generate_own_key(ecc_evp_params->negotiated_curve, &ecc_evp_params->evp_pkey) != 0,
182182
S2N_ERR_ECDHE_GEN_KEY);
183183
S2N_ERROR_IF(ecc_evp_params->evp_pkey == NULL, S2N_ERR_ECDHE_GEN_KEY);
@@ -198,15 +198,15 @@ int s2n_ecc_evp_compute_shared_secret_from_params(struct s2n_ecc_evp_params *pri
198198
return 0;
199199
}
200200

201-
int s2n_ecc_evp_compute_shared_secret_as_server(struct s2n_ecc_evp_params *ecc_evp_params,
201+
int s2n_ecc_evp_compute_shared_secret_as_server(struct s2n_ecc_evp_params *ecc_evp_params,
202202
struct s2n_stuffer *Yc_in, struct s2n_blob *shared_key) {
203203
notnull_check(ecc_evp_params->negotiated_curve);
204-
notnull_check(ecc_evp_params->evp_pkey);
204+
notnull_check(ecc_evp_params->evp_pkey);
205205
notnull_check(Yc_in);
206206

207207
uint8_t client_public_len;
208208
struct s2n_blob client_public_blob = {0};
209-
209+
210210
DEFER_CLEANUP(EVP_PKEY *peer_key = EVP_PKEY_new(), EVP_PKEY_free_pointer);
211211
S2N_ERROR_IF(peer_key == NULL, S2N_ERR_BAD_MESSAGE);
212212
GUARD(s2n_stuffer_read_uint8(Yc_in, &client_public_len));
@@ -244,17 +244,17 @@ int s2n_ecc_evp_compute_shared_secret_as_server(struct s2n_ecc_evp_params *ecc_e
244244

245245
}
246246

247-
int s2n_ecc_evp_compute_shared_secret_as_client(struct s2n_ecc_evp_params *ecc_evp_params,
247+
int s2n_ecc_evp_compute_shared_secret_as_client(struct s2n_ecc_evp_params *ecc_evp_params,
248248
struct s2n_stuffer *Yc_out, struct s2n_blob *shared_key) {
249249

250-
DEFER_CLEANUP(struct s2n_ecc_evp_params client_params = {0}, s2n_ecc_evp_params_free);
250+
DEFER_CLEANUP(struct s2n_ecc_evp_params client_params = {0}, s2n_ecc_evp_params_free);
251251

252252
notnull_check(ecc_evp_params->negotiated_curve);
253253
client_params.negotiated_curve = ecc_evp_params->negotiated_curve;
254254
GUARD(s2n_ecc_evp_generate_own_key(client_params.negotiated_curve, &client_params.evp_pkey));
255255
S2N_ERROR_IF(client_params.evp_pkey == NULL, S2N_ERR_ECDHE_GEN_KEY);
256256

257-
if (s2n_ecc_evp_compute_shared_secret(client_params.evp_pkey, ecc_evp_params->evp_pkey,
257+
if (s2n_ecc_evp_compute_shared_secret(client_params.evp_pkey, ecc_evp_params->evp_pkey,
258258
ecc_evp_params->negotiated_curve->iana_id, shared_key) != S2N_SUCCESS) {
259259
S2N_ERROR(S2N_ERR_ECDHE_SHARED_SECRET);
260260
}
@@ -265,7 +265,7 @@ int s2n_ecc_evp_compute_shared_secret_as_client(struct s2n_ecc_evp_params *ecc_e
265265
S2N_ERROR(S2N_ERR_ECDHE_SERIALIZING);
266266
}
267267
return 0;
268-
268+
269269
}
270270

271271
#if (!EVP_APIS_SUPPORTED)
@@ -352,7 +352,7 @@ int s2n_ecc_evp_write_params_point(struct s2n_ecc_evp_params *ecc_evp_params, st
352352
if (size != ecc_evp_params->negotiated_curve->share_size) {
353353
OPENSSL_free(encoded_point);
354354
S2N_ERROR(S2N_ERR_ECDHE_SERIALIZING);
355-
}
355+
}
356356
else {
357357
point_blob.data = s2n_stuffer_raw_write(out, ecc_evp_params->negotiated_curve->share_size);
358358
notnull_check(point_blob.data);

‎error/s2n_errno.h‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -283,18 +283,19 @@ extern __thread const char *s2n_debug_str;
283283
* Violations of the function contracts are undefined behaviour.
284284
*/
285285
#ifdef CBMC
286+
# define S2N_OBJECT_PTR_IS_READABLE(ptr) S2N_MEM_IS_READABLE((ptr), sizeof(*(ptr)))
287+
# define S2N_OBJECT_PTR_IS_WRITABLE(ptr) S2N_MEM_IS_WRITABLE((ptr), sizeof(*(ptr)))
286288
# define S2N_MEM_IS_READABLE(base, len) (((len) == 0) || __CPROVER_r_ok((base), (len)))
287289
# define S2N_MEM_IS_WRITABLE(base, len) (((len) == 0) || __CPROVER_w_ok((base), (len)))
288290
#else
289291
/* the C runtime does not give a way to check these properties,
290292
* but we can at least check that the pointer is valid */
291-
# define S2N_MEM_IS_READABLE(base, len) (((len) == 0) || (base))
292-
# define S2N_MEM_IS_WRITABLE(base, len) (((len) == 0) || (base))
293+
# define S2N_OBJECT_PTR_IS_READABLE(ptr) ((ptr) != NULL)
294+
# define S2N_OBJECT_PTR_IS_WRITABLE(ptr) ((ptr) != NULL)
295+
# define S2N_MEM_IS_READABLE(base, len) (((len) == 0) || (base) != NULL)
296+
# define S2N_MEM_IS_WRITABLE(base, len) (((len) == 0) || (base) != NULL)
293297
#endif /* CBMC */
294298

295-
#define S2N_OBJECT_PTR_IS_READABLE(ptr) S2N_MEM_IS_READABLE((ptr), sizeof(*(ptr)))
296-
#define S2N_OBJECT_PTR_IS_WRITABLE(ptr) S2N_MEM_IS_WRITABLE((ptr), sizeof(*(ptr)))
297-
298299
#define S2N_IMPLIES(a, b) (!(a) || (b))
299300

300301
/** Calculate and print stacktraces */

‎tls/s2n_async_pkey.c‎

Lines changed: 38 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ DEFINE_POINTER_CLEANUP_FUNC(struct s2n_async_pkey_op *, s2n_async_pkey_op_free);
9494

9595
static S2N_RESULT s2n_async_get_actions(s2n_async_pkey_op_type type, const struct s2n_async_pkey_op_actions **actions)
9696
{
97-
ENSURE_NONNULL(actions);
97+
ENSURE_REF(actions);
9898

9999
switch (type) {
100100
case S2N_ASYNC_DECRYPT:
@@ -111,7 +111,7 @@ static S2N_RESULT s2n_async_get_actions(s2n_async_pkey_op_type type, const struc
111111

112112
static S2N_RESULT s2n_async_pkey_op_allocate(struct s2n_async_pkey_op **op)
113113
{
114-
ENSURE_NONNULL(op);
114+
ENSURE_REF(op);
115115
ENSURE(*op == NULL, S2N_ERR_SAFETY);
116116

117117
/* allocate memory */
@@ -130,10 +130,10 @@ static S2N_RESULT s2n_async_pkey_op_allocate(struct s2n_async_pkey_op **op)
130130
S2N_RESULT s2n_async_pkey_decrypt(struct s2n_connection *conn, struct s2n_blob *encrypted,
131131
struct s2n_blob *init_decrypted, s2n_async_pkey_decrypt_complete on_complete)
132132
{
133-
ENSURE_NONNULL(conn);
134-
ENSURE_NONNULL(encrypted);
135-
ENSURE_NONNULL(init_decrypted);
136-
ENSURE_NONNULL(on_complete);
133+
ENSURE_REF(conn);
134+
ENSURE_REF(encrypted);
135+
ENSURE_REF(init_decrypted);
136+
ENSURE_REF(on_complete);
137137

138138
if (conn->config->async_pkey_cb) {
139139
GUARD_RESULT(s2n_async_pkey_decrypt_async(conn, encrypted, init_decrypted, on_complete));
@@ -147,10 +147,10 @@ S2N_RESULT s2n_async_pkey_decrypt(struct s2n_connection *conn, struct s2n_blob *
147147
S2N_RESULT s2n_async_pkey_decrypt_async(struct s2n_connection *conn, struct s2n_blob *encrypted,
148148
struct s2n_blob *init_decrypted, s2n_async_pkey_decrypt_complete on_complete)
149149
{
150-
ENSURE_NONNULL(conn);
151-
ENSURE_NONNULL(encrypted);
152-
ENSURE_NONNULL(init_decrypted);
153-
ENSURE_NONNULL(on_complete);
150+
ENSURE_REF(conn);
151+
ENSURE_REF(encrypted);
152+
ENSURE_REF(init_decrypted);
153+
ENSURE_REF(on_complete);
154154
ENSURE(conn->handshake.async_state == S2N_ASYNC_NOT_INVOKED, S2N_ERR_ASYNC_MORE_THAN_ONE);
155155

156156
DEFER_CLEANUP(struct s2n_async_pkey_op *op = NULL, s2n_async_pkey_op_free_pointer);
@@ -185,10 +185,10 @@ S2N_RESULT s2n_async_pkey_decrypt_async(struct s2n_connection *conn, struct s2n_
185185
S2N_RESULT s2n_async_pkey_decrypt_sync(struct s2n_connection *conn, struct s2n_blob *encrypted,
186186
struct s2n_blob *init_decrypted, s2n_async_pkey_decrypt_complete on_complete)
187187
{
188-
ENSURE_NONNULL(conn);
189-
ENSURE_NONNULL(encrypted);
190-
ENSURE_NONNULL(init_decrypted);
191-
ENSURE_NONNULL(on_complete);
188+
ENSURE_REF(conn);
189+
ENSURE_REF(encrypted);
190+
ENSURE_REF(init_decrypted);
191+
ENSURE_REF(on_complete);
192192

193193
const struct s2n_pkey *pkey = conn->handshake_params.our_chain_and_key->private_key;
194194

@@ -201,9 +201,9 @@ S2N_RESULT s2n_async_pkey_decrypt_sync(struct s2n_connection *conn, struct s2n_b
201201
S2N_RESULT s2n_async_pkey_sign(struct s2n_connection *conn, s2n_signature_algorithm sig_alg,
202202
struct s2n_hash_state *digest, s2n_async_pkey_sign_complete on_complete)
203203
{
204-
ENSURE_NONNULL(conn);
205-
ENSURE_NONNULL(digest);
206-
ENSURE_NONNULL(on_complete);
204+
ENSURE_REF(conn);
205+
ENSURE_REF(digest);
206+
ENSURE_REF(on_complete);
207207

208208
if (conn->config->async_pkey_cb) {
209209
GUARD_RESULT(s2n_async_pkey_sign_async(conn, sig_alg, digest, on_complete));
@@ -217,9 +217,9 @@ S2N_RESULT s2n_async_pkey_sign(struct s2n_connection *conn, s2n_signature_algori
217217
S2N_RESULT s2n_async_pkey_sign_async(struct s2n_connection *conn, s2n_signature_algorithm sig_alg,
218218
struct s2n_hash_state *digest, s2n_async_pkey_sign_complete on_complete)
219219
{
220-
ENSURE_NONNULL(conn);
221-
ENSURE_NONNULL(digest);
222-
ENSURE_NONNULL(on_complete);
220+
ENSURE_REF(conn);
221+
ENSURE_REF(digest);
222+
ENSURE_REF(on_complete);
223223
ENSURE(conn->handshake.async_state == S2N_ASYNC_NOT_INVOKED, S2N_ERR_ASYNC_MORE_THAN_ONE);
224224

225225
DEFER_CLEANUP(struct s2n_async_pkey_op *op = NULL, s2n_async_pkey_op_free_pointer);
@@ -255,9 +255,9 @@ S2N_RESULT s2n_async_pkey_sign_async(struct s2n_connection *conn, s2n_signature_
255255
S2N_RESULT s2n_async_pkey_sign_sync(struct s2n_connection *conn, s2n_signature_algorithm sig_alg,
256256
struct s2n_hash_state *digest, s2n_async_pkey_sign_complete on_complete)
257257
{
258-
ENSURE_NONNULL(conn);
259-
ENSURE_NONNULL(digest);
260-
ENSURE_NONNULL(on_complete);
258+
ENSURE_REF(conn);
259+
ENSURE_REF(digest);
260+
ENSURE_REF(on_complete);
261261

262262
const struct s2n_pkey *pkey = conn->handshake_params.our_chain_and_key->private_key;
263263
DEFER_CLEANUP(struct s2n_blob signed_content = { 0 }, s2n_free);
@@ -274,8 +274,8 @@ S2N_RESULT s2n_async_pkey_sign_sync(struct s2n_connection *conn, s2n_signature_a
274274

275275
int s2n_async_pkey_op_perform(struct s2n_async_pkey_op *op, s2n_cert_private_key *key)
276276
{
277-
ENSURE_POSIX_NONNULL(op);
278-
ENSURE_POSIX_NONNULL(key);
277+
ENSURE_POSIX_REF(op);
278+
ENSURE_POSIX_REF(key);
279279
ENSURE_POSIX(!op->complete, S2N_ERR_ASYNC_ALREADY_PERFORMED);
280280

281281
const struct s2n_async_pkey_op_actions *actions = NULL;
@@ -290,8 +290,8 @@ int s2n_async_pkey_op_perform(struct s2n_async_pkey_op *op, s2n_cert_private_key
290290

291291
int s2n_async_pkey_op_apply(struct s2n_async_pkey_op *op, struct s2n_connection *conn)
292292
{
293-
ENSURE_POSIX_NONNULL(op);
294-
ENSURE_POSIX_NONNULL(conn);
293+
ENSURE_POSIX_REF(op);
294+
ENSURE_POSIX_REF(conn);
295295
ENSURE_POSIX(op->complete, S2N_ERR_ASYNC_NOT_PERFORMED);
296296
ENSURE_POSIX(!op->applied, S2N_ERR_ASYNC_ALREADY_APPLIED);
297297
/* We could have just used op->conn and removed a conn argument, but we want caller
@@ -318,7 +318,7 @@ int s2n_async_pkey_op_apply(struct s2n_async_pkey_op *op, struct s2n_connection
318318

319319
int s2n_async_pkey_op_free(struct s2n_async_pkey_op *op)
320320
{
321-
ENSURE_POSIX_NONNULL(op);
321+
ENSURE_POSIX_REF(op);
322322
const struct s2n_async_pkey_op_actions *actions = NULL;
323323
GUARD_AS_POSIX(s2n_async_get_actions(op->type, &actions));
324324

@@ -332,8 +332,8 @@ int s2n_async_pkey_op_free(struct s2n_async_pkey_op *op)
332332

333333
S2N_RESULT s2n_async_pkey_decrypt_perform(struct s2n_async_pkey_op *op, s2n_cert_private_key *pkey)
334334
{
335-
ENSURE_NONNULL(op);
336-
ENSURE_NONNULL(pkey);
335+
ENSURE_REF(op);
336+
ENSURE_REF(pkey);
337337

338338
struct s2n_async_pkey_decrypt_data *decrypt = &op->op.decrypt;
339339

@@ -344,8 +344,8 @@ S2N_RESULT s2n_async_pkey_decrypt_perform(struct s2n_async_pkey_op *op, s2n_cert
344344

345345
S2N_RESULT s2n_async_pkey_decrypt_apply(struct s2n_async_pkey_op *op, struct s2n_connection *conn)
346346
{
347-
ENSURE_NONNULL(op);
348-
ENSURE_NONNULL(conn);
347+
ENSURE_REF(op);
348+
ENSURE_REF(conn);
349349

350350
struct s2n_async_pkey_decrypt_data *decrypt = &op->op.decrypt;
351351

@@ -356,7 +356,7 @@ S2N_RESULT s2n_async_pkey_decrypt_apply(struct s2n_async_pkey_op *op, struct s2n
356356

357357
S2N_RESULT s2n_async_pkey_decrypt_free(struct s2n_async_pkey_op *op)
358358
{
359-
ENSURE_NONNULL(op);
359+
ENSURE_REF(op);
360360

361361
struct s2n_async_pkey_decrypt_data *decrypt = &op->op.decrypt;
362362

@@ -370,8 +370,8 @@ S2N_RESULT s2n_async_pkey_decrypt_free(struct s2n_async_pkey_op *op)
370370

371371
S2N_RESULT s2n_async_pkey_sign_perform(struct s2n_async_pkey_op *op, s2n_cert_private_key *pkey)
372372
{
373-
ENSURE_NONNULL(op);
374-
ENSURE_NONNULL(pkey);
373+
ENSURE_REF(op);
374+
ENSURE_REF(pkey);
375375

376376
struct s2n_async_pkey_sign_data *sign = &op->op.sign;
377377

@@ -385,8 +385,8 @@ S2N_RESULT s2n_async_pkey_sign_perform(struct s2n_async_pkey_op *op, s2n_cert_pr
385385

386386
S2N_RESULT s2n_async_pkey_sign_apply(struct s2n_async_pkey_op *op, struct s2n_connection *conn)
387387
{
388-
ENSURE_NONNULL(op);
389-
ENSURE_NONNULL(conn);
388+
ENSURE_REF(op);
389+
ENSURE_REF(conn);
390390

391391
struct s2n_async_pkey_sign_data *sign = &op->op.sign;
392392

@@ -397,7 +397,7 @@ S2N_RESULT s2n_async_pkey_sign_apply(struct s2n_async_pkey_op *op, struct s2n_co
397397

398398
S2N_RESULT s2n_async_pkey_sign_free(struct s2n_async_pkey_op *op)
399399
{
400-
ENSURE_NONNULL(op);
400+
ENSURE_REF(op);
401401

402402
struct s2n_async_pkey_sign_data *sign = &op->op.sign;
403403

@@ -406,4 +406,3 @@ S2N_RESULT s2n_async_pkey_sign_free(struct s2n_async_pkey_op *op)
406406

407407
return S2N_RESULT_OK;
408408
}
409-

0 commit comments

Comments
 (0)