[#181] Delta holder validation (#186)

* This is a quick fix to ensure that a delta that is being uploaded has a holder serial number that exists in the database.

* Fixed syntax issues.

* Through further testing with delta certificates that had differing begin validity dates, the code to test the sorting failed.  This push includes a fix that places the deltas in the proper order.

In addition, this code includes a placeholder for deltas that don't have an existing holder certificate in the database.

* Findbugs is a cumbersome COTS product that generates more hassle than help.  Upon indicating 'dodgy' code about redundant null checks, that didn't exist, it then didn't like using non-short circuit operators to verify that both objects are not null.  It then spells out what non-shorting curcuit operators do, without acknowledges that's what you mean to do.
This commit is contained in:
Cyrus 2019-08-29 13:35:41 -04:00 committed by GitHub
parent 9318c22549
commit f73d65c952
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 44 additions and 20 deletions

View File

@ -12,7 +12,6 @@ import org.springframework.context.annotation.Import;
import org.springframework.stereotype.Service; import org.springframework.stereotype.Service;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet; import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
@ -269,15 +268,6 @@ public class SupplyChainValidationServiceImpl implements SupplyChainValidationSe
.select(certificateManager) .select(certificateManager)
.byBoardSerialNumber(pc.getPlatformSerial()) .byBoardSerialNumber(pc.getPlatformSerial())
.getCertificates().stream().collect(Collectors.toList()); .getCertificates().stream().collect(Collectors.toList());
Collections.sort(chainCertificates,
new Comparator<PlatformCredential>() {
@Override
public int compare(final PlatformCredential obj1,
final PlatformCredential obj2) {
return obj1.getBeginValidity()
.compareTo(obj2.getBeginValidity());
}
});
SupplyChainValidation deltaScv; SupplyChainValidation deltaScv;
KeyStore trustedCa; KeyStore trustedCa;

View File

@ -698,7 +698,25 @@ public class CertificateRequestPageController extends PageController<NoPageParam
} }
} }
} }
} /**else {
// this is a delta, check if the holder exists.
PlatformCredential holderPC = PlatformCredential
.select(certificateManager)
.bySerialNumber(platformCertificate.getHolderSerialNumber())
.getCertificate();
if (holderPC == null) {
final String failMessage = "Storing certificate failed: "
+ "delta credential"
+ " must have an existing holder stored. "
+ "Credential serial "
+ platformCertificate.getHolderSerialNumber()
+ " doesn't exist.";
messages.addError(failMessage);
LOGGER.error(failMessage);
return;
} }
}**/
} }
certificateManager.save(certificate); certificateManager.save(certificate);

View File

@ -59,6 +59,7 @@ import hirs.data.persist.SupplyChainValidation;
import hirs.data.persist.certificate.Certificate; import hirs.data.persist.certificate.Certificate;
import hirs.data.persist.certificate.attributes.V2.ComponentIdentifierV2; import hirs.data.persist.certificate.attributes.V2.ComponentIdentifierV2;
import java.util.Collections; import java.util.Collections;
import java.util.Comparator;
import java.util.LinkedList; import java.util.LinkedList;
import org.apache.logging.log4j.util.Strings; import org.apache.logging.log4j.util.Strings;
@ -559,6 +560,7 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
.filter(identifier -> identifier.getComponentManufacturer() != null .filter(identifier -> identifier.getComponentManufacturer() != null
&& identifier.getComponentModel() != null) && identifier.getComponentModel() != null)
.collect(Collectors.toList()); .collect(Collectors.toList());
List<PlatformCredential> chainCertificates = new LinkedList<>(deltaMapping.keySet());
// map the components throughout the chain // map the components throughout the chain
Map<String, ComponentIdentifier> chainCiMapping = new HashMap<>(); Map<String, ComponentIdentifier> chainCiMapping = new HashMap<>();
@ -567,18 +569,32 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
chainCiMapping.put(ci.getComponentSerial().toString(), ci); chainCiMapping.put(ci.getComponentSerial().toString(), ci);
}); });
Collections.sort(chainCertificates, new Comparator<PlatformCredential>() {
@Override
public int compare(final PlatformCredential obj1,
final PlatformCredential obj2) {
if (obj1 == null) {
return 0;
}
if (obj2 == null) {
return 0;
}
if (obj1.getBeginValidity() == null || obj2.getBeginValidity() == null) {
return 0;
}
return obj1.getBeginValidity().compareTo(obj2.getBeginValidity());
}
});
String ciSerial; String ciSerial;
List<Certificate> certificateList = null; List<Certificate> certificateList = null;
PlatformCredential delta = null;
SupplyChainValidation scv = null; SupplyChainValidation scv = null;
resultMessage.append("There are errors with Delta " resultMessage.append("There are errors with Delta "
+ "Component Statuses components:\n"); + "Component Statuses components:\n");
// go through the leaf and check the changes against the valid components // go through the leaf and check the changes against the valid components
// forget modifying validOrigPcComponents // forget modifying validOrigPcComponents
for (Map.Entry<PlatformCredential, SupplyChainValidation> deltaEntry for (PlatformCredential delta : chainCertificates) {
: deltaMapping.entrySet()) {
StringBuilder failureMsg = new StringBuilder(); StringBuilder failureMsg = new StringBuilder();
delta = deltaEntry.getKey();
certificateList = new ArrayList<>(); certificateList = new ArrayList<>();
certificateList.add(delta); certificateList.add(delta);
@ -594,7 +610,7 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
failureMsg.append(String.format( failureMsg.append(String.format(
"%s attempted MODIFIED with no prior instance.%n", "%s attempted MODIFIED with no prior instance.%n",
ciSerial)); ciSerial));
scv = deltaEntry.getValue(); scv = deltaMapping.get(delta);
if (scv.getResult() != AppraisalStatus.Status.PASS) { if (scv.getResult() != AppraisalStatus.Status.PASS) {
failureMsg.append(scv.getMessage()); failureMsg.append(scv.getMessage());
} }
@ -604,7 +620,7 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
certificateList, certificateList,
failureMsg.toString())); failureMsg.toString()));
} else { } else {
chainCiMapping.put(ci.getComponentSerial().toString(), ci); chainCiMapping.put(ciSerial, ci);
} }
} else if (ciV2.isRemoved()) { } else if (ciV2.isRemoved()) {
if (!chainCiMapping.containsKey(ciSerial)) { if (!chainCiMapping.containsKey(ciSerial)) {
@ -613,7 +629,7 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
failureMsg.append(String.format( failureMsg.append(String.format(
"%s attempted REMOVED with no prior instance.%n", "%s attempted REMOVED with no prior instance.%n",
ciSerial)); ciSerial));
scv = deltaEntry.getValue(); scv = deltaMapping.get(delta);
if (scv.getResult() != AppraisalStatus.Status.PASS) { if (scv.getResult() != AppraisalStatus.Status.PASS) {
failureMsg.append(scv.getMessage()); failureMsg.append(scv.getMessage());
} }
@ -623,7 +639,7 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
certificateList, certificateList,
failureMsg.toString())); failureMsg.toString()));
} else { } else {
chainCiMapping.remove(ci.getComponentSerial().toString()); chainCiMapping.remove(ciSerial);
} }
} else if (ciV2.isAdded()) { } else if (ciV2.isAdded()) {
// ADDED // ADDED
@ -633,7 +649,7 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
failureMsg.append(String.format( failureMsg.append(String.format(
"%s was ADDED, the serial already exists.%n", "%s was ADDED, the serial already exists.%n",
ciSerial)); ciSerial));
scv = deltaEntry.getValue(); scv = deltaMapping.get(delta);
if (scv.getResult() != AppraisalStatus.Status.PASS) { if (scv.getResult() != AppraisalStatus.Status.PASS) {
failureMsg.append(scv.getMessage()); failureMsg.append(scv.getMessage());
} }