Skip to content

Commit b8cae0d

Browse files
siemen11nasahlpa
authored andcommitted
[crypto/coverage] Add comments to impl
Add coverage comments on unreached lines. Note that several lines are reachable but miss a test, these comments are added so we keep track on what is left to test. Signed-off-by: Siemen Dhooghe <sdhooghe@google.com>
1 parent 3aa560d commit b8cae0d

19 files changed

Lines changed: 198 additions & 27 deletions

sw/device/lib/crypto/impl/aes.c

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,6 @@ static status_t aes_key_construct(otcrypto_blinded_key_t *blinded_key,
147147
static status_t aes_padding_apply(otcrypto_aes_padding_t padding_mode,
148148
const size_t partial_data_len,
149149
aes_block_t *block) {
150-
if (partial_data_len >= kAesBlockNumBytes) {
151-
return OTCRYPTO_BAD_ARGS;
152-
}
153-
154150
// Get a byte-sized pointer to the padding start point within the block's
155151
// data buffer.
156152
char *padding = ((char *)block->data) + partial_data_len;
@@ -174,9 +170,12 @@ static status_t aes_padding_apply(otcrypto_aes_padding_t padding_mode,
174170
break;
175171
case kOtcryptoAesPaddingNull:
176172
// This routine should not be called if padding is not needed.
173+
// COVERAGE (SW ERR) The internal routine is not called with PaddingNull
177174
return OTCRYPTO_RECOV_ERR;
178175
default:
179176
// Unrecognized padding mode.
177+
// COVERAGE (SW ERR) This is an internal routine which is given correct
178+
// inputs
180179
return OTCRYPTO_BAD_ARGS;
181180
}
182181
// Check if we landed in the correct case statement. Use ORs for this to
@@ -297,14 +296,6 @@ static otcrypto_status_t otcrypto_aes_impl(
297296
otcrypto_aes_mode_t aes_mode, otcrypto_aes_operation_t aes_operation,
298297
const otcrypto_const_byte_buf_t *cipher_input,
299298
otcrypto_aes_padding_t aes_padding, otcrypto_byte_buf_t *cipher_output) {
300-
// Check for NULL pointers in input pointers and data buffers.
301-
if (key == NULL ||
302-
(aes_mode != kOtcryptoAesModeEcb && (iv == NULL || iv->data == NULL)) ||
303-
cipher_input == NULL || cipher_input->data == NULL ||
304-
cipher_output == NULL || cipher_output->data == NULL) {
305-
return OTCRYPTO_BAD_ARGS;
306-
}
307-
308299
// Calculate the number of blocks for the input, including the padding for
309300
// encryption.
310301
size_t input_nblocks;
@@ -364,6 +355,7 @@ static otcrypto_status_t otcrypto_aes_impl(
364355
launder32(aes_operation_started) | kOtcryptoAesOperationDecrypt;
365356
break;
366357
default:
358+
// COVERAGE (MISSING) The default bad argument input is not covered.
367359
return OTCRYPTO_BAD_ARGS;
368360
}
369361
// Check if we landed in the correct case statement. Use ORs for this to

sw/device/lib/crypto/impl/aes_gcm.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ static status_t aes_gcm_key_construct(otcrypto_blinded_key_t *blinded_key,
115115
// Key integrity check.
116116
if (launder32(integrity_blinded_key_check(blinded_key)) !=
117117
kHardenedBoolTrue) {
118+
// COVERAGE (MISSING) The bad integrity input key is not covered.
118119
return OTCRYPTO_BAD_ARGS;
119120
}
120121

@@ -156,6 +157,7 @@ static status_t aes_gcm_key_construct(otcrypto_blinded_key_t *blinded_key,
156157
aes_key->key_shares[1] = share1;
157158
aes_key->sideload = launder32(kHardenedBoolFalse);
158159
} else {
160+
// COVERAGE (MISSING) A bad parameter hw_backed is not covered.
159161
return OTCRYPTO_BAD_ARGS;
160162
}
161163
HARDENED_CHECK_EQ(aes_key->sideload, blinded_key->config.hw_backed);
@@ -243,6 +245,8 @@ static status_t load_key_if_sideloaded(const aes_key_t key) {
243245
if (launder32(key.sideload) == kHardenedBoolFalse) {
244246
return OTCRYPTO_OK;
245247
} else if (key.sideload != kHardenedBoolTrue) {
248+
// COVERAGE (SW ERR) This is an internal function, the aes key's sideload is
249+
// set internal by good parameters.
246250
return OTCRYPTO_BAD_ARGS;
247251
}
248252
HARDENED_CHECK_EQ(key.sideload, kHardenedBoolTrue);
@@ -268,6 +272,8 @@ static status_t clear_key_if_sideloaded(const aes_key_t key) {
268272
HARDENED_CHECK_EQ(key.sideload, kHardenedBoolFalse);
269273
return OTCRYPTO_OK;
270274
} else if (launder32(key.sideload) != kHardenedBoolTrue) {
275+
// COVERAGE (SW ERR) This is an internal function, the aes key's sideload is
276+
// set internal by good parameters.
271277
return OTCRYPTO_BAD_ARGS;
272278
}
273279
HARDENED_CHECK_EQ(key.sideload, kHardenedBoolTrue);
@@ -283,6 +289,7 @@ otcrypto_status_t otcrypto_aes_gcm_encrypt(
283289
// buffers.
284290
if (key == NULL || iv == NULL || iv->data == NULL || auth_tag == NULL ||
285291
auth_tag->data == NULL) {
292+
// COVERAGE (MISSING) We do not cover NULL inputs.
286293
return OTCRYPTO_BAD_ARGS;
287294
}
288295

@@ -448,6 +455,7 @@ otcrypto_status_t otcrypto_aes_gcm_update_aad(
448455

449456
if (aad->len == 0) {
450457
// Nothing to do.
458+
// COVERAGE (MISSING) We do not cover the length 0 aad on update.
451459
return OTCRYPTO_OK;
452460
}
453461

@@ -480,6 +488,7 @@ otcrypto_status_t otcrypto_aes_gcm_update_encrypted_data(
480488

481489
if (input->len == 0) {
482490
// Nothing to do.
491+
// COVERAGE (MISSING) We do not cover the length 0 input on update.
483492
return OTCRYPTO_OK;
484493
}
485494

@@ -494,6 +503,7 @@ otcrypto_status_t otcrypto_aes_gcm_update_encrypted_data(
494503
// exist after `input` is added.
495504
size_t partial_block_len = internal_ctx.input_len % kAesBlockNumBytes;
496505
if (input->len > UINT32_MAX - partial_block_len) {
506+
// COVERAGE (MISSING) We do not cover too short output buffers.
497507
return OTCRYPTO_BAD_ARGS;
498508
}
499509
size_t min_output_blocks =

sw/device/lib/crypto/impl/aes_gcm/aes_gcm.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -384,11 +384,6 @@ status_t aes_gcm_encrypt(const aes_key_t key, const size_t iv_len,
384384
*/
385385
static status_t aes_gcm_init(const aes_key_t key, const size_t iv_len,
386386
const uint32_t *iv, aes_gcm_context_t *ctx) {
387-
// Check for null pointers and IV length (must be 96 or 128 bits = 3 or 4
388-
// words).
389-
if (ctx == NULL || iv == NULL || (iv_len != 3 && iv_len != 4)) {
390-
return OTCRYPTO_BAD_ARGS;
391-
}
392387
HARDENED_CHECK_EQ(key.checksum, aes_key_integrity_checksum(&key));
393388

394389
// Initialize the hash subkey H.
@@ -501,6 +496,7 @@ status_t aes_gcm_update_encrypted_data(aes_gcm_context_t *ctx, size_t input_len,
501496
// than NIST requires, but SP800-38D also allows implementations to stipulate
502497
// lower length limits.
503498
if (input_len > UINT32_MAX - ctx->input_len) {
499+
// COVERAGE (MISSING) We do not cover too large inputs.
504500
return OTCRYPTO_BAD_ARGS;
505501
}
506502

@@ -532,6 +528,8 @@ status_t aes_gcm_update_encrypted_data(aes_gcm_context_t *ctx, size_t input_len,
532528
// Since we only generate ciphertext in full-block increments, no partial
533529
// blocks are possible in this case.
534530
if (*output_len % kGhashBlockNumBytes != 0) {
531+
// COVERAGE (SW ERR) Since we call aes_gcm_gctr before, this should
532+
// normally not be called.
535533
return OTCRYPTO_RECOV_ERR;
536534
}
537535
HARDENED_TRY(ghash_process_full_blocks(&ctx->ghash_ctx, /*partial_len=*/0,

sw/device/lib/crypto/impl/aes_gcm/ghash.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,8 @@ hardened_bool_t ghash_context_integrity_checksum_check(
169169
launder32(ghash_context_integrity_checksum(ghash_ctx))) {
170170
return kHardenedBoolTrue;
171171
}
172+
// COVERAGE (FI CM) We only provide correct encoded contexts, this is to check
173+
// for faults.
172174
return kHardenedBoolFalse;
173175
}
174176

sw/device/lib/crypto/impl/ecc/p256.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ hardened_bool_t p256_masked_scalar_checksum_check(
152152
if (scalar->checksum == launder32(p256_masked_scalar_checksum(scalar))) {
153153
return kHardenedBoolTrue;
154154
}
155+
// COVERAGE (FI CM) We only provide correct encoded scalars, this is to check
156+
// for faults.
155157
return kHardenedBoolFalse;
156158
}
157159

@@ -172,6 +174,8 @@ hardened_bool_t p256_ecdh_shared_key_checksum_check(
172174
if (key->checksum == launder32(p256_ecdh_shared_key_checksum(key))) {
173175
return kHardenedBoolTrue;
174176
}
177+
// COVERAGE (FI CM) We only provide correct encoded keys, this is to check for
178+
// faults.
175179
return kHardenedBoolFalse;
176180
}
177181

@@ -205,6 +209,7 @@ status_t p256_sideload_keygen_start(void) {
205209
status_t p256_sideload_attestation_keygen_start(
206210
const otcrypto_const_word32_buf_t *attestation_seed) {
207211
if (launder32(attestation_seed->len) > kDiceAttestationMaxSeedLength) {
212+
// COVERAGE (MISSING) We do not cover too long attestation seed inputs.
208213
return OTCRYPTO_BAD_ARGS;
209214
}
210215

@@ -356,6 +361,7 @@ status_t p256_sideload_attestation_sign_start(
356361
const uint32_t digest[kP256ScalarWords],
357362
const otcrypto_const_word32_buf_t *attestation_seed) {
358363
if (launder32(attestation_seed->len) > kDiceAttestationMaxSeedLength) {
364+
// COVERAGE (MISSING) We do not cover too long attestation seed inputs.
359365
return OTCRYPTO_BAD_ARGS;
360366
}
361367
HARDENED_CHECK_EQ(kHardenedBoolTrue, OTCRYPTO_CHECK_BUF(attestation_seed));

sw/device/lib/crypto/impl/ecc/p384.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,8 @@ hardened_bool_t p384_masked_scalar_checksum_check(
160160
if (scalar->checksum == launder32(p384_masked_scalar_checksum(scalar))) {
161161
return kHardenedBoolTrue;
162162
}
163+
// COVERAGE (FI CM) We only provide correct encoded scalars, this is to check
164+
// for faults.
163165
return kHardenedBoolFalse;
164166
}
165167

@@ -223,6 +225,8 @@ hardened_bool_t p384_ecdh_shared_key_checksum_check(
223225
if (key->checksum == launder32(p384_ecdh_shared_key_checksum(key))) {
224226
return kHardenedBoolTrue;
225227
}
228+
// COVERAGE (FI CM) We only provide correct encoded keys, this is to check for
229+
// faults.
226230
return kHardenedBoolFalse;
227231
}
228232

@@ -453,6 +457,8 @@ status_t p384_ecdh_finalize(p384_ecdh_shared_key_t *shared_key) {
453457
HARDENED_TRY(otbn_dmem_read(1, kOtbnVarOk, &ok));
454458
if (launder32(ok) != kHardenedBoolTrue) {
455459
HARDENED_TRY(otbn_dmem_sec_wipe());
460+
// COVERAGE (MISSING) We do not cover a negative ECDH check where a bad
461+
// public key is given.
456462
return OTCRYPTO_BAD_ARGS;
457463
}
458464
HARDENED_CHECK_EQ(ok, kHardenedBoolTrue);
@@ -526,6 +532,7 @@ status_t p384_point_on_curve_check(const p384_point_t *point,
526532
} else if (launder32(ins_cnt) == kModePointOnCurveCheckInvld1InsCnt) {
527533
HARDENED_CHECK_EQ(ins_cnt, kModePointOnCurveCheckInvld1InsCnt);
528534
} else {
535+
// COVERAGE (MISSING) We do not cover PointOnCurveCheckInvld2
529536
HARDENED_CHECK_EQ(ins_cnt, kModePointOnCurveCheckInvld2InsCnt);
530537
}
531538

0 commit comments

Comments
 (0)