Skip to content

Commit

Permalink
On a previous commit, I removed a piece of code that checked the base…
Browse files Browse the repository at this point in the history 
… credential first. Because the delta fixed a problem in the base, the base failed before the delta was checked. This was completely removed. On a test that we had previously done, the test passes when it should fail because there is only a base, so that check isn't being done. This change reintroduces the check but in a different location with flags for when there is a delta present.
  • Loading branch information
@cyrus-dev
cyrus-dev committed Feb 9, 2021
1 parent 69cd06d commit 9917fad
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 43 deletions.
63 changes: 45 additions & 18 deletions ...estationCA/src/main/java/hirs/attestationca/service/SupplyChainValidationServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ public SupplyChainValidationSummary validateSupplyChain(final EndorsementCredent
boolean acceptExpiredCerts = policy.isExpiredCertificateValidationEnabled();
PlatformCredential baseCredential = null;
SupplyChainValidation platformScv = null;
boolean chkDeltas = false;
String pcErrorMessage = "";
List<SupplyChainValidation> validations = new LinkedList<>();
Map<PlatformCredential, SupplyChainValidation> deltaMapping = new HashMap<>();
Expand Down Expand Up @@ -174,6 +175,7 @@ public SupplyChainValidationSummary validateSupplyChain(final EndorsementCredent
if (pc.isBase()) {
baseCredential = pc;
} else {
chkDeltas = true;
deltaMapping.put(pc, null);
}
pc.setDevice(device);
Expand Down Expand Up @@ -214,33 +216,54 @@ public SupplyChainValidationSummary validateSupplyChain(final EndorsementCredent
&& pcErrorMessage.isEmpty()) {
// Ensure there are platform credentials to validate
SupplyChainValidation attributeScv = null;

List<ArchivableEntity> aes = new ArrayList<>();
if (platformScv != null) {
aes.addAll(platformScv.getCertificatesUsed());
}
Iterator<PlatformCredential> it = pcs.iterator();
String attrErrorMessage = "";
while (it.hasNext()) {
PlatformCredential pc = it.next();
if (pc != null) {
if (!pc.isBase()) {
attributeScv = validateDeltaPlatformCredentialAttributes(
pc, device.getDeviceInfo(),
baseCredential, deltaMapping);
if (attributeScv.getResult() == FAIL) {
attrErrorMessage = String.format("%s%s%n", attrErrorMessage,
attributeScv.getMessage());
List<ArchivableEntity> aes = new ArrayList<>();
// need to check if there are deltas, if not then just verify
// components of the base
if (baseCredential == null) {
validations.add(buildValidationRecord(
SupplyChainValidation.ValidationType.PLATFORM_CREDENTIAL,
AppraisalStatus.Status.FAIL,
"Base Platform credential missing."
+ " Cannot validate attributes",
null, Level.ERROR));
} else {
if (chkDeltas) {
if (platformScv != null) {
aes.addAll(platformScv.getCertificatesUsed());
}
Iterator<PlatformCredential> it = pcs.iterator();
while (it.hasNext()) {
PlatformCredential pc = it.next();
if (pc != null) {
if (!pc.isBase()) {
attributeScv = validateDeltaPlatformCredentialAttributes(
pc, device.getDeviceInfo(),
baseCredential, deltaMapping);
if (attributeScv.getResult() == FAIL) {
attrErrorMessage = String.format("%s%s%n", attrErrorMessage,
attributeScv.getMessage());
}
}
}
}
} else {
aes.add(baseCredential);
validations.remove(platformScv);
// if there are no deltas, just check base credential
platformScv = validatePlatformCredentialAttributes(
baseCredential, device.getDeviceInfo(), ec);
validations.add(new SupplyChainValidation(
SupplyChainValidation.ValidationType.PLATFORM_CREDENTIAL,
platformScv.getResult(), aes, platformScv.getMessage()));
}
}
if (!attrErrorMessage.isEmpty()) {
//combine platform and platform attributes
validations.remove(platformScv);
if (platformScv != null) {
validations.add(new SupplyChainValidation(
platformScv.getValidationType(),
SupplyChainValidation.ValidationType.PLATFORM_CREDENTIAL,
attributeScv.getResult(), aes, attributeScv.getMessage()));
}
}
Expand All @@ -252,7 +275,7 @@ public SupplyChainValidationSummary validateSupplyChain(final EndorsementCredent
validations.add(validateFirmware(device, policy.getPcrPolicy()));
}

LOGGER.error("The service finished and now summarizing");
LOGGER.info("The service finished and now summarizing");
// Generate validation summary, save it, and return it.
SupplyChainValidationSummary summary
= new SupplyChainValidationSummary(device, validations);
Expand Down Expand Up @@ -662,6 +685,10 @@ private SupplyChainValidation validatePlatformCredentialAttributes(
return buildValidationRecord(validationType, PASS,
result.getMessage(), pc, Level.INFO);
case FAIL:
if (!result.getAdditionalInfo().isEmpty()) {
pc.setComponentFailures(result.getAdditionalInfo());
this.certificateManager.update(pc);
}
return buildValidationRecord(validationType, AppraisalStatus.Status.FAIL,
result.getMessage(), pc, Level.WARN);
case ERROR:
Expand Down
19 changes: 19 additions & 0 deletions HIRS_Utils/src/main/java/hirs/data/persist/certificate/attributes/ComponentIdentifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public class ComponentIdentifier {
private ASN1ObjectIdentifier componentManufacturerId;
private ASN1Boolean fieldReplaceable;
private List<ComponentAddress> componentAddress;
private boolean validationResult = true;

/**
* Default constructor.
Expand Down Expand Up @@ -264,6 +265,24 @@ public boolean isVersion2() {
return false;
}

/**
* Holds the status of the validation process for attributes
* specific to this instance.
* @return true is passed, false if failed.
*/
public boolean isValidationResult() {
return validationResult;
}

/**
* Sets the flag for the validation status for this instance
* of the attribute.
* @param validationResult validation flag.
*/
public void setValidationResult(final boolean validationResult) {
this.validationResult = validationResult;
}

/**
* Get all the component addresses inside the sequence.
*
Expand Down
46 changes: 21 additions & 25 deletions HIRS_Utils/src/main/java/hirs/validation/SupplyChainCredentialValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,6 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
*/
public static final String FIRMWARE_VALID = "Firmware validated";

private static final Map<PlatformCredential, StringBuilder> DELTA_FAILURES = new HashMap<>();

/*
* Ensure that BouncyCastle is configured as a javax.security.Security provider, as this
* class expects it to be available.
Expand Down Expand Up @@ -318,8 +316,6 @@ public AppraisalStatus validateDeltaPlatformCredentialAttributes(
final Map<PlatformCredential, SupplyChainValidation> deltaMapping) {
String message;

LOGGER.error("Starting the method validateDeltaPlatformCredentialAttributes");

// this needs to be a loop for all deltas, link to issue #110
// check that they don't have the same serial number
for (PlatformCredential delta : deltaMapping.keySet()) {
Expand All @@ -342,7 +338,6 @@ public AppraisalStatus validateDeltaPlatformCredentialAttributes(
}
}

LOGGER.error("This is before validateDeltaAttributesChainV2p0");
// parse out the provided delta and its specific chain.
List<ComponentIdentifier> origPcComponents
= new LinkedList<>(basePlatformCredential.getComponentIdentifiers());
Expand Down Expand Up @@ -560,17 +555,23 @@ static AppraisalStatus validatePlatformCredentialAttributesV2p0(
return new AppraisalStatus(ERROR, baseErrorMessage + e.getMessage());
}

StringBuilder additionalInfo = new StringBuilder();
if (!fieldValidation) {
resultMessage.append("There are unmatched components:\n");
resultMessage.append(unmatchedComponents);

// pass information of which ones failed in additionInfo
for (ComponentIdentifier ci : validPcComponents) {
additionalInfo.append(String.format("%d;", ci.hashCode()));
}
}

passesValidation &= fieldValidation;

if (passesValidation) {
return new AppraisalStatus(PASS, PLATFORM_ATTRIBUTES_VALID);
} else {
return new AppraisalStatus(FAIL, resultMessage.toString());
return new AppraisalStatus(FAIL, resultMessage.toString(), additionalInfo.toString());
}
}

Expand Down Expand Up @@ -635,6 +636,7 @@ public int compare(final PlatformCredential obj1,
// finished up
List<ArchivableEntity> certificateList = null;
SupplyChainValidation scv = null;
StringBuilder deltaSb = new StringBuilder();

// non-empty serial values
for (ComponentIdentifier deltaCi : leftOverDeltas) {
Expand All @@ -653,6 +655,7 @@ public int compare(final PlatformCredential obj1,
if (ciV2.isAdded()) {
// error
resultMessage.append("ADDED attempted with prior instance\n");
deltaSb.append(String.format("%s;", ci.hashCode()));
}
if (ciV2.isModified()) {
// since the base list doesn't have this ci
Expand Down Expand Up @@ -680,19 +683,21 @@ public int compare(final PlatformCredential obj1,
// error because you can't modify something
// that isn't here
resultMessage.append("MODIFIED attempted without prior instance\n");
deltaSb.append(String.format("%s;", ci.hashCode()));
}

if (ciV2.isRemoved()) {
// error because you can't remove something
// that isn't here
resultMessage.append("REMOVED attempted without prior instance\n");
deltaSb.append(String.format("%s;", ci.hashCode()));
}
}
}
}

if (!fieldValidation) {
return new AppraisalStatus(FAIL, resultMessage.toString());
return new AppraisalStatus(FAIL, resultMessage.toString(), deltaSb.toString());
}

String paccorOutputString = deviceInfoReport.getPaccorOutputString();
Expand Down Expand Up @@ -766,7 +771,6 @@ && isMatch(cId, cInfo)) {

if (!subCompInfoList.isEmpty()) {
for (ComponentInfo ci : subCompInfoList) {
LOGGER.error("For subComInfoList -> {}", ci.getComponentSerial());
invalidDeviceInfo.append(String.format("%d;",
ci.hashCode()));
}
Expand All @@ -783,6 +787,8 @@ && isMatch(cId, cInfo)) {
* components not represented in the platform credential.
*
* @param untrimmedPcComponents the platform credential components (may contain end whitespace)
* **NEW** this is updated with just the unmatched components
* if there are any failures, otherwise it remains unchanged.
* @param allDeviceInfoComponents the device info report components
* @return true if validation passes
*/
Expand Down Expand Up @@ -813,17 +819,15 @@ private static String validateV2p0PlatformCredentialComponentsExpectingExactMatc
componentSerial, componentRevision,
component.getComponentManufacturerId(),
component.getFieldReplaceable(),
component.getComponentAddress()
));
component.getComponentAddress()));
}

LOGGER.info("Validating the following Platform Cert components...");
pcComponents.forEach(component -> LOGGER.info(component.toString()));
LOGGER.info("...against the the following DeviceInfoReport components:");
allDeviceInfoComponents.forEach(component -> LOGGER.info(component.toString()));
Set<DERUTF8String> manufacturerSet = new HashSet<>();
pcComponents.forEach(component -> manufacturerSet.add(
component.getComponentManufacturer()));
pcComponents.forEach(pcComp -> manufacturerSet.add(pcComp.getComponentManufacturer()));

// Create a list for unmatched components across all manufacturers to display at the end.
List<ComponentIdentifier> pcUnmatchedComponents = new ArrayList<>();
Expand Down Expand Up @@ -859,8 +863,7 @@ private static String validateV2p0PlatformCredentialComponentsExpectingExactMatc
.filter(componentInfo
-> StringUtils.isNotEmpty(componentInfo.getComponentSerial()))
.filter(componentInfo -> componentInfo.getComponentSerial()
.equals(pcComponent.getComponentSerial().getString()))
.findFirst();
.equals(pcComponent.getComponentSerial().getString())).findFirst();

if (first.isPresent()) {
ComponentInfo potentialMatch = first.get();
Expand Down Expand Up @@ -905,12 +908,11 @@ private static String validateV2p0PlatformCredentialComponentsExpectingExactMatc
// just match them.
List<ComponentIdentifier> templist = new ArrayList<>(pcComponentsFromManufacturer);
for (ComponentIdentifier ci : templist) {
ComponentIdentifier pcComponent = ci;
Iterator<ComponentInfo> diComponentIter
= deviceInfoComponentsFromManufacturer.iterator();
while (diComponentIter.hasNext()) {
ComponentInfo potentialMatch = diComponentIter.next();
if (isMatch(pcComponent, potentialMatch)) {
if (isMatch(ci, potentialMatch)) {
pcComponentsFromManufacturer.remove(ci);
diComponentIter.remove();
}
Expand All @@ -920,6 +922,7 @@ private static String validateV2p0PlatformCredentialComponentsExpectingExactMatc
}

if (!pcUnmatchedComponents.isEmpty()) {
untrimmedPcComponents.clear();
StringBuilder sb = new StringBuilder();
LOGGER.error(String.format("Platform Credential contained %d unmatched components:",
pcUnmatchedComponents.size()));
Expand All @@ -933,6 +936,8 @@ private static String validateV2p0PlatformCredentialComponentsExpectingExactMatc
unmatchedComponent.getComponentModel(),
unmatchedComponent.getComponentSerial(),
unmatchedComponent.getComponentRevision()));
unmatchedComponent.setValidationResult(false);
untrimmedPcComponents.add(unmatchedComponent);
}
return sb.toString();
}
Expand Down Expand Up @@ -1664,13 +1669,4 @@ private static boolean isSelfSigned(final X509Certificate cert)
return false;
}
}

/**
* Getter for the collection of delta certificates that have failed and the
* associated message.
* @return unmodifiable list of failed certificates
*/
public Map<PlatformCredential, StringBuilder> getDeltaFailures() {
return Collections.unmodifiableMap(DELTA_FAILURES);
}
}

0 comments on commit 9917fad

Please sign in to comment.