Skip to content

Commit

Permalink
fix: bug in hmac verification (refs #216) (#217)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tomasbjerre
tomasbjerre committed Aug 28, 2021
1 parent cc4c324 commit 14f62f8
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 78 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# Generic Webhook Plugin Changelog
Changelog of Generic Webhook Plugin.
## Unreleased
### No issue

**chore: refactoring**


[cc4c3246b9f5a00](https://github.com/jenkinsci/generic-webhook-trigger-plugin/commit/cc4c3246b9f5a00) Tomas Bjerre *2021-08-28 04:56:02*


## 1.75 (2021-08-20 17:05:23)
### GitHub [#214](https://github.com/jenkinsci/generic-webhook-trigger-plugin/issues/214) Folder credentials not working when using tokenCredentialId *bug*

Expand Down
23 changes: 23 additions & 0 deletions src/main/java/org/jenkinsci/plugins/gwt/global/WhitelistAlgorithm.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package org.jenkinsci.plugins.gwt.global;

public enum WhitelistAlgorithm {
HMAC_MD5("HmacMD5", "md5"),
HMAC_SHA1("HmacSHA1", "sha1"),
HMAC_SHA256("HmacSHA256", "sha256");

private String fullName;
private String algorithm;

WhitelistAlgorithm(final String fullName, final String algorithm) {
this.fullName = fullName;
this.algorithm = algorithm;
}

public String getFullName() {
return this.fullName;
}

public String getAlgorithm() {
return this.algorithm;
}
}
27 changes: 11 additions & 16 deletions src/main/java/org/jenkinsci/plugins/gwt/global/WhitelistItem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
import hudson.util.FormValidation;
import hudson.util.ListBoxModel;
import java.io.Serializable;
import java.util.Arrays;
import java.util.List;
import org.jenkinsci.plugins.gwt.whitelist.WhitelistException;
import org.jenkinsci.plugins.gwt.whitelist.WhitelistHost;
import org.kohsuke.stapler.AncestorInPath;
Expand All @@ -19,17 +17,12 @@

public class WhitelistItem extends AbstractDescribableImpl<WhitelistItem> implements Serializable {

public static final String HMAC_MD5 = "HmacMD5";
public static final String HMAC_SHA1 = "HmacSHA1";
public static final String HMAC_SHA256 = "HmacSHA256";
private static final long serialVersionUID = 1176246137502450635L;
private String host;
private boolean hmacEnabled;
private String hmacHeader;
private String hmacCredentialId;
private String hmacAlgorithm;
private static final List<String> MAC_ALGORITHMS =
Arrays.asList(HMAC_MD5, HMAC_SHA1, HMAC_SHA256);

public WhitelistItem() {}

Expand All @@ -39,12 +32,14 @@ public WhitelistItem(final String host) {
}

public String getHost() {
if (host == null) return null;
return host.trim();
if (this.host == null) {
return null;
}
return this.host.trim();
}

public String getHmacAlgorithm() {
return hmacAlgorithm;
return this.hmacAlgorithm;
}

@DataBoundSetter
Expand All @@ -53,7 +48,7 @@ public void setHmacAlgorithm(final String hmacAlgorithm) {
}

public String getHmacCredentialId() {
return hmacCredentialId;
return this.hmacCredentialId;
}

@DataBoundSetter
Expand All @@ -62,7 +57,7 @@ public void setHmacCredentialId(final String hmacCredentialId) {
}

public boolean isHmacEnabled() {
return hmacEnabled;
return this.hmacEnabled;
}

@DataBoundSetter
Expand All @@ -71,7 +66,7 @@ public void setHmacEnabled(final boolean hmacEnabled) {
}

public String getHmacHeader() {
return hmacHeader;
return this.hmacHeader;
}

@DataBoundSetter
Expand All @@ -89,8 +84,8 @@ public String getDisplayName() {

public ListBoxModel doFillHmacAlgorithmItems() {
final ListBoxModel listBoxModel = new ListBoxModel();
for (final String a : MAC_ALGORITHMS) {
listBoxModel.add(a);
for (final WhitelistAlgorithm a : WhitelistAlgorithm.values()) {
listBoxModel.add(a.getFullName());
}
return listBoxModel;
}
Expand All @@ -114,7 +109,7 @@ public FormValidation doCheckHost(@QueryParameter final String value) {
try {
new WhitelistHost(value);
return FormValidation.ok();
} catch (WhitelistException e) {
} catch (final WhitelistException e) {
return FormValidation.error(e.getMessage());
}
}
Expand Down
34 changes: 23 additions & 11 deletions src/main/java/org/jenkinsci/plugins/gwt/whitelist/HMACVerifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.Map.Entry;
import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;
import org.jenkinsci.plugins.gwt.global.WhitelistAlgorithm;

public class HMACVerifier {

Expand All @@ -22,12 +23,15 @@ public static void hmacVerify(
final String algorithm)
throws WhitelistException {
final String headerValue = getHeaderValue(hmacHeader, headers);
final String calculateHmac = getCalculatedHmac(postContent, hmacSecret, algorithm);
final String calculateHmacBase64 =
new String(Base64.getEncoder().encode(calculateHmac.getBytes(UTF_8)), UTF_8);
final byte[] calculateHmacBytes = getCalculatedHmac(postContent, hmacSecret, algorithm);
final String calculateHmacAsString = Base64.getEncoder().encodeToString(calculateHmacBytes);
final String calculateHmacAsHex = bytesToHex(calculateHmacBytes);
final String calculateHmacAsHexAndBase64 =
new String(Base64.getEncoder().encode(calculateHmacAsHex.getBytes(UTF_8)), UTF_8);

if (!headerValue.equalsIgnoreCase(calculateHmac)
&& !headerValue.equalsIgnoreCase(calculateHmacBase64)) {
if (!headerValue.equalsIgnoreCase(calculateHmacAsHex)
&& !headerValue.equalsIgnoreCase(calculateHmacAsHexAndBase64)
&& !headerValue.equalsIgnoreCase(calculateHmacAsString)) {
throw new WhitelistException(
"HMAC verification failed with \""
+ hmacHeader
Expand All @@ -38,15 +42,14 @@ public static void hmacVerify(
}
}

private static String getCalculatedHmac(
private static byte[] getCalculatedHmac(
final String postContent, final String hmacSecret, final String algorithm) {
try {
final byte[] byteKey = hmacSecret.getBytes(UTF_8.name());
final Mac mac = Mac.getInstance(algorithm);
final SecretKeySpec keySpec = new SecretKeySpec(byteKey, algorithm);
mac.init(keySpec);
final byte[] mac_data = mac.doFinal(postContent.getBytes(UTF_8));
return bytesToHex(mac_data);
return mac.doFinal(postContent.getBytes(UTF_8));
} catch (UnsupportedEncodingException | NoSuchAlgorithmException | InvalidKeyException e) {
throw new RuntimeException(e);
}
Expand All @@ -70,9 +73,18 @@ private static String getHeaderValue(
final boolean oneValue = ck.getValue().size() == 1;
if (sameHeader && oneValue) {
final String value = ck.getValue().get(0);
if (value.contains("=") && !value.endsWith("=")) {
// To handle X-Hub-Signature: sha256=87e3e7...
return value.split("=")[1];
for (final WhitelistAlgorithm algorithm : WhitelistAlgorithm.values()) {
final String startString = algorithm.getAlgorithm() + "=";
if (value.startsWith(startString)) {
// To handle X-Hub-Signature: sha256=87e3e7...
return value.substring(startString.length());
}
final String startStringHmac = "HMAC ";
if (value.startsWith(startStringHmac)) {
// To handle teams signature authorization: HMAC
// w2g2swwmrsvRLZ5W68LfjaLrSR4fN0ErKGyfTPbLrBs=
return value.substring(startStringHmac.length()).trim();
}
}
return value;
}
Expand Down
119 changes: 68 additions & 51 deletions src/test/java/org/jenkinsci/plugins/gwt/whitelist/HMACVerifierTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,56 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.jenkinsci.plugins.gwt.whitelist.HMACVerifier.hmacVerify;

import java.io.IOException;
import java.net.URISyntaxException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.jenkinsci.plugins.gwt.global.WhitelistItem;
import org.jenkinsci.plugins.gwt.global.WhitelistAlgorithm;
import org.junit.Test;

public class HMACVerifierTest {

@Test
public void testThatHmacCanBeVerifiedAndValidHex() throws Exception {
final String postContent = this.getPostContent();
final String hmacSecret = "this is secret";
final String algorithm = WhitelistAlgorithm.HMAC_SHA256.getFullName();
final String hmacHeader = "X-Hub-Signature";
final String headerValue =
"sha256=87e3e7b7e4567f528342a75b6d88c619f272c68a4d0d565c68d596a830213164";
final Map<String, List<String>> headers = this.getHeaders(hmacHeader, headerValue);

final boolean actual =
this.testHmacVerify(headers, postContent, hmacHeader, hmacSecret, algorithm);
assertThat(actual).isTrue();
}

@Test
public void testThatHmacCanBeVerifiedAndValid() throws Exception {
final Map<String, List<String>> headers;
final String postContent =
new String(
Files.readAllBytes(
Paths.get(
this.getClass()
.getResource("/hmac/hmac-bitbucket-server-payload.json")
.toURI())),
UTF_8);
final String postContent = this.getPostContent();
final String hmacSecret = "this is secret";
final String algorithm = WhitelistAlgorithm.HMAC_SHA256.getFullName();
final String hmacHeader = "X-Hub-Signature";
final String headerValue = "sha256=h+Pnt+RWf1KDQqdbbYjGGfJyxopNDVZcaNWWqDAhMWQ=";
final Map<String, List<String>> headers = this.getHeaders(hmacHeader, headerValue);

final boolean actual =
this.testHmacVerify(headers, postContent, hmacHeader, hmacSecret, algorithm);
assertThat(actual).isTrue();
}

@Test
public void testThatHmacCanBeVerifiedAndValidHmacPrefixHeader() throws Exception {
final String postContent = this.getPostContent();
final String hmacSecret = "this is secret";
final String algorithm = WhitelistItem.HMAC_SHA256;
headers = new HashMap<>();
headers.put(
hmacHeader,
Arrays.asList("sha256=87e3e7b7e4567f528342a75b6d88c619f272c68a4d0d565c68d596a830213164"));
final String algorithm = WhitelistAlgorithm.HMAC_SHA256.getFullName();
final String hmacHeader = "X-Hub-Signature";
final String headerValue = "HMAC h+Pnt+RWf1KDQqdbbYjGGfJyxopNDVZcaNWWqDAhMWQ=";
final Map<String, List<String>> headers = this.getHeaders(hmacHeader, headerValue);

final boolean actual =
this.testHmacVerify(headers, postContent, hmacHeader, hmacSecret, algorithm);
Expand All @@ -41,16 +62,13 @@ public void testThatHmacCanBeVerifiedAndValid() throws Exception {

@Test
public void testThatHmacCanBeBase64() throws Exception {
final Map<String, List<String>> headers;
final String postContent = "whatever";
final String hmacHeader = "hmac";
final String hmacSecret = "this is secret";
final String algorithm = WhitelistItem.HMAC_SHA256;
headers = new HashMap<>();
headers.put(
hmacHeader,
Arrays.asList(
"NzEyMTJGODU0RTIzQzU3NUQ3QjFBQUQ0QzM0NjcwRkYwOEVCRjcyMUMzODM3NjY4NjEzRTk2Qzg3RjZFRThCMg=="));
final String algorithm = WhitelistAlgorithm.HMAC_SHA256.getFullName();
final String hmacHeader = "hmac";
final String headerValue =
"NzEyMTJGODU0RTIzQzU3NUQ3QjFBQUQ0QzM0NjcwRkYwOEVCRjcyMUMzODM3NjY4NjEzRTk2Qzg3RjZFRThCMg==";
final Map<String, List<String>> headers = this.getHeaders(hmacHeader, headerValue);

final boolean actual =
this.testHmacVerify(headers, postContent, hmacHeader, hmacSecret, algorithm);
Expand All @@ -60,22 +78,12 @@ public void testThatHmacCanBeBase64() throws Exception {

@Test
public void testThatHmacCanBeVerifiedAndValidWIthoutAlgorithmInHeader() throws Exception {
final Map<String, List<String>> headers;
final String postContent =
new String(
Files.readAllBytes(
Paths.get(
this.getClass()
.getResource("/hmac/hmac-bitbucket-server-payload.json")
.toURI())),
UTF_8);
final String hmacHeader = "X-Hub-Signature";
final String postContent = this.getPostContent();
final String hmacSecret = "this is secret";
final String algorithm = WhitelistItem.HMAC_SHA256;
headers = new HashMap<>();
headers.put(
hmacHeader,
Arrays.asList("87e3e7b7e4567f528342a75b6d88c619f272c68a4d0d565c68d596a830213164"));
final String algorithm = WhitelistAlgorithm.HMAC_SHA256.getFullName();
final String hmacHeader = "X-Hub-Signature";
final String headerValue = "87e3e7b7e4567f528342a75b6d88c619f272c68a4d0d565c68d596a830213164";
final Map<String, List<String>> headers = this.getHeaders(hmacHeader, headerValue);

final boolean actual =
this.testHmacVerify(headers, postContent, hmacHeader, hmacSecret, algorithm);
Expand All @@ -85,7 +93,27 @@ public void testThatHmacCanBeVerifiedAndValidWIthoutAlgorithmInHeader() throws E

@Test
public void testThatHmacCanBeVerifiedAndInvalid() throws Exception {
final Map<String, List<String>> headers;
final String postContent = this.getPostContent();
final String hmacSecret = "this is secret";
final String algorithm = WhitelistAlgorithm.HMAC_SHA256.getFullName();
final String hmacHeader = "X-Hub-Signature";
final String headerValue =
"sha256=97e3e7b7e4567f528342a75b6d88c619f272c68a4d0d565c68d596a830213164";
final Map<String, List<String>> headers = this.getHeaders(hmacHeader, headerValue);

final boolean actual =
this.testHmacVerify(headers, postContent, hmacHeader, hmacSecret, algorithm);

assertThat(actual).isFalse();
}

private Map<String, List<String>> getHeaders(final String hmacHeader, final String value) {
final Map<String, List<String>> headers = new HashMap<>();
headers.put(hmacHeader, Arrays.asList(value));
return headers;
}

private String getPostContent() throws IOException, URISyntaxException {
final String postContent =
new String(
Files.readAllBytes(
Expand All @@ -94,18 +122,7 @@ public void testThatHmacCanBeVerifiedAndInvalid() throws Exception {
.getResource("/hmac/hmac-bitbucket-server-payload.json")
.toURI())),
UTF_8);
final String hmacHeader = "X-Hub-Signature";
final String hmacSecret = "this is secret";
final String algorithm = WhitelistItem.HMAC_SHA256;
headers = new HashMap<>();
headers.put(
hmacHeader,
Arrays.asList("sha256=97e3e7b7e4567f528342a75b6d88c619f272c68a4d0d565c68d596a830213164"));

final boolean actual =
this.testHmacVerify(headers, postContent, hmacHeader, hmacSecret, algorithm);

assertThat(actual).isFalse();
return postContent;
}

private boolean testHmacVerify(
Expand Down

0 comments on commit 14f62f8

Please sign in to comment.