From bf00ff67f6cae30b69bf8df68ea35c9b3b0ec409 Mon Sep 17 00:00:00 2001 From: Ruud Senden <8635138+rsenden@users.noreply.github.com> Date: Tue, 14 May 2024 19:37:52 +0200 Subject: [PATCH] chore: Improve action validation failure messages --- .../action/helper/ActionLoaderHelper.java | 82 ++++++++++--------- .../_common/action/AbstractActionTest.java | 5 +- .../_common/action/AbstractActionTest.java | 5 +- 3 files changed, 48 insertions(+), 44 deletions(-) diff --git a/fcli-core/fcli-common/src/main/java/com/fortify/cli/common/action/helper/ActionLoaderHelper.java b/fcli-core/fcli-common/src/main/java/com/fortify/cli/common/action/helper/ActionLoaderHelper.java index d26e81dc6c..23bd5550a8 100644 --- a/fcli-core/fcli-common/src/main/java/com/fortify/cli/common/action/helper/ActionLoaderHelper.java +++ b/fcli-core/fcli-common/src/main/java/com/fortify/cli/common/action/helper/ActionLoaderHelper.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Supplier; import java.util.regex.Pattern; @@ -82,13 +83,14 @@ private static final Stream _streamAsJson(List sources .sorted((a,b)->a.get("name").asText().compareTo(b.get("name").asText())); } - public static final String getSignatureStatusMessage(SignatureStatus signatureStatus) { + public static final String getSignatureStatusMessage(ActionProperties properties, SignatureStatus signatureStatus) { + var name = properties.getName(); switch (signatureStatus) { - case INVALID_SIGNATURE: return "Action signature is invalid."; - case NO_PUBLIC_KEY: return "No trusted public key found to verify action signature."; - case NO_SIGNATURE: return "Action is not signed."; - case NOT_VERIFIED: return "Action signature verification skipped."; - case VALID_SIGNATURE: return "Action has a valid signature."; + case INVALID_SIGNATURE: return "Signature for action "+name+" is invalid."; + case NO_PUBLIC_KEY: return "No trusted public key found to verify "+name+" action signature."; + case NO_SIGNATURE: return "Action "+name+" is not signed."; + case NOT_VERIFIED: return "Signature verification skipped for action "+name+"."; + case VALID_SIGNATURE: return "Action "+name+" has a valid signature."; default: throw new RuntimeException("Unknown signature status: "+signatureStatus); } } @@ -173,16 +175,16 @@ private final ActionLoadResult load(ZipInputStream zis, ZipEntry ze, ActionPrope } final ActionLoadResult load(InputStream is, ActionProperties properties) { - return new ActionLoadResult(actionValidationHandler, loadSignedTextDescriptor(is, properties.isCustom()), properties); + return new ActionLoadResult(actionValidationHandler, loadSignedTextDescriptor(properties, is), properties); } - private final SignedTextDescriptor loadSignedTextDescriptor(InputStream is, boolean isCustom) { + private final SignedTextDescriptor loadSignedTextDescriptor(ActionProperties properties, InputStream is) { return signedTextReader.load(is, StandardCharsets.UTF_8, // TODO For now, we only evaluate/check signatures for custom actions, // until we've figured out how to sign internal actions during (or // potentially after) Gradle build. - isCustom - ? actionValidationHandler.getSignatureValidator() + properties.isCustom() + ? actionValidationHandler.getSignatureValidator(properties) : null); } @@ -265,7 +267,7 @@ private final String updateSchema(String actionText) { } var schemaVersion = ActionSchemaHelper.getSchemaVersion(schemaUri); if ( !ActionSchemaHelper.isSupportedSchemaVersion(schemaVersion) ) { - actionValidationHandler.onUnsupportedSchemaVersion(schemaVersion); + actionValidationHandler.onUnsupportedSchemaVersion(properties, schemaVersion); } return result; } @@ -399,55 +401,55 @@ public static final class ActionValidationHandler { .onUnsupportedSchemaVersion(ActionInvalidSchemaVersionHandler.ignore) .build(); @Singular private final List extraPublicKeys; - @Singular private final Map> onSignatureStatuses; - @Builder.Default private final Consumer onSignatureStatusDefault = ActionInvalidSignatureHandler.prompt; - @Builder.Default private final Consumer onUnsupportedSchemaVersion = ActionInvalidSchemaVersionHandler.prompt; + @Singular private final Map> onSignatureStatuses; + @Builder.Default private final BiConsumer onSignatureStatusDefault = ActionInvalidSignatureHandler.prompt; + @Builder.Default private final BiConsumer onUnsupportedSchemaVersion = ActionInvalidSchemaVersionHandler.prompt; - public final SignatureValidator getSignatureValidator() { - return new SignatureValidator(this::handleInvalidSignature, extraPublicKeys.toArray(String[]::new)); + public final SignatureValidator getSignatureValidator(ActionProperties properties) { + return new SignatureValidator(d->handleInvalidSignature(properties, d), extraPublicKeys.toArray(String[]::new)); } - public final void onUnsupportedSchemaVersion(String schemaVersion) { - this.onUnsupportedSchemaVersion.accept(schemaVersion); + public final void onUnsupportedSchemaVersion(ActionProperties properties, String schemaVersion) { + this.onUnsupportedSchemaVersion.accept(properties, schemaVersion); } - private final void handleInvalidSignature(SignedTextDescriptor signedTextDescriptor) { + private final void handleInvalidSignature(ActionProperties properties, SignedTextDescriptor signedTextDescriptor) { var consumer = onSignatureStatuses.get(signedTextDescriptor.getSignatureStatus()); if ( consumer==null ) { consumer = onSignatureStatusDefault; } - consumer.accept(signedTextDescriptor); + consumer.accept(properties, signedTextDescriptor); } @RequiredArgsConstructor - public static enum ActionInvalidSignatureHandler implements Consumer { - ignore(d->{}), - warn(d->_warn(signatureFailureMessage(d))), - fail(d->_throw(signatureFailureMessage(d))), - prompt(d->_prompt(signatureFailureMessage(d))); - private final Consumer onInvalidSignature; + public static enum ActionInvalidSignatureHandler implements BiConsumer { + ignore((p,d)->{}), + warn((p,d)->_warn(signatureFailureMessage(p,d))), + fail((p,d)->_throw(signatureFailureMessage(p,d))), + prompt((p,d)->_prompt(signatureFailureMessage(p,d))); + private final BiConsumer onInvalidSignature; @Override - public void accept(SignedTextDescriptor descriptor) { - onInvalidSignature.accept(descriptor); + public void accept(ActionProperties properties, SignedTextDescriptor descriptor) { + onInvalidSignature.accept(properties, descriptor); } - private static final String signatureFailureMessage(SignedTextDescriptor descriptor) { - return getSignatureStatusMessage(descriptor.getSignatureStatus()); + private static final String signatureFailureMessage(ActionProperties properties, SignedTextDescriptor descriptor) { + return getSignatureStatusMessage(properties, descriptor.getSignatureStatus()); } } @RequiredArgsConstructor - public static enum ActionInvalidSchemaVersionHandler implements Consumer { - ignore(v->{}), - warn(v->_warn(unsupportedSchemaMessage(v))), - fail(v->_throw(unsupportedSchemaMessage(v))), - prompt(v->_prompt(unsupportedSchemaMessage(v))); - private final Consumer onInvalidSchemaVersion; + public static enum ActionInvalidSchemaVersionHandler implements BiConsumer { + ignore((p,v)->{}), + warn((p,v)->_warn(unsupportedSchemaMessage(p,v))), + fail((p,v)->_throw(unsupportedSchemaMessage(p,v))), + prompt((p,v)->_prompt(unsupportedSchemaMessage(p,v))); + private final BiConsumer onInvalidSchemaVersion; @Override - public void accept(String schemaVersion) { - onInvalidSchemaVersion.accept(schemaVersion); + public void accept(ActionProperties properties, String schemaVersion) { + onInvalidSchemaVersion.accept(properties, schemaVersion); } - public static final String unsupportedSchemaMessage(String unsupportedVersion) { - return String.format("Action uses unsupported schema version %s and may fail.", unsupportedVersion); + public static final String unsupportedSchemaMessage(ActionProperties properties, String unsupportedVersion) { + return String.format("Action "+properties.getName()+" uses unsupported schema version %s and may fail.", unsupportedVersion); } } diff --git a/fcli-core/fcli-fod/src/test/java/com/fortify/cli/fod/_common/action/AbstractActionTest.java b/fcli-core/fcli-fod/src/test/java/com/fortify/cli/fod/_common/action/AbstractActionTest.java index d70cac5ede..054665c342 100644 --- a/fcli-core/fcli-fod/src/test/java/com/fortify/cli/fod/_common/action/AbstractActionTest.java +++ b/fcli-core/fcli-fod/src/test/java/com/fortify/cli/fod/_common/action/AbstractActionTest.java @@ -12,7 +12,7 @@ */ package com.fortify.cli.fod._common.action; -import java.util.function.Consumer; +import java.util.function.BiConsumer; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.TestInstance; @@ -25,6 +25,7 @@ import com.fortify.cli.common.action.helper.ActionLoaderHelper.ActionValidationHandler; import com.fortify.cli.common.action.helper.ActionLoaderHelper.ActionValidationHandler.ActionInvalidSchemaVersionHandler; import com.fortify.cli.common.action.helper.ActionLoaderHelper.ActionValidationHandler.ActionInvalidSignatureHandler; +import com.fortify.cli.common.action.model.Action.ActionProperties; import com.fortify.cli.common.crypto.helper.SignatureHelper.SignedTextDescriptor; // TODO Move this class to a common test utility module; currently @@ -55,7 +56,7 @@ public final String[] getActions() { .toArray(String[]::new); } - private static final Consumer invalidSignatureHandler() { + private static final BiConsumer invalidSignatureHandler() { return "true".equalsIgnoreCase((System.getProperty("test.action.requireValidSignature"))) ? ActionInvalidSignatureHandler.fail : ActionInvalidSignatureHandler.warn; diff --git a/fcli-core/fcli-ssc/src/test/java/com/fortify/cli/ssc/_common/action/AbstractActionTest.java b/fcli-core/fcli-ssc/src/test/java/com/fortify/cli/ssc/_common/action/AbstractActionTest.java index 3533254069..3ee9cd0726 100644 --- a/fcli-core/fcli-ssc/src/test/java/com/fortify/cli/ssc/_common/action/AbstractActionTest.java +++ b/fcli-core/fcli-ssc/src/test/java/com/fortify/cli/ssc/_common/action/AbstractActionTest.java @@ -12,7 +12,7 @@ */ package com.fortify.cli.ssc._common.action; -import java.util.function.Consumer; +import java.util.function.BiConsumer; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.TestInstance; @@ -25,6 +25,7 @@ import com.fortify.cli.common.action.helper.ActionLoaderHelper.ActionValidationHandler; import com.fortify.cli.common.action.helper.ActionLoaderHelper.ActionValidationHandler.ActionInvalidSchemaVersionHandler; import com.fortify.cli.common.action.helper.ActionLoaderHelper.ActionValidationHandler.ActionInvalidSignatureHandler; +import com.fortify.cli.common.action.model.Action.ActionProperties; import com.fortify.cli.common.crypto.helper.SignatureHelper.SignedTextDescriptor; // TODO Move this class to a common test utility module; currently @@ -55,7 +56,7 @@ public final String[] getActions() { .toArray(String[]::new); } - private static final Consumer invalidSignatureHandler() { + private static final BiConsumer invalidSignatureHandler() { return "true".equalsIgnoreCase((System.getProperty("test.action.requireValidSignature"))) ? ActionInvalidSignatureHandler.fail : ActionInvalidSignatureHandler.warn;