From aeebd068f5a3ec1dbce8de796c0c517b3d16aff3 Mon Sep 17 00:00:00 2001 From: Cyrus <24922493+cyrus-dev@users.noreply.github.com> Date: Mon, 25 Feb 2019 10:37:11 -0500 Subject: [PATCH] [#72] Supply Chain Validator fix and update (#94) * This fix correct an IllegalStateException for the SupplyChainValider when all policy settings are true. When trying to remove a value from the iterator in the validator, the item was null and caused this issue. This also takes out the Platform Serial as a required field. Closes #72 * checking in a small change that puts back in a line for checking the serial number. It has been changed from FAIL to PASS however. * Committing updated changes. * Committing test certs for changes. * Updated unit tests * Fixing travis checkstyle for URISyntaxException missing from UnitTests --- .../SupplyChainCredentialValidator.java | 71 ++++++++++++++---- .../SupplyChainCredentialValidatorTest.java | 49 ++++++++++-- ...ple_paccor_output_not_specified_values.txt | 34 +++++++++ .../paccor_platform_cert.crt | Bin 0 -> 1102 bytes 4 files changed, 132 insertions(+), 22 deletions(-) create mode 100755 HIRS_Utils/src/test/resources/hirs/validation/sample_paccor_output_not_specified_values.txt create mode 100644 HIRS_Utils/src/test/resources/validation/platform_credentials_2/paccor_platform_cert.crt diff --git a/HIRS_Utils/src/main/java/hirs/validation/SupplyChainCredentialValidator.java b/HIRS_Utils/src/main/java/hirs/validation/SupplyChainCredentialValidator.java index 79ab39f6..fca96339 100644 --- a/HIRS_Utils/src/main/java/hirs/validation/SupplyChainCredentialValidator.java +++ b/HIRS_Utils/src/main/java/hirs/validation/SupplyChainCredentialValidator.java @@ -142,6 +142,7 @@ public final class SupplyChainCredentialValidator implements CredentialValidator * @param acceptExpired whether or not to accept expired certificates as valid. * @return The result of the validation. */ + @Override public AppraisalStatus validatePlatformCredential(final PlatformCredential pc, final KeyStore trustStore, final boolean acceptExpired) { @@ -210,6 +211,7 @@ public final class SupplyChainCredentialValidator implements CredentialValidator * identity request as the platform credential. * @return The result of the validation. */ + @Override public AppraisalStatus validatePlatformCredentialAttributes( final PlatformCredential platformCredential, final DeviceInfoReport deviceInfoReport, @@ -397,12 +399,12 @@ public final class SupplyChainCredentialValidator implements CredentialValidator // check PlatformSerial against both system-serial-number and baseboard-serial-number fieldValidation = ( ( - requiredPlatformCredentialFieldIsNonEmptyAndMatches( + optionalPlatformCredentialFieldNullOrMatches( "PlatformSerial", platformCredential.getPlatformSerial(), hardwareInfo.getSystemSerialNumber()) ) || ( - requiredPlatformCredentialFieldIsNonEmptyAndMatches( + optionalPlatformCredentialFieldNullOrMatches( "PlatformSerial", platformCredential.getPlatformSerial(), hardwareInfo.getBaseboardSerialNumber()) @@ -493,8 +495,8 @@ public final class SupplyChainCredentialValidator implements CredentialValidator // on the leftovers in the lists and the policy in place. final List pcComponents = new ArrayList<>(); for (ComponentIdentifier component : untrimmedPcComponents) { - DERUTF8String componentSerial = null; - DERUTF8String componentRevision = null; + DERUTF8String componentSerial = new DERUTF8String(""); + DERUTF8String componentRevision = new DERUTF8String(""); if (component.getComponentSerial() != null) { componentSerial = new DERUTF8String( component.getComponentSerial().getString().trim()); @@ -605,15 +607,15 @@ public final class SupplyChainCredentialValidator implements CredentialValidator // The remaining components from the manufacturer have only the 2 required fields so // just match them. - Iterator pcComponentIter = pcComponentsFromManufacturer.iterator(); - while (pcComponentIter.hasNext()) { - ComponentIdentifier pcComponent = pcComponentIter.next(); + List templist = new ArrayList<>(pcComponentsFromManufacturer); + for (ComponentIdentifier ci : templist) { + ComponentIdentifier pcComponent = ci; Iterator diComponentIter = deviceInfoComponentsFromManufacturer.iterator(); while (diComponentIter.hasNext()) { ComponentInfo potentialMatch = diComponentIter.next(); if (isMatch(pcComponent, potentialMatch)) { - pcComponentIter.remove(); + pcComponentsFromManufacturer.remove(ci); diComponentIter.remove(); } } @@ -636,13 +638,12 @@ public final class SupplyChainCredentialValidator implements CredentialValidator return true; } - -/** - * Returns true if fieldValue is null or empty. - * @param description description of the value - * @param fieldValue value of the field - * @return true if fieldValue is null or empty; false otherwise - */ + /** + * Returns true if fieldValue is null or empty. + * @param description description of the value + * @param fieldValue value of the field + * @return true if fieldValue is null or empty; false otherwise + */ private static boolean hasEmptyValueForRequiredField(final String description, final String fieldValue) { if (StringUtils.isEmpty(fieldValue)) { @@ -669,6 +670,15 @@ public final class SupplyChainCredentialValidator implements CredentialValidator return false; } + /** + * Validates the information supplied for the Platform Credential. This + * method checks if the field is required and therefore if the value is + * present then verifies that the values match. + * @param platformCredentialFieldName name of field to be compared + * @param platformCredentialFieldValue first value to compare + * @param otherValue second value to compare + * @return true if values match + */ private static boolean requiredPlatformCredentialFieldIsNonEmptyAndMatches( final String platformCredentialFieldName, final String platformCredentialFieldValue, @@ -678,6 +688,35 @@ public final class SupplyChainCredentialValidator implements CredentialValidator return false; } + return platformCredentialFieldMatches(platformCredentialFieldName, + platformCredentialFieldValue, otherValue); + } + + /** + * Validates the information supplied for the Platform Credential. This + * method checks if the value is present then verifies that the values match. + * If not present, then returns true. + * @param platformCredentialFieldName name of field to be compared + * @param platformCredentialFieldValue first value to compare + * @param otherValue second value to compare + * @return true if values match or null + */ + private static boolean optionalPlatformCredentialFieldNullOrMatches( + final String platformCredentialFieldName, + final String platformCredentialFieldValue, + final String otherValue) { + if (platformCredentialFieldValue == null) { + return true; + } + + return platformCredentialFieldMatches(platformCredentialFieldName, + platformCredentialFieldValue, otherValue); + } + + private static boolean platformCredentialFieldMatches( + final String platformCredentialFieldName, + final String platformCredentialFieldValue, + final String otherValue) { String trimmedFieldValue = platformCredentialFieldValue.trim(); String trimmedOtherValue = otherValue.trim(); @@ -692,6 +731,7 @@ public final class SupplyChainCredentialValidator implements CredentialValidator + "a related field in the DeviceInfoReport (%s)", platformCredentialFieldName, trimmedFieldValue) ); + return true; } @@ -812,6 +852,7 @@ public final class SupplyChainCredentialValidator implements CredentialValidator * as valid. * @return the result of the validation. */ + @Override public AppraisalStatus validateEndorsementCredential(final EndorsementCredential ec, final KeyStore trustStore, final boolean acceptExpired) { diff --git a/HIRS_Utils/src/test/java/hirs/validation/SupplyChainCredentialValidatorTest.java b/HIRS_Utils/src/test/java/hirs/validation/SupplyChainCredentialValidatorTest.java index d6006a4d..61af4f5c 100644 --- a/HIRS_Utils/src/test/java/hirs/validation/SupplyChainCredentialValidatorTest.java +++ b/HIRS_Utils/src/test/java/hirs/validation/SupplyChainCredentialValidatorTest.java @@ -93,6 +93,10 @@ import static org.powermock.api.mockito.PowerMockito.when; public class SupplyChainCredentialValidatorTest { private static final String SAMPLE_PACCOR_OUTPUT_TXT = "sample_paccor_output.txt"; + private static final String SAMPLE_PACCOR_OUTPUT_NOT_SPECIFIED_TXT + = "sample_paccor_output_not_specified_values.txt"; + private static final String SAMPLE_TEST_PACCOR_CERT + = "/validation/platform_credentials_2/paccor_platform_cert.crt"; private static final String SAMPLE_PACCOR_OUTPUT_WITH_EXTRA_COMPONENT_TXT = "sample_paccor_output_with_extra_component.txt"; @@ -1382,6 +1386,11 @@ public class SupplyChainCredentialValidatorTest { return setupDeviceInfoReportWithComponents(SAMPLE_PACCOR_OUTPUT_TXT); } + private static DeviceInfoReport setupDeviceInfoReportWithNotSpecifiedComponents() + throws IOException { + return setupDeviceInfoReportWithComponents(SAMPLE_PACCOR_OUTPUT_NOT_SPECIFIED_TXT); + } + private static DeviceInfoReport setupDeviceInfoReportWithComponents( final String paccorOutputResource) throws IOException { DeviceInfoReport deviceInfoReport = setupDeviceInfoReport(); @@ -1464,11 +1473,19 @@ public class SupplyChainCredentialValidatorTest { deviceInfoReport.getPaccorOutputString()); List componentIdentifierList = new ArrayList<>(); for (ComponentInfo deviceInfoComponent : deviceInfoComponents) { + DERUTF8String serial = null; + DERUTF8String revision = null; + if (deviceInfoComponent.getComponentSerial() != null) { + serial = new DERUTF8String(deviceInfoComponent.getComponentSerial()); + } + if (deviceInfoComponent.getComponentRevision() != null) { + revision = new DERUTF8String(deviceInfoComponent.getComponentRevision()); + } componentIdentifierList.add(new ComponentIdentifier( new DERUTF8String(deviceInfoComponent.getComponentManufacturer()), new DERUTF8String(deviceInfoComponent.getComponentModel()), - new DERUTF8String(deviceInfoComponent.getComponentSerial()), - new DERUTF8String(deviceInfoComponent.getComponentRevision()), + serial, + revision, null, ASN1Boolean.TRUE, Collections.emptyList() @@ -1487,7 +1504,7 @@ public class SupplyChainCredentialValidatorTest { * @throws IOException if unable to set up DeviceInfoReport from resource file */ @Test - public final void testvalidatePlatformCredentialAttributesV2p0NoComponentsPass() + public final void testValidatePlatformCredentialAttributesV2p0NoComponentsPass() throws IOException { DeviceInfoReport deviceInfoReport = setupDeviceInfoReport(); PlatformCredential platformCredential = setupMatchingPlatformCredential(deviceInfoReport); @@ -1505,7 +1522,7 @@ public class SupplyChainCredentialValidatorTest { * @throws IOException if unable to set up DeviceInfoReport from resource file */ @Test - public final void testvalidatePlatformCredentialAttributesV2p0WithComponentsPass() + public final void testValidatePlatformCredentialAttributesV2p0WithComponentsPass() throws IOException { DeviceInfoReport deviceInfoReport = setupDeviceInfoReportWithComponents(); PlatformCredential platformCredential = setupMatchingPlatformCredential(deviceInfoReport); @@ -1538,6 +1555,26 @@ public class SupplyChainCredentialValidatorTest { SupplyChainCredentialValidator.PLATFORM_ATTRIBUTES_VALID); } + /** + * Tests that TPM 2.0 Platform Credentials validate correctly against the device info report + * when there are components present, and when the PlatformSerial field holds the system's + * serial number instead of the baseboard serial number. + * @throws IOException if unable to set up DeviceInfoReport from resource file + * @throws URISyntaxException failed to read certificate + */ + @Test + public final void testValPCAttributesV2p0WithComponentsPassPlatformSerialWithSystemSerial2() + throws IOException, URISyntaxException { + DeviceInfoReport deviceInfoReport = setupDeviceInfoReportWithNotSpecifiedComponents(); + PlatformCredential platformCredential = new PlatformCredential( + Files.readAllBytes(Paths.get(CertificateTest.class. + getResource((SAMPLE_TEST_PACCOR_CERT)).toURI()))); + + AppraisalStatus appraisalStatus = SupplyChainCredentialValidator + .validatePlatformCredentialAttributesV2p0(platformCredential, deviceInfoReport); + Assert.assertEquals(appraisalStatus.getAppStatus(), AppraisalStatus.Status.FAIL); + } + /** * Tests that the SupplyChainCredentialValidator fails when required fields are null. * @throws IOException if unable to set up DeviceInfoReport from resource file @@ -1586,9 +1623,7 @@ public class SupplyChainCredentialValidatorTest { result = SupplyChainCredentialValidator .validatePlatformCredentialAttributesV2p0(platformCredential, deviceInfoReport); - Assert.assertEquals(result.getAppStatus(), AppraisalStatus.Status.FAIL); - Assert.assertEquals(result.getMessage(), "Platform serial did not match\n"); - + Assert.assertEquals(result.getAppStatus(), AppraisalStatus.Status.PASS); platformCredential = setupMatchingPlatformCredential(deviceInfoReport); result = SupplyChainCredentialValidator .validatePlatformCredentialAttributesV2p0(platformCredential, diff --git a/HIRS_Utils/src/test/resources/hirs/validation/sample_paccor_output_not_specified_values.txt b/HIRS_Utils/src/test/resources/hirs/validation/sample_paccor_output_not_specified_values.txt new file mode 100755 index 00000000..54cf2ec4 --- /dev/null +++ b/HIRS_Utils/src/test/resources/hirs/validation/sample_paccor_output_not_specified_values.txt @@ -0,0 +1,34 @@ +{ + + "PLATFORM": { + "PLATFORMMANUFACTURERSTR": "Not Specified","PLATFORMMODEL": "Not Specified","PLATFORMVERSION": "Not Specified" + }, + "COMPONENTS": [ + { + "MANUFACTURER": "Not Specified","MODEL": "Not Specified" + }, + { + "MANUFACTURER": "Not Specified","MODEL": "Not Specified","FIELDREPLACEABLE": "false" + }, + { + "MANUFACTURER": "Not Specified","MODEL": "UEFI" + }, + { + "MANUFACTURER": "Broadcom Inc. and subsidiaries","MODEL": "NetXtreme BCM5722 Gigabit Ethernet PCI Express","FIELDREPLACEABLE": "true","REVISION": "00" + }, + { + "MANUFACTURER": "Intel Corporation","MANUFACTURERID": "1.3.6.1.4.1.343","MODEL": "Ethernet Connection (2) I219-LM","FIELDREPLACEABLE": "true","REVISION": "31" + } + ], + "PROPERTIES": [ + { + "NAME": "uname -r", + "VALUE": "3.10.0-957.1.3.el7.x86_64" + }, + { + "NAME": "OS Release", + "VALUE": "CentOS Linux 7 (Core)" + } + ] +} + diff --git a/HIRS_Utils/src/test/resources/validation/platform_credentials_2/paccor_platform_cert.crt b/HIRS_Utils/src/test/resources/validation/platform_credentials_2/paccor_platform_cert.crt new file mode 100644 index 0000000000000000000000000000000000000000..b6ad38c4dd9df89239f6e0f6365850f1dc01fe90 GIT binary patch literal 1102 zcmXqLV(~I)Vm4x8WHbn05NhDP#M8jtklTQhjX9KsO_(V(7{=k?VG0g06foce$*}XV zyGA(r2Kcxd@)&S|M7VgE^NZ49ikW$sk`tNO7*jNa%exj>8CWbaGcbWkLW~tL5Q6CA zPOV7HEyzjLOU}1vrPK7MB3IoLX%jZQpqr8M#>*K+aT>;5RZbv@kFP zLNJH|2^b*@7&I{k8*sC+rZ=)O2^%sQFc`={ID&@427(5hKx3E~nVA^HSXdYs7#M-d z`PkTjidh-i84a6&b~OH5(D>6pj)&JTzeFLpAT>ENEi*L*jccHYCV<9mW@IoBMHAy; z33YYzGzjOBb1KSDOaXdX!80#ePa!ccMWMJfsW>wwGqET$wU|fGFSR71q$o8vRl&*G z*VNp|NWncbJuxY>M8UNrBef_mwL~Gn*;B!_qM#_XxVVwYz@VA&zkv&npl4o5YL0?) zeo;YwQDRAEejblJT!(XhUS4W4NKiq;NK?Vn$k0;P$G4Hm*szIRn~jl$u_+vA%0dPM z5lDE6gW?PvTEL(e0EQE%K?*Q_WR+QB4Wb)F7VIfJaxrFWpp@r|C1LAtteL-kanM4t z;%sc-IA%eM6C^1{ z+)}J)QfMiY8JKJsdW;?Rs!s_$Q#wJvA;sTyO}zJt$>&Qv)|G^0EKRd$?Ra}UJ!$UB zMNdL9L*nL?9aL4izyGU5Irq#J5@l9FN!F25=O`q-P+HWhanJj==7%5iRKDK0DZYJ& z{_~fC#}*5!9X!%fB`zv_^DDy(nMV(&O+34cVRn7s7mY`^gd3-yU}_CLy6?rDm<6lP zRtCNAtz5+RNH+G<(TvAFCMu_oT>IqBZ+!pZ+k1jd5lg?%z419tc30w+TFy=PK29*& zdA0c^e~L))`ps*3c%Pms{LVaSmuuenfNg^MucTw8^)lyptT(?qm;FxQHO;X7HI+Nn cTPLg+n