From e255e190c5f1d8224f25d7f6ba530273439dfb88 Mon Sep 17 00:00:00 2001 From: Yunus Kurt Date: Wed, 4 Dec 2024 09:34:31 +0100 Subject: [PATCH] Remove unused flags from old linking approach (#264) Remove unused flags from old linking approach With the transition to the new linking principle now complete, this commit removes flags that were needed for the previous approach. These flags are no longer relevant or used in the current implementation. --- lib/src/signed_video_h26x_auth.c | 20 ++------- lib/src/signed_video_h26x_internal.h | 6 +-- lib/src/signed_video_h26x_nalu_list.c | 19 ++------ lib/src/signed_video_h26x_nalu_list.h | 3 +- tests/check/check_signed_video_auth.c | 64 +++++++++++++++++++++++++++ 5 files changed, 73 insertions(+), 39 deletions(-) diff --git a/lib/src/signed_video_h26x_auth.c b/lib/src/signed_video_h26x_auth.c index 9cd785b..1616b5a 100644 --- a/lib/src/signed_video_h26x_auth.c +++ b/lib/src/signed_video_h26x_auth.c @@ -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; } @@ -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); @@ -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; - 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; diff --git a/lib/src/signed_video_h26x_internal.h b/lib/src/signed_video_h26x_internal.h index af8f651..9bf34dd 100644 --- a/lib/src/signed_video_h26x_internal.h +++ b/lib/src/signed_video_h26x_internal.h @@ -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|. diff --git a/lib/src/signed_video_h26x_nalu_list.c b/lib/src/signed_video_h26x_nalu_list.c index 72ce35b..0a9e3db 100644 --- a/lib/src/signed_video_h26x_nalu_list.c +++ b/lib/src/signed_video_h26x_nalu_list.c @@ -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; @@ -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 "); @@ -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) @@ -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; @@ -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; } diff --git a/lib/src/signed_video_h26x_nalu_list.h b/lib/src/signed_video_h26x_nalu_list.h index 760dc5e..2ff4c2e 100644 --- a/lib/src/signed_video_h26x_nalu_list.h +++ b/lib/src/signed_video_h26x_nalu_list.h @@ -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. diff --git a/tests/check/check_signed_video_auth.c b/tests/check/check_signed_video_auth.c index f118bec..a81d19c 100644 --- a/tests/check/check_signed_video_auth.c +++ b/tests/check/check_signed_video_auth.c @@ -1053,6 +1053,69 @@ 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) + // TODO: There is a flaw in this scenario. It would be best to expect no invalid GOPs in the + // stream, as the first validation should reset the validation status to ensure consistency. + struct validation_stats expected = {.valid_gops = 2, + .invalid_gops = 2, + .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 @@ -2152,6 +2215,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);