From 3fbb79b15ebd00f79e2a3334a79f612348c436a2 Mon Sep 17 00:00:00 2001 From: Dolph Flynn <96876199+DolphFlynn@users.noreply.github.com> Date: Mon, 12 Feb 2024 19:14:58 +0000 Subject: [PATCH 1/3] Remove optionals for dependencies that will always be fulfilled. --- src/main/java/burp/JWTEditorExtension.java | 3 +- .../burp/intruder/JWSPayloadProcessor.java | 47 +++++++++---------- .../jwteditor/model/keys/KeysModel.java | 4 +- .../burp/api/montoya/logging/StubLogging.java | 41 ++++++++++++++++ ...Test.java => JWSPayloadProcessorTest.java} | 36 ++++++-------- 5 files changed, 80 insertions(+), 51 deletions(-) create mode 100644 src/test/java/burp/api/montoya/logging/StubLogging.java rename src/test/java/burp/intruder/{JWTPayloadProcessorTest.java => JWSPayloadProcessorTest.java} (84%) diff --git a/src/main/java/burp/JWTEditorExtension.java b/src/main/java/burp/JWTEditorExtension.java index 8f54e75..23f6d5b 100644 --- a/src/main/java/burp/JWTEditorExtension.java +++ b/src/main/java/burp/JWTEditorExtension.java @@ -25,7 +25,6 @@ import com.blackberry.jwteditor.view.rsta.RstaFactory; import java.awt.*; -import java.util.Optional; import static burp.api.montoya.core.BurpSuiteEdition.COMMUNITY_EDITION; import static burp.api.montoya.core.BurpSuiteEdition.PROFESSIONAL; @@ -106,7 +105,7 @@ public void initialize(MontoyaApi api) { ); Intruder intruder = api.intruder(); - intruder.registerPayloadProcessor(new JWSPayloadProcessor(burpConfig.intruderConfig(), Optional.of(api.logging()), Optional.of(keysModel))); + intruder.registerPayloadProcessor(new JWSPayloadProcessor(burpConfig.intruderConfig(), api.logging(), keysModel)); if (api.burpSuite().version().edition() != COMMUNITY_EDITION) { api.scanner().registerInsertionPointProvider(new JWSHeaderInsertionPointProvider(burpConfig.scannerConfig())); diff --git a/src/main/java/burp/intruder/JWSPayloadProcessor.java b/src/main/java/burp/intruder/JWSPayloadProcessor.java index c49c419..2d46aac 100644 --- a/src/main/java/burp/intruder/JWSPayloadProcessor.java +++ b/src/main/java/burp/intruder/JWSPayloadProcessor.java @@ -5,7 +5,6 @@ import burp.api.montoya.intruder.PayloadProcessingResult; import burp.api.montoya.intruder.PayloadProcessor; import burp.api.montoya.logging.Logging; - import com.blackberry.jwteditor.exceptions.SigningException; import com.blackberry.jwteditor.model.jose.JOSEObject; import com.blackberry.jwteditor.model.jose.JWS; @@ -22,11 +21,11 @@ import static com.blackberry.jwteditor.utils.Constants.INTRUDER_NO_SIGNING_KEY_ID_LABEL; public class JWSPayloadProcessor implements PayloadProcessor { - Optional logging; + private final Logging logging; private final IntruderConfig intruderConfig; - private final Optional keysModel; + private final KeysModel keysModel; - public JWSPayloadProcessor(IntruderConfig intruderConfig, Optional logging, Optional keysModel) { + public JWSPayloadProcessor(IntruderConfig intruderConfig, Logging logging, KeysModel keysModel) { this.logging = logging; this.intruderConfig = intruderConfig; this.keysModel = keysModel; @@ -53,7 +52,7 @@ public PayloadProcessingResult processPayload(PayloadData payloadData) { ? Base64URL.encode(targetJson.toString()) : jws.getEncodedPayload(); - JWS updatedJws = this.createJWS(updatedHeader, updatedPayload, jws.getEncodedSignature()); + JWS updatedJws = createJWS(updatedHeader, updatedPayload, jws.getEncodedSignature()); baseValue = ByteArray.byteArray(updatedJws.serialize()); } } @@ -61,28 +60,26 @@ public PayloadProcessingResult processPayload(PayloadData payloadData) { return PayloadProcessingResult.usePayload(baseValue); } - @Override - public String displayName() { - return "JWS payload processor"; - } - private Optional loadKey() { - if (keysModel.isPresent()) { - String keyId = intruderConfig.signingKeyId(); - // only try to load key if the input value is non-empty - if (keyId.trim().length() > 0 && keyId != INTRUDER_NO_SIGNING_KEY_ID_LABEL) { - Key key = keysModel.get().getKey(intruderConfig.signingKeyId()); - if (key != null) { - return Optional.of(key); - } else { - this.logging.ifPresent(log -> log.logToError("Key with ID " + intruderConfig.signingKeyId() + " not found")); - } - } - } else { - this.logging.ifPresent(log -> log.logToOutput("No keysModel available. Will not be able to sign JWS")); + String keyId = intruderConfig.signingKeyId(); + + // only try to load key if the input value is non-empty + if (keyId == INTRUDER_NO_SIGNING_KEY_ID_LABEL || keyId == null || keyId.trim().isEmpty()) { + return Optional.empty(); } - return Optional.empty(); + Key key = keysModel.getKey(intruderConfig.signingKeyId()); + + if (key == null) { + logging.logToError("Key with ID " + intruderConfig.signingKeyId() + " not found."); + } + + return Optional.ofNullable(key); + } + + @Override + public String displayName() { + return "JWS payload processor"; } // Creates a JWS object from the given attributes. Signs the JWS if possible (i.e., available key selected in Intruder settings) @@ -93,7 +90,7 @@ private JWS createJWS(Base64URL header, Base64URL payload, Base64URL originalSig try { result = Optional.of(JWSFactory.sign(key, key.getSigningAlgorithms()[0], header, payload)); } catch (SigningException ex) { - this.logging.ifPresent(log -> log.logToError("Failed to sign JWS: " + ex)); + logging.logToError("Failed to sign JWS: " + ex); } return result; diff --git a/src/main/java/com/blackberry/jwteditor/model/keys/KeysModel.java b/src/main/java/com/blackberry/jwteditor/model/keys/KeysModel.java index d8eb6b3..e71531b 100644 --- a/src/main/java/com/blackberry/jwteditor/model/keys/KeysModel.java +++ b/src/main/java/com/blackberry/jwteditor/model/keys/KeysModel.java @@ -38,11 +38,11 @@ public class KeysModel { private final Map keys; private final Object lock; - private List modelListeners; + private final List modelListeners; public KeysModel() { this.keys = new LinkedHashMap<>(); - this.modelListeners = new ArrayList(); + this.modelListeners = new ArrayList<>(); this.lock = new Object(); } diff --git a/src/test/java/burp/api/montoya/logging/StubLogging.java b/src/test/java/burp/api/montoya/logging/StubLogging.java new file mode 100644 index 0000000..e9af542 --- /dev/null +++ b/src/test/java/burp/api/montoya/logging/StubLogging.java @@ -0,0 +1,41 @@ +package burp.api.montoya.logging; + +import java.io.PrintStream; + +public class StubLogging implements Logging { + public static final Logging LOGGING = new StubLogging(); + + @Override + public PrintStream output() { + return null; + } + + @Override + public PrintStream error() { + return null; + } + + @Override + public void logToOutput(String message) { + } + + @Override + public void logToError(String message) { + } + + @Override + public void raiseDebugEvent(String message) { + } + + @Override + public void raiseInfoEvent(String message) { + } + + @Override + public void raiseErrorEvent(String message) { + } + + @Override + public void raiseCriticalEvent(String message) { + } +} diff --git a/src/test/java/burp/intruder/JWTPayloadProcessorTest.java b/src/test/java/burp/intruder/JWSPayloadProcessorTest.java similarity index 84% rename from src/test/java/burp/intruder/JWTPayloadProcessorTest.java rename to src/test/java/burp/intruder/JWSPayloadProcessorTest.java index a774453..e800908 100644 --- a/src/test/java/burp/intruder/JWTPayloadProcessorTest.java +++ b/src/test/java/burp/intruder/JWSPayloadProcessorTest.java @@ -8,17 +8,15 @@ import burp.api.montoya.intruder.FakePayloadProcessingResult; import burp.api.montoya.intruder.PayloadData; import burp.api.montoya.intruder.PayloadProcessingResult; -import burp.api.montoya.logging.Logging; - +import com.blackberry.jwteditor.model.keys.KeysModel; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.stubbing.Answer; -import com.blackberry.jwteditor.model.keys.KeysModel; - import static burp.api.montoya.intruder.FakePayloadData.payloadData; import static burp.api.montoya.intruder.PayloadProcessingAction.USE_PAYLOAD; +import static burp.api.montoya.logging.StubLogging.LOGGING; import static burp.intruder.FuzzLocation.HEADER; import static burp.intruder.FuzzLocation.PAYLOAD; import static org.assertj.core.api.Assertions.assertThat; @@ -26,10 +24,9 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.when; -import java.util.Optional; - @ExtendWith(MontoyaExtension.class) -class JWTPayloadProcessorTest { +class JWSPayloadProcessorTest { + @BeforeEach void configureMocks() { MontoyaObjectFactory factory = ObjectFactoryLocator.FACTORY; @@ -45,9 +42,8 @@ void configureMocks() { void givenBaseValueNotJWS_whenPayloadProcessed_thenPayloadLeftUnchanged() { String baseValue = "isogeny"; PayloadData payloadData = payloadData().withBaseValue(baseValue).build(); - Optional emptyLogging = Optional.empty(); - Optional emptyKeysModel = Optional.empty(); - JWSPayloadProcessor processor = new JWSPayloadProcessor(intruderConfig("role", PAYLOAD), emptyLogging, emptyKeysModel); + KeysModel keysModel = new KeysModel(); + JWSPayloadProcessor processor = new JWSPayloadProcessor(intruderConfig("role", PAYLOAD), LOGGING, keysModel); PayloadProcessingResult result = processor.processPayload(payloadData); @@ -59,9 +55,8 @@ void givenBaseValueNotJWS_whenPayloadProcessed_thenPayloadLeftUnchanged() { void givenBaseValueJWSAndFuzzParameterNotPresent_whenPayloadProcessed_thenPayloadLeftUnchanged() { String baseValue = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"; PayloadData payloadData = payloadData().withBaseValue(baseValue).build(); - Optional emptyLogging = Optional.empty(); - Optional emptyKeysModel = Optional.empty(); - JWSPayloadProcessor processor = new JWSPayloadProcessor(intruderConfig("role", PAYLOAD), emptyLogging, emptyKeysModel); + KeysModel keysModel = new KeysModel(); + JWSPayloadProcessor processor = new JWSPayloadProcessor(intruderConfig("role", PAYLOAD), LOGGING, keysModel); PayloadProcessingResult result = processor.processPayload(payloadData); @@ -73,9 +68,8 @@ void givenBaseValueJWSAndFuzzParameterNotPresent_whenPayloadProcessed_thenPayloa void givenBaseValueJWSAndFuzzParameterPresentInWrongContext_whenPayloadProcessed_thenPayloadLeftUnchanged() { String baseValue = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"; PayloadData payloadData = payloadData().withBaseValue(baseValue).build(); - Optional emptyLogging = Optional.empty(); - Optional emptyKeysModel = Optional.empty(); - JWSPayloadProcessor processor = new JWSPayloadProcessor(intruderConfig("alg", PAYLOAD), emptyLogging, emptyKeysModel); + KeysModel keysModel = new KeysModel(); + JWSPayloadProcessor processor = new JWSPayloadProcessor(intruderConfig("alg", PAYLOAD), LOGGING, keysModel); PayloadProcessingResult result = processor.processPayload(payloadData); @@ -87,9 +81,8 @@ void givenBaseValueJWSAndFuzzParameterPresentInWrongContext_whenPayloadProcessed void givenBaseValueJWSAndFuzzParameterPresentInHeader_whenPayloadProcessed_thenPayloadModified() { String baseValue = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"; PayloadData payloadData = payloadData().withBaseValue(baseValue).withCurrentPayload("RS256").build(); - Optional emptyLogging = Optional.empty(); - Optional emptyKeysModel = Optional.empty(); - JWSPayloadProcessor processor = new JWSPayloadProcessor(intruderConfig("alg", HEADER), emptyLogging, emptyKeysModel); + KeysModel keysModel = new KeysModel(); + JWSPayloadProcessor processor = new JWSPayloadProcessor(intruderConfig("alg", HEADER), LOGGING, keysModel); PayloadProcessingResult result = processor.processPayload(payloadData); @@ -101,9 +94,8 @@ void givenBaseValueJWSAndFuzzParameterPresentInHeader_whenPayloadProcessed_thenP void givenBaseValueJWSAndFuzzParameterPresentInPayload_whenPayloadProcessed_thenPayloadModified() { String baseValue = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"; PayloadData payloadData = payloadData().withBaseValue(baseValue).withCurrentPayload("emanon").build(); - Optional emptyLogging = Optional.empty(); - Optional emptyKeysModel = Optional.empty(); - JWSPayloadProcessor processor = new JWSPayloadProcessor(intruderConfig("name", PAYLOAD), emptyLogging, emptyKeysModel); + KeysModel keysModel = new KeysModel(); + JWSPayloadProcessor processor = new JWSPayloadProcessor(intruderConfig("name", PAYLOAD), LOGGING, keysModel); PayloadProcessingResult result = processor.processPayload(payloadData); From e83c8371914a78dbe2d5e97f166f7f36b0c0284a Mon Sep 17 00:00:00 2001 From: Dolph Flynn <96876199+DolphFlynn@users.noreply.github.com> Date: Mon, 12 Feb 2024 19:31:47 +0000 Subject: [PATCH 2/3] Only allow resign to be set when signingKeyId is non-empty. --- .../java/burp/intruder/IntruderConfig.java | 9 ++- .../config/BurpConfigPersistenceTest.java | 4 +- .../java/burp/config/IntruderConfigTest.java | 78 +++++++++++++++++++ 3 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 src/test/java/burp/config/IntruderConfigTest.java diff --git a/src/main/java/burp/intruder/IntruderConfig.java b/src/main/java/burp/intruder/IntruderConfig.java index 9f3c1e3..ad4d222 100644 --- a/src/main/java/burp/intruder/IntruderConfig.java +++ b/src/main/java/burp/intruder/IntruderConfig.java @@ -19,6 +19,8 @@ package burp.intruder; import static burp.intruder.FuzzLocation.PAYLOAD; +import static com.blackberry.jwteditor.utils.Constants.INTRUDER_NO_SIGNING_KEY_ID_LABEL; +import static org.apache.commons.lang3.StringUtils.isNotEmpty; public class IntruderConfig { private String fuzzParameter; @@ -53,6 +55,7 @@ public String signingKeyId() { public void setSigningKeyId(String signingKeyId) { this.signingKeyId = signingKeyId; + this.resign = resign && isSigningKeyIdValid(); } public boolean resign() { @@ -60,6 +63,10 @@ public boolean resign() { } public void setResign(boolean resign) { - this.resign = resign; + this.resign = resign && isSigningKeyIdValid(); + } + + private boolean isSigningKeyIdValid() { + return !INTRUDER_NO_SIGNING_KEY_ID_LABEL.equals(signingKeyId) && isNotEmpty(signingKeyId); } } diff --git a/src/test/java/burp/config/BurpConfigPersistenceTest.java b/src/test/java/burp/config/BurpConfigPersistenceTest.java index 22057bd..bd0da54 100644 --- a/src/test/java/burp/config/BurpConfigPersistenceTest.java +++ b/src/test/java/burp/config/BurpConfigPersistenceTest.java @@ -240,11 +240,11 @@ private static Stream validIntruderConfigJson() { null ), arguments( - "{\"intruder_payload_processor_fuzz_location\":\"header\",\"intruder_payload_processor_parameter_name\":\"sub\",\"intruder_payload_processor_resign\":true}", + "{\"intruder_payload_processor_fuzz_location\":\"header\",\"intruder_payload_processor_parameter_name\":\"sub\",\"intruder_payload_processor_resign\":true, \"intruder_payload_processor_signing_key_id\": \"uuid\"}", HEADER, "sub", true, - null + "uuid" ), arguments( "{\"intruder_payload_processor_fuzz_location\":\"header\",\"intruder_payload_processor_parameter_name\":\"sub\",\"intruder_payload_processor_signing_key_id\":\"131da5fb-8484-4717-b0d2-b79925978596\"}", diff --git a/src/test/java/burp/config/IntruderConfigTest.java b/src/test/java/burp/config/IntruderConfigTest.java new file mode 100644 index 0000000..012aad3 --- /dev/null +++ b/src/test/java/burp/config/IntruderConfigTest.java @@ -0,0 +1,78 @@ +/* +Author : Dolph Flynn + +Copyright 2022 Dolph Flynn + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package burp.config; + +import burp.intruder.IntruderConfig; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +class IntruderConfigTest { + @Test + void givenNullKeyID_whenResignIsSetTrue_thenResignIsFalse() { + IntruderConfig config = new IntruderConfig(); + config.setSigningKeyId(null); + + config.setResign(true); + + assertThat(config.resign()).isFalse(); + } + + @Test + void givenEmptyKeyID_whenResignIsSetTrue_thenResignIsFalse() { + IntruderConfig config = new IntruderConfig(); + config.setSigningKeyId(""); + + config.setResign(true); + + assertThat(config.resign()).isFalse(); + } + + @Test + void givenValidKeyID_whenResignIsSetTrue_thenResignIsTrue() { + IntruderConfig config = new IntruderConfig(); + config.setSigningKeyId("keyID"); + + config.setResign(true); + + assertThat(config.resign()).isTrue(); + } + + @Test + void givenResignIsSetTrue_whenNullKeyID_thenResignIsFalse() { + IntruderConfig config = new IntruderConfig(); + config.setSigningKeyId("keyId"); + config.setResign(true); + + config.setSigningKeyId(null); + + assertThat(config.resign()).isFalse(); + } + + @Test + void givenResignIsSetTrue_whenEmptyKeyID_thenResignIsFalse() { + IntruderConfig config = new IntruderConfig(); + config.setSigningKeyId("keyId"); + config.setResign(true); + + config.setSigningKeyId(""); + + assertThat(config.resign()).isFalse(); + } +} \ No newline at end of file From 3989cfe5c1e28d5b2e0c12a4170866660a5b7cd5 Mon Sep 17 00:00:00 2001 From: Dolph Flynn <96876199+DolphFlynn@users.noreply.github.com> Date: Mon, 12 Feb 2024 19:36:04 +0000 Subject: [PATCH 3/3] Update README. --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 91b5a5a..f6e19c7 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,9 @@ Additionally it facilitates several well-known attacks against JWT implementatio ## Changelog +**2.2** +- Allow resigning of JWS tokens during fuzzing (Thanks to [@BafDyce](https://github.com/BafDyce)). + **2.1.1 2024-01-22** - Use split panes to improve JWT editor with small screens or large font sizes (Thanks to [@eldstal](https://github.com/eldstal)).