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);