dsp: firmware_update: Reimplement decode_pldm_package_header_info_errno()

Do so in terms of the msgbuf APIs for safety, correctness, ergonomics
and performance.

decode_pldm_package_header_info() is re-implemented in terms of the
reworked API for decode_pldm_package_header_info_errno(). This gives us
a common implementation that will be exploited by future changes, with
the accommodations kept on the exceptional path.

gitlint-ignore: T1
Change-Id: Ia57fa007e5896b63e18063704787bbbc3c89f8e2
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
diff --git a/src/api.h b/src/api.h
index 643fbff..8faee05 100644
--- a/src/api.h
+++ b/src/api.h
@@ -26,8 +26,10 @@
 
 	assert(err < 0);
 	switch (err) {
-	case -EINVAL:
 	case -EBADMSG:
+	case -EINVAL:
+	case -EPROTO:
+	case -EUCLEAN:
 		rc = PLDM_ERROR_INVALID_DATA;
 		break;
 	case -ENOMSG:
@@ -36,6 +38,9 @@
 	case -EOVERFLOW:
 		rc = PLDM_ERROR_INVALID_LENGTH;
 		break;
+	case -ENOTSUP:
+		rc = PLDM_ERROR;
+		break;
 	default:
 		assert(false);
 		rc = PLDM_ERROR;
diff --git a/src/dsp/firmware_update.c b/src/dsp/firmware_update.c
index ea8cfd2..d6e4e98 100644
--- a/src/dsp/firmware_update.c
+++ b/src/dsp/firmware_update.c
@@ -326,58 +326,151 @@
 	}
 }
 
+// Identifier for a valid PLDM Firmware Update Package for Version 1.0.x as per
+// spec
+static const uint8_t PLDM_FWUP_HDR_IDENTIFIER_V1[PLDM_FWUP_UUID_LENGTH] = {
+	0xF0, 0x18, 0x87, 0x8C, 0xCB, 0x7D, 0x49, 0x43,
+	0x98, 0x00, 0xA0, 0x2F, 0x05, 0x9A, 0xCA, 0x02
+};
+
+/* TODO: Remove struct pldm_package_header_information from public header, rename padded struct, drop typedef */
+struct pldm__package_header_information {
+	uint8_t package_header_identifier[PLDM_FWUP_UUID_LENGTH];
+	uint8_t package_header_format_revision;
+	uint8_t package_release_date_time[PLDM_TIMESTAMP104_SIZE];
+	uint16_t component_bitmap_bit_length;
+	uint8_t package_version_string_type;
+	struct variable_field package_version_string;
+	struct variable_field areas;
+	struct variable_field images;
+};
+typedef struct pldm__package_header_information
+	pldm_package_header_information_pad;
+
+#define PLDM_FWUP_PACKAGE_HEADER_FIXED_SIZE 36
 static int decode_pldm_package_header_info_errno(
 	const void *data, size_t length,
-	struct pldm_package_header_information *package_header_info,
-	struct variable_field *package_version_str)
+	pldm_package_header_information_pad *header)
 {
-	if (data == NULL || package_header_info == NULL ||
-	    package_version_str == NULL) {
+	uint32_t package_header_checksum = 0;
+	size_t package_header_variable_size;
+	size_t package_header_payload_size;
+	size_t package_header_areas_size;
+	uint16_t package_header_size;
+	PLDM_MSGBUF_DEFINE_P(buf);
+	int checksums = 1;
+	int rc;
+
+	if (!data || !header) {
 		return -EINVAL;
 	}
 
-	if (length < sizeof(struct pldm_package_header_information)) {
-		return -EOVERFLOW;
+	rc = pldm_msgbuf_init_errno(buf, PLDM_FWUP_PACKAGE_HEADER_FIXED_SIZE,
+				    data, length);
+	if (rc) {
+		return rc;
 	}
 
-	struct pldm_package_header_information *data_header =
-		(struct pldm_package_header_information *)(data);
-
-	if (!is_string_type_valid(data_header->package_version_string_type) ||
-	    (data_header->package_version_string_length == 0)) {
-		return -EBADMSG;
+	rc = pldm_msgbuf_extract_array(
+		buf, sizeof(header->package_header_identifier),
+		header->package_header_identifier,
+		sizeof(header->package_header_identifier));
+	if (rc) {
+		return pldm_msgbuf_discard(buf, rc);
 	}
 
-	if (length < sizeof(struct pldm_package_header_information) +
-			     data_header->package_version_string_length) {
-		return -EOVERFLOW;
+	static_assert(sizeof(PLDM_FWUP_HDR_IDENTIFIER_V1) ==
+			      sizeof(header->package_header_identifier),
+		      "UUID field size");
+	if (memcmp(PLDM_FWUP_HDR_IDENTIFIER_V1,
+		   header->package_header_identifier,
+		   sizeof(PLDM_FWUP_HDR_IDENTIFIER_V1)) != 0) {
+		return pldm_msgbuf_discard(buf, -ENOTSUP);
 	}
 
-	if ((data_header->component_bitmap_bit_length %
-	     PLDM_FWUP_COMPONENT_BITMAP_MULTIPLE) != 0) {
-		return -EBADMSG;
+	rc = pldm_msgbuf_extract(buf, header->package_header_format_revision);
+	if (rc) {
+		return pldm_msgbuf_discard(buf, rc);
+	}
+	if (header->package_header_format_revision != 0x01) {
+		return pldm_msgbuf_discard(buf, -EBADMSG);
 	}
 
-	memcpy(package_header_info->uuid, data_header->uuid,
-	       sizeof(data_header->uuid));
-	package_header_info->package_header_format_version =
-		data_header->package_header_format_version;
-	package_header_info->package_header_size =
-		le16toh(data_header->package_header_size);
-	memcpy(package_header_info->package_release_date_time,
-	       data_header->package_release_date_time,
-	       sizeof(data_header->package_release_date_time));
-	package_header_info->component_bitmap_bit_length =
-		le16toh(data_header->component_bitmap_bit_length);
-	package_header_info->package_version_string_type =
-		data_header->package_version_string_type;
-	package_header_info->package_version_string_length =
-		data_header->package_version_string_length;
-	package_version_str->ptr =
-		(const uint8_t *)data +
-		sizeof(struct pldm_package_header_information);
-	package_version_str->length =
-		package_header_info->package_version_string_length;
+	rc = pldm_msgbuf_extract(buf, package_header_size);
+	if (rc) {
+		return pldm_msgbuf_discard(buf, rc);
+	}
+
+	rc = pldm_msgbuf_extract_array(
+		buf, sizeof(header->package_release_date_time),
+		header->package_release_date_time,
+		sizeof(header->package_release_date_time));
+	if (rc) {
+		return pldm_msgbuf_discard(buf, rc);
+	}
+
+	rc = pldm_msgbuf_extract(buf, header->component_bitmap_bit_length);
+	if (rc) {
+		return pldm_msgbuf_discard(buf, rc);
+	}
+	if (header->component_bitmap_bit_length & 7) {
+		return pldm_msgbuf_discard(buf, -EPROTO);
+	}
+
+	rc = pldm_msgbuf_extract(buf, header->package_version_string_type);
+	if (rc) {
+		return pldm_msgbuf_discard(buf, rc);
+	}
+	if (!is_string_type_valid(header->package_version_string_type)) {
+		return pldm_msgbuf_discard(buf, -EPROTO);
+	}
+
+	rc = pldm_msgbuf_extract_uint8_to_size(
+		buf, header->package_version_string.length);
+	if (rc) {
+		return pldm_msgbuf_discard(buf, rc);
+	}
+	if (header->package_version_string.length == 0) {
+		return pldm_msgbuf_discard(buf, -EBADMSG);
+	}
+
+	pldm_msgbuf_span_required(buf, header->package_version_string.length,
+				  (void **)&header->package_version_string.ptr);
+
+	if (package_header_size < (PLDM_FWUP_PACKAGE_HEADER_FIXED_SIZE + 3 +
+				   checksums * sizeof(uint32_t))) {
+		return pldm_msgbuf_discard(buf, -EBADMSG);
+	}
+	package_header_payload_size =
+		package_header_size - (checksums * sizeof(uint32_t));
+	package_header_variable_size = package_header_payload_size -
+				       PLDM_FWUP_PACKAGE_HEADER_FIXED_SIZE;
+
+	if (package_header_variable_size <
+	    header->package_version_string.length) {
+		return pldm_msgbuf_discard(buf, -EOVERFLOW);
+	}
+
+	package_header_areas_size = package_header_variable_size -
+				    header->package_version_string.length;
+	rc = pldm_msgbuf_span_required(buf, package_header_areas_size,
+				       (void **)&header->areas.ptr);
+	if (rc) {
+		return pldm_msgbuf_discard(buf, rc);
+	}
+	header->areas.length = package_header_areas_size;
+
+	pldm_msgbuf_extract(buf, package_header_checksum);
+
+	rc = pldm_msgbuf_complete(buf);
+	if (rc) {
+		return rc;
+	}
+
+	if (package_header_checksum !=
+	    crc32(data, package_header_payload_size)) {
+		return -EUCLEAN;
+	}
 
 	return 0;
 }
@@ -388,14 +481,42 @@
 	struct pldm_package_header_information *package_header_info,
 	struct variable_field *package_version_str)
 {
+	pldm_package_header_information_pad header;
 	int rc;
 
-	rc = decode_pldm_package_header_info_errno(
-		data, length, package_header_info, package_version_str);
+	if (!data || !package_header_info || !package_version_str) {
+		return PLDM_ERROR_INVALID_DATA;
+	}
+
+	rc = decode_pldm_package_header_info_errno(data, length, &header);
 	if (rc < 0) {
 		return pldm_xlate_errno(rc);
 	}
 
+	static_assert(sizeof(package_header_info->uuid) ==
+			      sizeof(header.package_header_identifier),
+		      "UUID field size");
+	memcpy(package_header_info->uuid, header.package_header_identifier,
+	       sizeof(header.package_header_identifier));
+	package_header_info->package_header_format_version =
+		header.package_header_format_revision;
+	memcpy(&package_header_info->package_header_size, data + 17,
+	       sizeof(package_header_info->package_header_size));
+	LE16TOH(package_header_info->package_header_size);
+	static_assert(sizeof(package_header_info->package_release_date_time) ==
+			      sizeof(header.package_release_date_time),
+		      "TIMESTAMP104 field size");
+	memcpy(package_header_info->package_release_date_time,
+	       header.package_release_date_time,
+	       sizeof(header.package_release_date_time));
+	package_header_info->component_bitmap_bit_length =
+		header.component_bitmap_bit_length;
+	package_header_info->package_version_string_type =
+		header.package_version_string_type;
+	package_header_info->package_version_string_length =
+		header.package_version_string.length;
+	*package_version_str = header.package_version_string;
+
 	return PLDM_SUCCESS;
 }
 
diff --git a/tests/dsp/firmware_update.cpp b/tests/dsp/firmware_update.cpp
index 05b8dd1..e12b4de 100644
--- a/tests/dsp/firmware_update.cpp
+++ b/tests/dsp/firmware_update.cpp
@@ -48,26 +48,25 @@
                                              0x05, 0x9a, 0xca, 0x02};
 
 static constexpr uint8_t PLDM_FWUP_PACKAGE_HEADER_FORMAT_REVISION_V1_0 = 0x01;
+static constexpr size_t PLDM_FWUP_PACKAGE_HEADER_EMPTY_SIZE_V1_0 = 43;
+
+static constexpr std::array<uint8_t, PLDM_TIMESTAMP104_SIZE>
+    testPackageReleaseDateTime{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                               0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00};
 
 TEST(DecodePackageHeaderInfo, goodPath)
 {
-    // Random PackageHeaderSize
-    constexpr uint16_t pkgHeaderSize = 303;
-    // PackageReleaseDateTime - "25/12/2021 00:00:00"
-    std::array<uint8_t, PLDM_TIMESTAMP104_SIZE> package_release_date_time{
-        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00};
     constexpr uint16_t componentBitmapBitLength = 8;
-    // PackageVersionString
     constexpr std::string_view packageVersionStr{"OpenBMCv1.0"};
     constexpr size_t packageHeaderSize =
-        sizeof(pldm_package_header_information) + packageVersionStr.size();
+        PLDM_FWUP_PACKAGE_HEADER_EMPTY_SIZE_V1_0 + packageVersionStr.size();
 
     constexpr std::array<uint8_t, packageHeaderSize> packagerHeaderInfo{
-        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0, 0x2f,
-        0x05, 0x9a, 0xca, 0x02, 0x01, 0x2f, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x08, 0x00, 0x01, 0x0b,
-        0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76, 0x31, 0x2e, 0x30};
+        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0,
+        0x2f, 0x05, 0x9a, 0xca, 0x02, 0x01, 0x36, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x08,
+        0x00, 0x01, 0x0b, 0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76,
+        0x31, 0x2e, 0x30, 0x00, 0x00, 0x00, 0x61, 0xe3, 0x64, 0x6e};
     pldm_package_header_information pkgHeader{};
     variable_field packageVersion{};
 
@@ -75,19 +74,19 @@
                                               packagerHeaderInfo.size(),
                                               &pkgHeader, &packageVersion);
 
-    EXPECT_EQ(rc, PLDM_SUCCESS);
+    ASSERT_EQ(rc, PLDM_SUCCESS);
     EXPECT_EQ(true,
               std::equal(pkgHeader.uuid, pkgHeader.uuid + PLDM_FWUP_UUID_LENGTH,
                          PLDM_FWUP_PACKAGE_HEADER_IDENTIFIER_V1_0.begin(),
                          PLDM_FWUP_PACKAGE_HEADER_IDENTIFIER_V1_0.end()));
     EXPECT_EQ(pkgHeader.package_header_format_version,
               PLDM_FWUP_PACKAGE_HEADER_FORMAT_REVISION_V1_0);
-    EXPECT_EQ(pkgHeader.package_header_size, pkgHeaderSize);
+    EXPECT_EQ(pkgHeader.package_header_size, packageHeaderSize);
     EXPECT_EQ(true, std::equal(pkgHeader.package_release_date_time,
                                pkgHeader.package_release_date_time +
                                    PLDM_TIMESTAMP104_SIZE,
-                               package_release_date_time.begin(),
-                               package_release_date_time.end()));
+                               testPackageReleaseDateTime.begin(),
+                               testPackageReleaseDateTime.end()));
     EXPECT_EQ(pkgHeader.component_bitmap_bit_length, componentBitmapBitLength);
     EXPECT_EQ(pkgHeader.package_version_string_type, PLDM_STR_TYPE_ASCII);
     EXPECT_EQ(pkgHeader.package_version_string_length,
@@ -103,13 +102,14 @@
 {
     constexpr std::string_view packageVersionStr{"OpenBMCv1.0"};
     constexpr size_t packageHeaderSize =
-        sizeof(pldm_package_header_information) + packageVersionStr.size();
+        PLDM_FWUP_PACKAGE_HEADER_EMPTY_SIZE_V1_0 + packageVersionStr.size();
 
     constexpr std::array<uint8_t, packageHeaderSize> packagerHeaderInfo{
-        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0, 0x2f,
-        0x05, 0x9a, 0xca, 0x02, 0x01, 0x2f, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x08, 0x00, 0x01, 0x0b,
-        0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76, 0x31, 0x2e, 0x30};
+        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0,
+        0x2f, 0x05, 0x9a, 0xca, 0x02, 0x01, 0x36, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x08,
+        0x00, 0x01, 0x0b, 0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76,
+        0x31, 0x2e, 0x30, 0x00, 0x00, 0x00, 0x96, 0x8b, 0x5b, 0xcc};
 
     pldm_package_header_information packageHeader{};
     variable_field packageVersion{};
@@ -134,35 +134,95 @@
 {
     constexpr std::string_view packageVersionStr{"OpenBMCv1.0"};
     constexpr size_t packageHeaderSize =
-        sizeof(pldm_package_header_information) + packageVersionStr.size();
+        PLDM_FWUP_PACKAGE_HEADER_EMPTY_SIZE_V1_0 + packageVersionStr.size();
 
     constexpr std::array<uint8_t, packageHeaderSize> packagerHeaderInfo{
-        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0, 0x2f,
-        0x05, 0x9a, 0xca, 0x02, 0x01, 0x2f, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x08, 0x00, 0x01, 0x0b,
-        0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76, 0x31, 0x2e, 0x30};
+        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0,
+        0x2f, 0x05, 0x9a, 0xca, 0x02, 0x01, 0x36, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x08,
+        0x00, 0x01, 0x0b, 0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76,
+        0x31, 0x2e, 0x30, 0x00, 0x00, 0x00, 0x96, 0x8b, 0x5b, 0xcc};
 
     pldm_package_header_information packageHeader{};
     variable_field packageVersion{};
     int rc = 0;
 
-    rc = decode_pldm_package_header_info(
-        packagerHeaderInfo.data(), sizeof(pldm_package_header_information) - 1,
-        &packageHeader, &packageVersion);
+    rc = decode_pldm_package_header_info(packagerHeaderInfo.data(), 0,
+                                         &packageHeader, &packageVersion);
     EXPECT_EQ(rc, PLDM_ERROR_INVALID_LENGTH);
+
+    rc = decode_pldm_package_header_info(packagerHeaderInfo.data(), 35,
+                                         &packageHeader, &packageVersion);
+    EXPECT_EQ(rc, PLDM_ERROR_INVALID_LENGTH);
+
+    rc = decode_pldm_package_header_info(packagerHeaderInfo.data(), 36,
+                                         &packageHeader, &packageVersion);
+    EXPECT_EQ(rc, PLDM_ERROR_INVALID_LENGTH);
+
+    rc = decode_pldm_package_header_info(packagerHeaderInfo.data(),
+                                         packagerHeaderInfo.size() - 1,
+                                         &packageHeader, &packageVersion);
+    EXPECT_EQ(rc, PLDM_ERROR_INVALID_LENGTH);
+}
+
+TEST(DecodePackageHeaderInfo, unspecifiedPackageHeaderIdentifier)
+{
+    constexpr std::string_view packageVersionStr{"OpenBMCv1.0"};
+    constexpr size_t packageHeaderSize =
+        PLDM_FWUP_PACKAGE_HEADER_EMPTY_SIZE_V1_0 + packageVersionStr.size();
+
+    constexpr std::array<uint8_t, packageHeaderSize> packagerHeaderInfo{
+        0xff, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0,
+        0x2f, 0x05, 0x9a, 0xca, 0x02, 0x01, 0x36, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x08,
+        0x00, 0x01, 0x0b, 0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76,
+        0x31, 0x2e, 0x30, 0x00, 0x00, 0x00, 0x96, 0x8b, 0x5b, 0xcc};
+
+    pldm_package_header_information packageHeader{};
+    variable_field packageVersion{};
+    int rc = 0;
+
+    rc = decode_pldm_package_header_info(packagerHeaderInfo.data(),
+                                         packagerHeaderInfo.size(),
+                                         &packageHeader, &packageVersion);
+    EXPECT_EQ(rc, PLDM_ERROR);
+}
+
+TEST(DecodePackageHeaderInfo, incongruentPackageHeaderFormatRevision)
+{
+    constexpr std::string_view packageVersionStr{"OpenBMCv1.0"};
+    constexpr size_t packageHeaderSize =
+        PLDM_FWUP_PACKAGE_HEADER_EMPTY_SIZE_V1_0 + 1 + packageVersionStr.size();
+
+    constexpr std::array<uint8_t, packageHeaderSize> packagerHeaderInfo{
+        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0,
+        0x2f, 0x05, 0x9a, 0xca, 0x02, 0x02, 0x37, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x08,
+        0x00, 0x01, 0x0b, 0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76,
+        0x31, 0x2e, 0x30, 0x00, 0x00, 0x00, 0x00, 0x96, 0x8b, 0x5b, 0xcc};
+
+    pldm_package_header_information packageHeader{};
+    variable_field packageVersion{};
+    int rc = 0;
+
+    rc = decode_pldm_package_header_info(packagerHeaderInfo.data(),
+                                         packagerHeaderInfo.size(),
+                                         &packageHeader, &packageVersion);
+    EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
 }
 
 TEST(DecodePackageHeaderInfo, invalidPackageVersionStringType)
 {
     constexpr std::string_view packageVersionStr{"OpenBMCv1.0"};
     constexpr size_t packageHeaderSize =
-        sizeof(pldm_package_header_information) + packageVersionStr.size();
+        PLDM_FWUP_PACKAGE_HEADER_EMPTY_SIZE_V1_0 + packageVersionStr.size();
 
     constexpr std::array<uint8_t, packageHeaderSize> invalidPackagerHeaderInfo{
-        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0, 0x2f,
-        0x05, 0x9a, 0xca, 0x02, 0x02, 0x2f, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x08, 0x00, 0x06, 0x0b,
-        0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76, 0x31, 0x2e, 0x30};
+        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0,
+        0x2f, 0x05, 0x9a, 0xca, 0x02, 0x01, 0x36, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x08,
+        0x00, 0x06, 0x0b, 0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76,
+        0x31, 0x2e, 0x30, 0x00, 0x00, 0x00, 0x96, 0x8b, 0x5b, 0xcc};
 
     pldm_package_header_information packageHeader{};
     variable_field packageVersion{};
@@ -178,13 +238,14 @@
 {
     constexpr std::string_view packageVersionStr{"OpenBMCv1.0"};
     constexpr size_t packageHeaderSize =
-        sizeof(pldm_package_header_information) + packageVersionStr.size();
+        PLDM_FWUP_PACKAGE_HEADER_EMPTY_SIZE_V1_0 + packageVersionStr.size();
 
     constexpr std::array<uint8_t, packageHeaderSize> invalidPackagerHeaderInfo{
-        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0, 0x2f,
-        0x05, 0x9a, 0xca, 0x02, 0x02, 0x2f, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x08, 0x00, 0x01, 0x00,
-        0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76, 0x31, 0x2e, 0x30};
+        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0,
+        0x2f, 0x05, 0x9a, 0xca, 0x02, 0x01, 0x36, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x08,
+        0x00, 0x01, 0x00, 0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76,
+        0x31, 0x2e, 0x30, 0x00, 0x00, 0x00, 0x96, 0x8b, 0x5b, 0xcc};
 
     pldm_package_header_information packageHeader{};
     variable_field packageVersion{};
@@ -200,15 +261,14 @@
 {
     constexpr std::string_view packageVersionStr{"OpenBMCv1.0"};
     constexpr size_t packageHeaderSize =
-        sizeof(pldm_package_header_information) + packageVersionStr.size();
+        PLDM_FWUP_PACKAGE_HEADER_EMPTY_SIZE_V1_0 + packageVersionStr.size();
 
-    constexpr std::array<uint8_t, packageHeaderSize - 1>
-        invalidPackagerHeaderInfo{
-            0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00,
-            0xa0, 0x2f, 0x05, 0x9a, 0xca, 0x02, 0x02, 0x2f, 0x01, 0x00,
-            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5,
-            0x07, 0x00, 0x08, 0x00, 0x01, 0x0b, 0x4f, 0x70, 0x65, 0x6e,
-            0x42, 0x4d, 0x43, 0x76, 0x31, 0x2e};
+    constexpr std::array<uint8_t, packageHeaderSize> invalidPackagerHeaderInfo{
+        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0,
+        0x2f, 0x05, 0x9a, 0xca, 0x02, 0x01, 0x36, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x08,
+        0x00, 0x01, 0x10, 0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76,
+        0x31, 0x2e, 0x30, 0x00, 0x00, 0x00, 0x96, 0x8b, 0x5b, 0xcc};
 
     pldm_package_header_information packageHeader{};
     variable_field packageVersion{};
@@ -224,13 +284,37 @@
 {
     constexpr std::string_view packageVersionStr{"OpenBMCv1.0"};
     constexpr size_t packageHeaderSize =
-        sizeof(pldm_package_header_information) + packageVersionStr.size();
+        PLDM_FWUP_PACKAGE_HEADER_EMPTY_SIZE_V1_0 + packageVersionStr.size();
 
     constexpr std::array<uint8_t, packageHeaderSize> invalidPackagerHeaderInfo{
-        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0, 0x2f,
-        0x05, 0x9a, 0xca, 0x02, 0x02, 0x2f, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x09, 0x00, 0x01, 0x0b,
-        0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76, 0x31, 0x2e, 0x30};
+        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0,
+        0x2f, 0x05, 0x9a, 0xca, 0x02, 0x01, 0x36, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x07,
+        0x00, 0x01, 0x0b, 0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76,
+        0x31, 0x2e, 0x30, 0x00, 0x00, 0x00, 0x96, 0x8b, 0x5b, 0xcc};
+
+    pldm_package_header_information packageHeader{};
+    variable_field packageVersion{};
+    int rc = 0;
+
+    rc = decode_pldm_package_header_info(invalidPackagerHeaderInfo.data(),
+                                         invalidPackagerHeaderInfo.size(),
+                                         &packageHeader, &packageVersion);
+    EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
+}
+
+TEST(DecodePackageHeaderInfo, badChecksum)
+{
+    constexpr std::string_view packageVersionStr{"OpenBMCv1.0"};
+    constexpr size_t packageHeaderSize =
+        PLDM_FWUP_PACKAGE_HEADER_EMPTY_SIZE_V1_0 + packageVersionStr.size();
+
+    constexpr std::array<uint8_t, packageHeaderSize> invalidPackagerHeaderInfo{
+        0xf0, 0x18, 0x87, 0x8c, 0xcb, 0x7d, 0x49, 0x43, 0x98, 0x00, 0xa0,
+        0x2f, 0x05, 0x9a, 0xca, 0x02, 0x01, 0x36, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x19, 0x0c, 0xe5, 0x07, 0x00, 0x10,
+        0x00, 0x01, 0x0b, 0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x76,
+        0x31, 0x2e, 0x30, 0x00, 0x00, 0x00, 0x96, 0x8b, 0x5b, 0xcc};
 
     pldm_package_header_information packageHeader{};
     variable_field packageVersion{};