Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused flags from old linking approach #264

Merged
merged 3 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 3 additions & 17 deletions lib/src/signed_video_h26x_auth.c
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,8 @@ verify_hashes_with_hash_list(signed_video_t *self,
latest_match_idx = compare_idx;
num_invalid_nalus_since_latest_match = 0;
// If the order is not correct, the validation status of the first NALU in the GOP should be
// 'N'. If that is the case, set |first_verification_not_authentic| to true and set
// |order_ok| to true for the next NALUs, so they are not affected by this issue.
// 'N'. If that is the case, set |order_ok| to true for the next NALUs, so they are not
// affected by this issue.
if (!order_ok) {
order_ok = true;
}
Expand Down Expand Up @@ -688,9 +688,7 @@ validate_authenticity(signed_video_t *self)
// part of the original stream not present in the file. Hence, mark as
// SV_AUTH_RESULT_SIGNATURE_PRESENT instead.
DEBUG_LOG("This first validation cannot be performed");
// Since we verify the linking hash twice we need to remove the set
// |first_verification_not_authentic|. Otherwise, the false failure leaks into the next GOP.
// Further, empty items marked 'M', may have been added at the beginning. These have no
// Empty items marked 'M', may have been added at the beginning. These have no
// meaning and may only confuse the user. These should be removed. This is handled in
// h26x_nalu_list_remove_missing_items().
h26x_nalu_list_remove_missing_items(self->nalu_list);
Expand Down Expand Up @@ -1025,18 +1023,6 @@ maybe_validate_gop(signed_video_t *self, h26x_nalu_t *nalu)
// validation in the middle of a stream. Now it is time to reset it.
validation_flags->is_first_validation = !validation_flags->signing_present;

if (validation_flags->reset_first_validation) {
validation_flags->is_first_validation = true;
bjornvolcker marked this conversation as resolved.
Show resolved Hide resolved
h26x_nalu_list_item_t *item = self->nalu_list->first_item;
while (item) {
if (item->nalu && item->nalu->is_first_nalu_in_gop) {
item->need_second_verification = false;
item->first_verification_not_authentic = false;
break;
}
item = item->next;
}
}
self->gop_info->verified_signature_hash = -1;
self->validation_flags.has_auth_result = true;

Expand Down
6 changes: 1 addition & 5 deletions lib/src/signed_video_h26x_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,7 @@ struct _h26x_nalu_list_item_t {
// Flags
bool taken_ownership_of_nalu; // Flag to indicate if the item has taken ownership of the |nalu|
// memory, hence need to free the memory if the item is released.
bool need_second_verification; // This NALU need another verification, either due to failures or
// because it is a chained hash, that is, used in two GOPs. The second verification is done with
// |second_hash|.
bool first_verification_not_authentic; // Marks the NALU as not authentic so the second one does
// not overwrite with an acceptable status.

bool has_been_decoded; // Marks a SEI as decoded. Decoding it twice might overwrite vital
// information.
bool used_in_gop_hash; // Marks the NALU as being part of a computed |gop_hash|.
Expand Down
19 changes: 4 additions & 15 deletions lib/src/signed_video_h26x_nalu_list.c
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,6 @@ h26x_nalu_list_item_print(const h26x_nalu_list_item_t *item)
// uint8_t hash[MAX_HASH_SIZE];
// uint8_t *second_hash;
// bool taken_ownership_of_nalu;
// bool need_second_verification;
// bool first_verification_not_authentic;
// bool has_been_decoded;
// bool used_in_gop_hash;

Expand All @@ -170,10 +168,8 @@ h26x_nalu_list_item_print(const h26x_nalu_list_item_t *item)
memcpy(validation_status_str, &item->validation_status, 1);

printf("NALU type = %s\n", nalu_type_str);
printf("validation_status = %s%s%s%s%s%s\n", validation_status_str,
printf("validation_status = %s%s%s%s\n", validation_status_str,
(item->taken_ownership_of_nalu ? ", taken_ownership_of_nalu" : ""),
(item->need_second_verification ? ", need_second_verification" : ""),
(item->first_verification_not_authentic ? ", first_verification_not_authentic" : ""),
(item->has_been_decoded ? ", has_been_decoded" : ""),
(item->used_in_gop_hash ? ", used_in_gop_hash" : ""));
sv_print_hex_data(item->hash, item->hash_size, "item->hash ");
Expand Down Expand Up @@ -406,9 +402,8 @@ h26x_nalu_list_add_missing(h26x_nalu_list_t *list,
return status;
}

/* Removes 'M' items present at the beginning of the |list|. The |first_verification_not_authentic|
* flag is reset on all items until we find the first pending item, inclusive. Further, a decoded
* SEI is marked as 'U' since it is not associated with this recording. The screening keeps going
/* Removes 'M' items present at the beginning of the |list|. A decoded SEI is marked
* as 'U' since it is not associated with this recording. The screening keeps going
* until we find the decoded SEI. */
void
h26x_nalu_list_remove_missing_items(h26x_nalu_list_t *list)
Expand All @@ -420,8 +415,6 @@ h26x_nalu_list_remove_missing_items(h26x_nalu_list_t *list)
h26x_nalu_list_item_t *item = list->first_item;
while (item && !(found_first_pending_nalu && found_decoded_sei)) {
// Reset the invalid verification failure if we have not past the first pending item.

if (!found_first_pending_nalu) item->first_verification_not_authentic = false;
// Remove the missing NALU in the front.
if (item->validation_status == 'M' && (item == list->first_item)) {
const h26x_nalu_list_item_t *item_to_remove = item;
Expand Down Expand Up @@ -492,11 +485,7 @@ h26x_nalu_list_get_stats(const h26x_nalu_list_t *list,
// happens for out-of-sync SEIs for example.
has_valid_nalus |= !(item->nalu && item->nalu->is_gop_sei);
}
if (item->validation_status == 'P') {
// Count NALUs that were verified successfully the first time and waiting for a second
// verification.
has_valid_nalus |= item->need_second_verification && !item->first_verification_not_authentic;
}

item = item->next;
}

Expand Down
3 changes: 1 addition & 2 deletions lib/src/signed_video_h26x_nalu_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ h26x_nalu_list_add_missing(h26x_nalu_list_t* list,
* @brief Removes 'M' items present at the beginning of a |list|
*
* There are scenarios when missing items are added to the front of the |list|, when the framework
* actually could not verify the hashes. This function removes them and resets the flag
* |first_verification_not_authentic| of non-pending items. Further, marks the decoded SEI as 'U',
* actually could not verify the hashes. This function marks the decoded SEI as 'U',
* even if it could be verified, because it is not associated with this recording.
*
* @param list The |list| to remove items from.
Expand Down
62 changes: 62 additions & 0 deletions tests/check/check_signed_video_auth.c
Original file line number Diff line number Diff line change
Expand Up @@ -1053,6 +1053,67 @@ START_TEST(add_one_sei_nalu_after_signing)
}
END_TEST

/* Test description
* Verify that we get a valid authentication if first two SEIs delayed and GOPs that belegs to those
* SEIs are removed.
*/
START_TEST(remove_two_gop_in_start_of_stream)
{
// Create a list of NAL Units given the input string.
test_stream_t *list = create_signed_nalus("IPIPIPPPIPPPPIPPIP", settings[_i]);
test_stream_check_types(list, "IPISPISPPPISPPPPISPPISP");

// Delay the first SEI by removing it from position 4 and appending it at position 5.
test_stream_item_t *sei = test_stream_item_remove(list, 4);
test_stream_item_check_type(sei, 'S');
test_stream_check_types(list, "IPIPISPPPISPPPPISPPISP");
test_stream_append_item(list, sei, 5);
test_stream_check_types(list, "IPIPISSPPPISPPPPISPPISP");
// Delay the second SEI by removing it from position 7 and appending it at position 8.
sei = test_stream_item_remove(list, 7);
test_stream_item_check_type(sei, 'S');
test_stream_check_types(list, "IPIPISPPPISPPPPISPPISP");
test_stream_append_item(list, sei, 8);
test_stream_check_types(list, "IPIPISPPSPISPPPPISPPISP");

// Remove the first 2 GOPs.
test_stream_t *removed_list = test_stream_pop(list, 4);
test_stream_check_types(removed_list, "IPIP");
test_stream_free(removed_list);
test_stream_check_types(list, "ISPPSPISPPPPISPPISP");

signed_video_accumulated_validation_t final_validation = {
SV_AUTH_RESULT_NOT_OK, false, 19, 16, 3, SV_PUBKEY_VALIDATION_NOT_FEASIBLE, true, 0, 0};
// ISPPSPISPPPPISPPISP
//
// IS PU -> (signature)
// ISPPS NUNN. -> (invalid)
// PIS MMMNP. -> (invalid)
// ISPPPPIS ......P. -> (valid)
// ISPPIS ....P. -> (valid)
struct validation_stats expected = {.valid_gops = 2,
.invalid_gops = 2,
bjornvolcker marked this conversation as resolved.
Show resolved Hide resolved
.pending_nalus = 4,
.missed_nalus = 2,
.has_signature = 1,
.final_validation = &final_validation};
// ISPPSPISPPPPISPPISP
// IS PMU -> (signature)
// ISPPS NMUNN. -> (invalid)
// PIS MMM.P. -> (valid with missing info)
// ISPPPPIS ......P. -> (valid)
// ISPPIS ....P. -> (valid)
if (settings[_i].auth_level == SV_AUTHENTICITY_LEVEL_FRAME) {
expected.valid_gops_with_missing_info = 1;
expected.invalid_gops = 1;
expected.final_validation->authenticity = SV_AUTH_RESULT_NOT_OK;
}
validate_nalu_list(NULL, list, expected, true);

test_stream_free(list);
}
END_TEST

/* Test description
* Verify that we do get a valid authentication if the signing on the camera was reset. From a
* signed video perspective this action is correct as long as recorded NAL Units are not transmitted
Expand Down Expand Up @@ -2152,6 +2213,7 @@ signed_video_suite(void)
tcase_add_loop_test(tc, lost_g_and_gop_with_late_sei_arrival, s, e);
tcase_add_loop_test(tc, lost_all_nalus_between_two_seis, s, e);
tcase_add_loop_test(tc, add_one_sei_nalu_after_signing, s, e);
tcase_add_loop_test(tc, remove_two_gop_in_start_of_stream, s, e);
tcase_add_loop_test(tc, camera_reset_on_signing_side, s, e);
tcase_add_loop_test(tc, detect_change_of_public_key, s, e);
tcase_add_loop_test(tc, fast_forward_stream_with_reset, s, e);
Expand Down