Skip to content

Commit 4490700

Browse files
HybridProgrammerJason Heithoffclaude
authored
[JENKINS-73851] Add SHA-256 HMAC webhook signature validation (#438)
* feat(JENKINS-73851): Add SHA-256 HMAC webhook signature validation - Add SignatureAlgorithm enum with SHA-256 as default and SHA-1 for legacy support - Extend GHWebhookSignature class to support SHA-256 HMAC computation - Update HookSecretConfig to include configurable signature algorithm - Modify RequirePostWithGHHookPayload.Processor to use configured algorithm - Add comprehensive unit tests for SHA-256 functionality - Maintain backwards compatibility with existing SHA-1 configurations - Log deprecation warnings when SHA-1 is used This implements GitHub's recommended SHA-256 HMAC signature validation while maintaining backwards compatibility through configuration. SHA-256 becomes the default for enhanced security. Resolves: JENKINS-73851 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * feat(JENKINS-73851): Add UI for signature algorithm selection - Add doFillSignatureAlgorithmItems() method to provide dropdown options - Create signature algorithm selection dropdown in Jenkins configuration UI - Add help documentation for signature algorithm selection - Update HookSecretConfig constructor to parse algorithm from UI string input - Add parseSignatureAlgorithm() method with case-insensitive parsing - Update tests to work with new string-based constructor - Add comprehensive test cases for algorithm parsing edge cases Users can now choose between SHA-256 (Recommended) and SHA-1 (Legacy) signature algorithms through the Jenkins UI in the GitHub plugin configuration section. The dropdown properly displays both options with SHA-256 set as default for enhanced security, while SHA-1 remains available for legacy compatibility. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * fix(JENKINS-73851): Fix checkstyle violations in SHA-256 implementation - Remove trailing whitespace from all modified files - Fix line length violations by properly wrapping long lines - Fix operator wrap issues by placing operators on new lines - Maintain consistent code formatting throughout 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * chore: Remove .vscode/settings.json and add to .gitignore - Remove IDE-specific configuration file from version control - Add .vscode/ directory to .gitignore to prevent future tracking - Keep IDE configurations local to individual developer environments 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * feat(JENKINS-73851): Add system property override for default signature algorithm - Add jenkins.github.webhook.signature.default system property - Allows overriding default from SHA-256 to SHA-1 for CI compatibility - Maintains SHA-256 as secure default when no property is set - Dynamic evaluation prevents static initialization issues - Added comprehensive test coverage and documentation Usage: - Default: SHA-256 (secure) - CI override: -Djenkins.github.webhook.signature.default=SHA1 - Invalid values fallback to SHA-256 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * fix(JENKINS-73851): Fix failing tests --------- Co-authored-by: Jason Heithoff <[email protected]> Co-authored-by: Claude <[email protected]>
1 parent 0ffb13e commit 4490700

File tree

12 files changed

+535
-25
lines changed

12 files changed

+535
-25
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@ target
1111

1212
# autogenerated resources
1313
src/main/webapp/css/*
14+
.vscode/

‎src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import hudson.util.ListBoxModel;
1313
import hudson.util.Secret;
1414
import jenkins.model.Jenkins;
15+
import org.jenkinsci.plugins.github.webhook.SignatureAlgorithm;
1516
import org.jenkinsci.plugins.plaincredentials.StringCredentials;
1617
import org.kohsuke.stapler.DataBoundConstructor;
1718

@@ -25,10 +26,19 @@
2526
public class HookSecretConfig extends AbstractDescribableImpl<HookSecretConfig> {
2627

2728
private String credentialsId;
29+
private SignatureAlgorithm signatureAlgorithm;
2830

2931
@DataBoundConstructor
30-
public HookSecretConfig(String credentialsId) {
32+
public HookSecretConfig(String credentialsId, String signatureAlgorithm) {
3133
this.credentialsId = credentialsId;
34+
this.signatureAlgorithm = parseSignatureAlgorithm(signatureAlgorithm);
35+
}
36+
37+
/**
38+
* Legacy constructor for backwards compatibility.
39+
*/
40+
public HookSecretConfig(String credentialsId) {
41+
this(credentialsId, null);
3242
}
3343

3444
/**
@@ -45,6 +55,26 @@ public String getCredentialsId() {
4555
return credentialsId;
4656
}
4757

58+
/**
59+
* Gets the signature algorithm to use for webhook validation.
60+
*
61+
* @return the configured signature algorithm, defaults to SHA-256
62+
* @since 1.45.0
63+
*/
64+
public SignatureAlgorithm getSignatureAlgorithm() {
65+
return signatureAlgorithm != null ? signatureAlgorithm : SignatureAlgorithm.getDefault();
66+
}
67+
68+
/**
69+
* Gets the signature algorithm name for UI binding.
70+
*
71+
* @return the algorithm name as string (e.g., "SHA256", "SHA1")
72+
* @since 1.45.0
73+
*/
74+
public String getSignatureAlgorithmName() {
75+
return getSignatureAlgorithm().name();
76+
}
77+
4878
/**
4979
* @param credentialsId a new ID
5080
* @deprecated rather treat this field as final and use {@link GitHubPluginConfig#setHookSecretConfigs}
@@ -54,6 +84,33 @@ public void setCredentialsId(String credentialsId) {
5484
this.credentialsId = credentialsId;
5585
}
5686

87+
/**
88+
* Ensures backwards compatibility during deserialization.
89+
* Sets default algorithm to SHA-256 for existing configurations.
90+
*/
91+
private Object readResolve() {
92+
if (signatureAlgorithm == null) {
93+
signatureAlgorithm = SignatureAlgorithm.getDefault();
94+
}
95+
return this;
96+
}
97+
98+
/**
99+
* Parses signature algorithm from UI string input.
100+
*/
101+
private SignatureAlgorithm parseSignatureAlgorithm(String algorithmName) {
102+
if (algorithmName == null || algorithmName.trim().isEmpty()) {
103+
return SignatureAlgorithm.getDefault();
104+
}
105+
106+
try {
107+
return SignatureAlgorithm.valueOf(algorithmName.trim().toUpperCase());
108+
} catch (IllegalArgumentException e) {
109+
// Default to SHA-256 for invalid input
110+
return SignatureAlgorithm.getDefault();
111+
}
112+
}
113+
57114
@Extension
58115
public static class DescriptorImpl extends Descriptor<HookSecretConfig> {
59116

@@ -62,6 +119,16 @@ public String getDisplayName() {
62119
return "Hook secret configuration";
63120
}
64121

122+
/**
123+
* Provides dropdown items for signature algorithm selection.
124+
*/
125+
public ListBoxModel doFillSignatureAlgorithmItems() {
126+
ListBoxModel items = new ListBoxModel();
127+
items.add("SHA-256 (Recommended)", "SHA256");
128+
items.add("SHA-1 (Legacy)", "SHA1");
129+
return items;
130+
}
131+
65132
@SuppressWarnings("unused")
66133
public ListBoxModel doFillCredentialsIdItems(@QueryParameter String credentialsId) {
67134
if (!Jenkins.getInstance().hasPermission(Jenkins.MANAGE)) {

‎src/main/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignature.java‎

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ public class GHWebhookSignature {
2323

2424
private static final Logger LOGGER = LoggerFactory.getLogger(GHWebhookSignature.class);
2525
private static final String HMAC_SHA1_ALGORITHM = "HmacSHA1";
26+
private static final String HMAC_SHA256_ALGORITHM = "HmacSHA256";
2627
public static final String INVALID_SIGNATURE = "COMPUTED_INVALID_SIGNATURE";
2728

2829
private final String payload;
@@ -47,19 +48,42 @@ public static GHWebhookSignature webhookSignature(String payload, Secret secret)
4748
/**
4849
* Computes a RFC 2104-compliant HMAC digest using SHA1 of a payloadFrom with a given key (secret).
4950
*
51+
* @deprecated Use {@link #sha256()} for enhanced security
5052
* @return HMAC digest of payloadFrom using secret as key. Will return COMPUTED_INVALID_SIGNATURE
5153
* on any exception during computation.
5254
*/
55+
@Deprecated
5356
public String sha1() {
57+
return computeSignature(HMAC_SHA1_ALGORITHM);
58+
}
59+
60+
/**
61+
* Computes a RFC 2104-compliant HMAC digest using SHA256 of a payload with a given key (secret).
62+
* This is the recommended method for webhook signature validation.
63+
*
64+
* @return HMAC digest of payload using secret as key. Will return COMPUTED_INVALID_SIGNATURE
65+
* on any exception during computation.
66+
* @since 1.45.0
67+
*/
68+
public String sha256() {
69+
return computeSignature(HMAC_SHA256_ALGORITHM);
70+
}
71+
/**
72+
* Computes HMAC signature using the specified algorithm.
73+
*
74+
* @param algorithm The HMAC algorithm to use (e.g., "HmacSHA1", "HmacSHA256")
75+
* @return HMAC digest as hex string, or INVALID_SIGNATURE on error
76+
*/
77+
private String computeSignature(String algorithm) {
5478
try {
55-
final SecretKeySpec keySpec = new SecretKeySpec(secret.getPlainText().getBytes(UTF_8), HMAC_SHA1_ALGORITHM);
56-
final Mac mac = Mac.getInstance(HMAC_SHA1_ALGORITHM);
79+
final SecretKeySpec keySpec = new SecretKeySpec(secret.getPlainText().getBytes(UTF_8), algorithm);
80+
final Mac mac = Mac.getInstance(algorithm);
5781
mac.init(keySpec);
5882
final byte[] rawHMACBytes = mac.doFinal(payload.getBytes(UTF_8));
5983

6084
return Hex.encodeHexString(rawHMACBytes);
6185
} catch (Exception e) {
62-
LOGGER.error("", e);
86+
LOGGER.error("Error computing {} signature", algorithm, e);
6387
return INVALID_SIGNATURE;
6488
}
6589
}
@@ -68,15 +92,44 @@ public String sha1() {
6892
* @param digest computed signature from external place (GitHub)
6993
*
7094
* @return true if computed and provided signatures identical
95+
* @deprecated Use {@link #matches(String, SignatureAlgorithm)} for explicit algorithm selection
7196
*/
97+
@Deprecated
7298
public boolean matches(String digest) {
73-
String computed = sha1();
74-
LOGGER.trace("Signature: calculated={} provided={}", computed, digest);
99+
return matches(digest, SignatureAlgorithm.SHA1);
100+
}
101+
102+
/**
103+
* Validates a signature using the specified algorithm.
104+
* Uses constant-time comparison to prevent timing attacks.
105+
*
106+
* @param digest the signature to validate (without algorithm prefix)
107+
* @param algorithm the signature algorithm to use
108+
* @return true if computed and provided signatures match
109+
* @since 1.45.0
110+
*/
111+
public boolean matches(String digest, SignatureAlgorithm algorithm) {
112+
String computed;
113+
switch (algorithm) {
114+
case SHA256:
115+
computed = sha256();
116+
break;
117+
case SHA1:
118+
computed = sha1();
119+
break;
120+
default:
121+
LOGGER.warn("Unsupported signature algorithm: {}", algorithm);
122+
return false;
123+
}
124+
125+
LOGGER.trace("Signature validation: algorithm={} calculated={} provided={}",
126+
algorithm, computed, digest);
75127
if (digest == null && computed == null) {
76128
return true;
77129
} else if (digest == null || computed == null) {
78130
return false;
79131
} else {
132+
// Use constant-time comparison to prevent timing attacks
80133
return MessageDigest.isEqual(computed.getBytes(UTF_8), digest.getBytes(UTF_8));
81134
}
82135
}

‎src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java‎

Lines changed: 68 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@
2727
import java.nio.charset.StandardCharsets;
2828
import java.security.interfaces.RSAPublicKey;
2929
import java.util.List;
30-
import java.util.Objects;
31-
import java.util.stream.Collectors;
3230

3331
import static com.cloudbees.jenkins.GitHubWebHook.X_INSTANCE_IDENTITY;
3432
import static com.google.common.base.Charsets.UTF_8;
@@ -61,12 +59,23 @@
6159
class Processor extends Interceptor {
6260
private static final Logger LOGGER = getLogger(Processor.class);
6361
/**
64-
* Header key being used for the payload signatures.
62+
* Header key being used for the legacy SHA-1 payload signatures.
6563
*
6664
* @see <a href=https://developer.github.com/webhooks/>Developer manual</a>
65+
* @deprecated Use SHA-256 signatures with X-Hub-Signature-256 header
6766
*/
67+
@Deprecated
6868
public static final String SIGNATURE_HEADER = "X-Hub-Signature";
69-
private static final String SHA1_PREFIX = "sha1=";
69+
/**
70+
* Header key being used for the SHA-256 payload signatures (recommended).
71+
*
72+
* @see <a href="https://docs.github.com/en/developers/webhooks-and-events/webhooks/securing-your-webhooks">
73+
* GitHub Documentation</a>
74+
* @since 1.45.0
75+
*/
76+
public static final String SIGNATURE_HEADER_SHA256 = "X-Hub-Signature-256";
77+
public static final String SHA1_PREFIX = "sha1=";
78+
public static final String SHA256_PREFIX = "sha256=";
7079

7180
@Override
7281
public Object invoke(StaplerRequest2 req, StaplerResponse2 rsp, Object instance, Object[] arguments)
@@ -139,25 +148,66 @@ protected void shouldContainParseablePayload(Object[] arguments) throws Invocati
139148
* if a hook secret is specified in the GitHub plugin config.
140149
* If no hook secret is configured, then the signature is ignored.
141150
*
151+
* Uses the configured signature algorithm (SHA-256 by default, SHA-1 for legacy support).
152+
*
142153
* @param req Incoming request.
143154
* @throws InvocationTargetException if any of preconditions is not satisfied
144155
*/
145156
protected void shouldProvideValidSignature(StaplerRequest2 req, Object[] args)
146157
throws InvocationTargetException {
147-
List<Secret> secrets = GitHubPlugin.configuration().getHookSecretConfigs().stream().
148-
map(HookSecretConfig::getHookSecret).filter(Objects::nonNull).collect(Collectors.toList());
149-
150-
if (!secrets.isEmpty()) {
151-
Optional<String> signHeader = Optional.fromNullable(req.getHeader(SIGNATURE_HEADER));
152-
isTrue(signHeader.isPresent(), "Signature was expected, but not provided");
153-
154-
String digest = substringAfter(signHeader.get(), SHA1_PREFIX);
155-
LOGGER.trace("Trying to verify sign from header {}", signHeader.get());
156-
isTrue(
157-
secrets.stream().anyMatch(secret ->
158-
GHWebhookSignature.webhookSignature(payloadFrom(req, args), secret).matches(digest)),
159-
String.format("Provided signature [%s] did not match to calculated", digest)
160-
);
158+
List<HookSecretConfig> secretConfigs = GitHubPlugin.configuration().getHookSecretConfigs();
159+
160+
if (!secretConfigs.isEmpty()) {
161+
boolean validSignatureFound = false;
162+
163+
for (HookSecretConfig config : secretConfigs) {
164+
Secret secret = config.getHookSecret();
165+
if (secret == null) {
166+
continue;
167+
}
168+
169+
SignatureAlgorithm algorithm = config.getSignatureAlgorithm();
170+
String headerName = algorithm.getHeaderName();
171+
String expectedPrefix = algorithm.getSignaturePrefix();
172+
173+
Optional<String> signHeader = Optional.fromNullable(req.getHeader(headerName));
174+
if (!signHeader.isPresent()) {
175+
LOGGER.debug("No signature header {} found for algorithm {}", headerName, algorithm);
176+
continue;
177+
}
178+
179+
String fullSignature = signHeader.get();
180+
if (!fullSignature.startsWith(expectedPrefix)) {
181+
LOGGER.debug("Signature header {} does not start with expected prefix {}",
182+
fullSignature, expectedPrefix);
183+
continue;
184+
}
185+
186+
String digest = substringAfter(fullSignature, expectedPrefix);
187+
LOGGER.trace("Verifying {} signature from header {}", algorithm, fullSignature);
188+
189+
boolean isValid = GHWebhookSignature.webhookSignature(payloadFrom(req, args), secret)
190+
.matches(digest, algorithm);
191+
192+
if (isValid) {
193+
validSignatureFound = true;
194+
// Log deprecation warning for SHA-1 usage
195+
if (algorithm == SignatureAlgorithm.SHA1) {
196+
LOGGER.warn("Using deprecated SHA-1 signature validation. "
197+
+ "Consider upgrading webhook configuration to use SHA-256 "
198+
+ "for enhanced security.");
199+
} else {
200+
LOGGER.debug("Successfully validated {} signature", algorithm);
201+
}
202+
break;
203+
} else {
204+
LOGGER.debug("Signature validation failed for algorithm {}", algorithm);
205+
}
206+
}
207+
208+
isTrue(validSignatureFound,
209+
"No valid signature found. Ensure webhook is configured with a supported signature algorithm "
210+
+ "(SHA-256 recommended, SHA-1 for legacy compatibility).");
161211
}
162212
}
163213

0 commit comments

Comments
 (0)