diff --git a/src/main/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignature.java b/src/main/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignature.java index 491223c76..9600512b5 100644 --- a/src/main/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignature.java +++ b/src/main/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignature.java @@ -40,7 +40,6 @@ private GHWebhookSignature(String payload, Secret secret) { */ public static GHWebhookSignature webhookSignature(String payload, Secret secret) { checkNotNull(payload, "Payload can't be null"); - checkNotNull(secret, "Secret should be defined to compute sign"); return new GHWebhookSignature(payload, secret); } @@ -69,19 +68,34 @@ public String sha256() { return computeSignature(HMAC_SHA256_ALGORITHM); } /** - * Computes HMAC signature using the specified algorithm. + * Computes HMAC or plain hash signature depending on whether secret is set. * - * @param algorithm The HMAC algorithm to use (e.g., "HmacSHA1", "HmacSHA256") - * @return HMAC digest as hex string, or INVALID_SIGNATURE on error + * @param algorithm The algorithm to use (e.g., "HmacSHA1", "HmacSHA256", "SHA-1", "SHA-256") + * @return digest as hex string, or INVALID_SIGNATURE on error */ private String computeSignature(String algorithm) { try { - final SecretKeySpec keySpec = new SecretKeySpec(secret.getPlainText().getBytes(UTF_8), algorithm); - final Mac mac = Mac.getInstance(algorithm); - mac.init(keySpec); - final byte[] rawHMACBytes = mac.doFinal(payload.getBytes(UTF_8)); - - return Hex.encodeHexString(rawHMACBytes); + byte[] rawBytes; + + if (secret != null) { + // Use HMAC + final Mac mac = Mac.getInstance(algorithm); + final SecretKeySpec keySpec = new SecretKeySpec(secret.getPlainText().getBytes(UTF_8), algorithm); + mac.init(keySpec); + rawBytes = mac.doFinal(payload.getBytes(UTF_8)); + } else { + // Fall back to plain hash + final MessageDigest digest; + if (algorithm.startsWith("Hmac")) { + // map HmacSHA256 -> SHA-256, HmacSHA1 -> SHA-1, etc. + digest = MessageDigest.getInstance(algorithm.replace("Hmac", "")); + } else { + digest = MessageDigest.getInstance(algorithm); + } + rawBytes = digest.digest(payload.getBytes(UTF_8)); + } + + return Hex.encodeHexString(rawBytes); } catch (Exception e) { LOGGER.error("Error computing {} signature", algorithm, e); return INVALID_SIGNATURE; diff --git a/src/test/java/com/cloudbees/jenkins/GitHubWebHookFullTest.java b/src/test/java/com/cloudbees/jenkins/GitHubWebHookFullTest.java index add363db8..e58b5d370 100644 --- a/src/test/java/com/cloudbees/jenkins/GitHubWebHookFullTest.java +++ b/src/test/java/com/cloudbees/jenkins/GitHubWebHookFullTest.java @@ -13,6 +13,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExternalResource; +import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.kohsuke.github.GHEvent; @@ -30,8 +31,7 @@ import static org.apache.commons.lang3.ClassUtils.PACKAGE_SEPARATOR; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.notNullValue; -import static org.jenkinsci.plugins.github.test.HookSecretHelper.removeSecretIn; -import static org.jenkinsci.plugins.github.test.HookSecretHelper.storeSecretIn; +import static org.jenkinsci.plugins.github.test.HookSecretHelper.*; import static org.jenkinsci.plugins.github.webhook.RequirePostWithGHHookPayload.Processor.*; /** @@ -88,6 +88,54 @@ public void shouldParseJsonWebHookFromGH() throws Exception { .expect().log().all().statusCode(SC_OK).request().post(getPath()); } + @Test + @Issue("JENKINS-76080") + public void shouldWorkWithoutSecretAndNoSignatureHeaderWebHookFromGH() throws Exception { + removeSecretIn(config); + given().spec(spec) + .header(eventHeader(GHEvent.PUSH)) + .header(JSON_CONTENT_TYPE) + .body(classpath("payloads/push.json")) + .log().all() + .expect().log().all().statusCode(SC_OK).request().post(getPath()); + } + + @Test + @Issue("JENKINS-76080") + public void shouldNotWorkWithoutSecretAndNoSignatureHeaderWebHookFromGH() throws Exception { + removeSecretIn(config); + storeGitHubPluginConfigWithNullSecret(config); + given().spec(spec) + .header(eventHeader(GHEvent.PUSH)) + .header(JSON_CONTENT_TYPE) + .body(classpath("payloads/push.json")) + .log().all() + .expect().log().all().statusCode(SC_BAD_REQUEST).request().post(getPath()); + } + + /** + * Github Webhook Docs + * X-Hub-Signature and X-Hub-Signature-256 are only sent when a webhook is configured with a secret + * Therefor if Jenkins Github plugin is configured with without a secret and receives a webhook with + * X-Hub-Signature and X-Hub-Signature-256 headers, it should fail + */ + @Test + @Issue("JENKINS-76080") + public void shouldNotWorkWithoutSecretParseJsonWebHookFromGH() throws Exception { + String hash = "notused"; + String hash256 = "a17ea241b16bc285513afd659651a2456e7c44273abe3c3d0c08febe3ef10063"; + removeSecretIn(config); + storeGitHubPluginConfigWithNullSecret(config); + given().spec(spec) + .header(eventHeader(GHEvent.PUSH)) + .header(JSON_CONTENT_TYPE) + .header(SIGNATURE_HEADER, format("sha1=%s", hash)) + .header(SIGNATURE_HEADER_SHA256, format("%s%s", SHA256_PREFIX, hash256)) + .body(classpath("payloads/push.json")) + .log().all() + .expect().log().all().statusCode(SC_BAD_REQUEST).request().post(getPath()); + } + @Test public void shouldParseJsonWebHookFromGHWithSignHeader() throws Exception { diff --git a/src/test/java/org/jenkinsci/plugins/github/test/HookSecretHelper.java b/src/test/java/org/jenkinsci/plugins/github/test/HookSecretHelper.java index 0d6d7e3db..f05d17664 100644 --- a/src/test/java/org/jenkinsci/plugins/github/test/HookSecretHelper.java +++ b/src/test/java/org/jenkinsci/plugins/github/test/HookSecretHelper.java @@ -9,6 +9,7 @@ import org.jenkinsci.plugins.github.config.GitHubPluginConfig; import org.jenkinsci.plugins.github.config.HookSecretConfig; import org.jenkinsci.plugins.plaincredentials.impl.StringCredentialsImpl; +import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -32,6 +33,12 @@ private HookSecretHelper() { * @param secretText The secret/key. */ public static void storeSecretIn(GitHubPluginConfig config, final String secretText) { + final StringCredentialsImpl credentials = createCredentail(secretText); + + config.setHookSecretConfigs(Collections.singletonList(new HookSecretConfig(credentials.getId()))); + } + + private static @NotNull StringCredentialsImpl createCredentail(String secretText) { final StringCredentialsImpl credentials = new StringCredentialsImpl( CredentialsScope.GLOBAL, UUID.randomUUID().toString(), @@ -53,10 +60,9 @@ public void run() { } } }); - - config.setHookSecretConfigs(Collections.singletonList(new HookSecretConfig(credentials.getId()))); + return credentials; } - + /** * Stores the secret and sets it as the current hook secret. * @param secretText The secret/key. @@ -65,6 +71,13 @@ public static void storeSecret(final String secretText) { storeSecretIn(Jenkins.getInstance().getDescriptorByType(GitHubPluginConfig.class), secretText); } + /** + * Stores the secret and sets it as the current hook secret. + */ + public static void storeGitHubPluginConfigWithNullSecret(GitHubPluginConfig config) { + config.setHookSecretConfigs(Collections.singletonList(new HookSecretConfig(null))); + } + /** * Unsets the current hook secret. *