Added GitHub signature checking feature + tests

pom.xml

@@ -10,7 +10,7 @@
 
     <artifactId>ghprb</artifactId>
     <name>GitHub Pull Request Builder</name>
-    <version>1.23-SNAPSHOT</version>
+    <version>1.24-SNAPSHOT</version>
     <packaging>hpi</packaging>
 
     <url>https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin</url>

src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java

@@ -391,9 +391,7 @@ private static String createCredentials(String serverAPIUrl, StandardCredentials
         specifications.add(new SchemeSpecification(serverUri.getScheme()));
         specifications.add(new PathSpecification(serverUri.getPath(), null, false));
 
-
-
-        Domain domain = new Domain(serverAPIUrl, "Auto generated credentials domain", specifications);
+        Domain domain = new Domain(serverUri.getHost(), "Auto generated credentials domain", specifications);
         CredentialsStore provider = new SystemCredentialsProvider.StoreImpl();
         provider.addDomain(domain, credentials);
         return credentials.getId();

src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java

@@ -6,6 +6,7 @@
 import hudson.model.TaskListener;
 import jenkins.model.Jenkins;
 
+import org.apache.commons.codec.binary.Hex;
 import org.jenkinsci.plugins.ghprb.extensions.GhprbCommentAppender;
 import org.jenkinsci.plugins.ghprb.extensions.GhprbCommitStatusException;
 import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension;
@@ -14,6 +15,9 @@
 import org.kohsuke.github.GHEventPayload.IssueComment;
 import org.kohsuke.github.GHEventPayload.PullRequest;
 
+import javax.crypto.Mac;
+import javax.crypto.spec.SecretKeySpec;
+
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.PrintStream;
@@ -302,6 +306,37 @@ void onPullRequestHook(PullRequest pr) {
         GhprbTrigger.getDscp().save();
     }
 
+    boolean checkSignature(String body, String signature) {
+        String secret = helper.getTrigger().getSecret();
+        if (secret != null && ! secret.isEmpty()) {
+            if (signature != null && signature.startsWith("sha1=")) {
+                String expected = signature.substring(5);
+                String algorithm = "HmacSHA1";
+                try {
+                    SecretKeySpec keySpec = new SecretKeySpec(secret.getBytes(), algorithm);
+                    Mac mac = Mac.getInstance(algorithm);
+                    mac.init(keySpec);
+                    byte[] localSignatureBytes = mac.doFinal(body.getBytes("UTF-8"));
+                    String localSignature = Hex.encodeHexString(localSignatureBytes);
+                    if (! localSignature.equals(expected)) {
+                        logger.log(Level.SEVERE, "Local signature {0} does not match external signature {1}",
+                                new Object[] {localSignature, expected});
+                        return false;
+                    }
+                } catch (Exception e) {
+                    logger.log(Level.SEVERE, "Couldn't match both signatures");
+                    return false;
+                }
+            } else {
+                logger.log(Level.SEVERE, "Request doesn't contain a signature. Check that github has a secret that should be attached to the hook");
+                return false;
+            }
+        }
+
+        logger.log(Level.INFO, "Signatures checking OK");
+        return true;
+    }
+
     @VisibleForTesting
     void setHelper(Ghprb helper) {
         this.helper = helper;

src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java

@@ -15,6 +15,9 @@
 
 import java.io.BufferedReader;
 import java.io.IOException;
+import java.io.StringReader;
+import java.io.UnsupportedEncodingException;
+import java.net.URLDecoder;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.logging.Level;
@@ -47,27 +50,31 @@ public String getUrlName() {
 
     public void doIndex(StaplerRequest req, StaplerResponse resp) {
         String event = req.getHeader("X-GitHub-Event");
+        String signature = req.getHeader("X-Hub-Signature");
         String type = req.getContentType();
         String payload = null;
+        String body = null;
 
         if ("application/json".equals(type)) {
-            BufferedReader br = null;
-            try {
-                br = req.getReader();
-                payload = IOUtils.toString(req.getReader());
-            } catch (IOException e) {
+            body = extractRequestBody(req);
+            if (body == null) {
                 logger.log(Level.SEVERE, "Can't get request body for application/json.");
                 return;
-            } finally {
-                IOUtils.closeQuietly(br);
             }
+            payload = body;
         } else if ("application/x-www-form-urlencoded".equals(type)) {
-            payload = req.getParameter("payload");
-            if (payload == null) {
+            body = extractRequestBody(req);
+            if (body == null || body.length() <= 8) {
                 logger.log(Level.SEVERE, "Request doesn't contain payload. "
                         + "You're sending url encoded request, so you should pass github payload through 'payload' request parameter");
                 return;
             }
+            try {
+                payload = URLDecoder.decode(body.substring(8), req.getCharacterEncoding());
+            } catch (UnsupportedEncodingException e) {
+                logger.log(Level.SEVERE, "Error while trying to encode the payload");
+                return;
+            }
         }
 
         if (payload == null) {
@@ -79,13 +86,28 @@ public void doIndex(StaplerRequest req, StaplerResponse resp) {
 
         for (GhprbWebHook webHook : getWebHooks()) {
             try {
-                webHook.handleWebHook(event, payload);
+                webHook.handleWebHook(event, payload, body, signature);
             } catch (Exception e) {
                 logger.log(Level.SEVERE, "Unable to process web hook for: " + webHook.getProjectName(), e);
             }
         }
 
     }
+
+    private String extractRequestBody(StaplerRequest req) {
+        String body = null;
+        BufferedReader br = null;
+        try {
+            br = req.getReader();
+            body = IOUtils.toString(br);
+        } catch (IOException e) {
+            body = null;
+        } finally {
+            IOUtils.closeQuietly(br);
+        }
+        return body;
+    }
+
 
     private Set<GhprbWebHook> getWebHooks() {
         final Set<GhprbWebHook> webHooks = new HashSet<GhprbWebHook>();

src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java

@@ -67,10 +67,11 @@ public class GhprbTrigger extends GhprbTriggerBackwardsCompatible {
     private List<GhprbBranch> whiteListTargetBranches;
     private transient Ghprb helper;
     private String project;
+    private String secret;
     private GhprbGitHubAuth gitHubApiAuth;
 
 
-    private DescribableList<GhprbExtension, GhprbExtensionDescriptor> extensions;
+    private DescribableList<GhprbExtension, GhprbExtensionDescriptor> extensions = new DescribableList<GhprbExtension, GhprbExtensionDescriptor>(Saveable.NOOP);
 
     public DescribableList<GhprbExtension, GhprbExtensionDescriptor> getExtensions() {
         if (extensions == null) {
@@ -94,23 +95,25 @@ private void setExtensions(List<GhprbExtension> extensions) {
     }
 
     @DataBoundConstructor
-    public GhprbTrigger(String adminlist, 
-            String whitelist, 
-            String orgslist, 
-            String cron, 
-            String triggerPhrase, 
+
+    public GhprbTrigger(String adminlist,
+            String whitelist,
+            String orgslist,
+            String cron,
+            String triggerPhrase,
             Boolean onlyTriggerPhrase, 
-            Boolean useGitHubHooks, 
+            Boolean useGitHubHooks,
             Boolean permitAll,
-            Boolean autoCloseFailedPullRequests, 
-            Boolean displayBuildErrorsOnDownstreamBuilds, 
+            Boolean autoCloseFailedPullRequests,
+            Boolean displayBuildErrorsOnDownstreamBuilds,
             String commentFilePath, 
             List<GhprbBranch> whiteListTargetBranches,
             Boolean allowMembersOfWhitelistedOrgsAsAdmin, 
             String msgSuccess, 
             String msgFailure, 
             String commitStatusContext,
             String gitHubAuthId,
+            String secret,
             List<GhprbExtension> extensions
             ) throws ANTLRException {
         super(cron);
@@ -127,6 +130,7 @@ public GhprbTrigger(String adminlist,
         this.whiteListTargetBranches = whiteListTargetBranches;
         this.gitHubApiAuth = getDescriptor().getGitHubAuth(gitHubAuthId);
         this.allowMembersOfWhitelistedOrgsAsAdmin = allowMembersOfWhitelistedOrgsAsAdmin;
+        this.secret = secret;
         setExtensions(extensions);
         configVersion = 1;
     }
@@ -357,6 +361,10 @@ public String getCron() {
         return cron;
     }
 
+
+    public String getSecret() {
+        return secret;
+    }
     public String getProject() {
         return project;
     }
@@ -482,7 +490,7 @@ public GhprbGitHubAuth getGitHubAuth(String gitHubAuthId) {
         public List<GhprbGitHubAuth> getGithubAuth() {
             if (githubAuth == null || githubAuth.size() == 0) {
                 githubAuth = new ArrayList<GhprbGitHubAuth>(1);
-                githubAuth.add(new GhprbGitHubAuth(null, null, "Blank description", null));
+                githubAuth.add(new GhprbGitHubAuth(null, null, "Anonymous connection", null));
             }
             return githubAuth;
         }
@@ -491,16 +499,11 @@ public List<GhprbGitHubAuth> getDefaultAuth(List<GhprbGitHubAuth> githubAuth) {
             if (githubAuth != null && githubAuth.size() > 0) {
                 return githubAuth;
             }
-            List<GhprbGitHubAuth> defaults = new ArrayList<GhprbGitHubAuth>(1);
-            defaults.add(new GhprbGitHubAuth(null, null, "Blank description", null));
-            return defaults;
+            return getGithubAuth();
         }
 
-
-
         private String adminlist;
 
-
         private String requestForTestingPhrase;
 
         // map of jobs (by their fullName) and their map of pull requests
@@ -655,7 +658,7 @@ public boolean isUseDetailedComments() {
 
         public ListBoxModel doFillGitHubAuthIdItems(@QueryParameter("gitHubAuth") String gitHubAuthId) {
             ListBoxModel model = new ListBoxModel();
-            for (GhprbGitHubAuth auth : githubAuth) {
+            for (GhprbGitHubAuth auth : getGithubAuth()) {
                 String description = Util.fixNull(auth.getDescription());
                 int length = description.length();
                 length = length > 50 ? 50 : length;

src/main/java/org/jenkinsci/plugins/ghprb/GhprbWebHook.java

@@ -19,7 +19,7 @@ public GhprbWebHook(GhprbTrigger trigger) {
         this.trigger = trigger;
     }
 
-    public void handleWebHook(String event, String payload) {
+    public void handleWebHook(String event, String payload, String body, String signature) {
 
         GhprbRepository repo = trigger.getRepository();
 
@@ -41,7 +41,8 @@ public void handleWebHook(String event, String payload) {
                     logger.log(Level.INFO, "Checking issue comment ''{0}'' for repo {1}", 
                             new Object[] { issueComment.getComment(), repo.getName() }
                     );
-                    repo.onIssueCommentHook(issueComment);
+                    if (repo.checkSignature(body, signature))
+                        repo.onIssueCommentHook(issueComment);
                 }
 
             } else if ("pull_request".equals(event)) {
@@ -50,7 +51,8 @@ public void handleWebHook(String event, String payload) {
                         GHEventPayload.PullRequest.class);
                 if (matchRepo(repo, pr.getPullRequest().getRepository())) {
                     logger.log(Level.INFO, "Checking PR #{1} for {0}", new Object[] { repo.getName(), pr.getNumber() });
-                    repo.onPullRequestHook(pr);
+                    if (repo.checkSignature(body, signature))
+                        repo.onPullRequestHook(pr);
                 }
 
             } else {

src/main/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus.java

@@ -111,7 +111,7 @@ private void createCommitStatus(AbstractBuild<?, ?> build, TaskListener listener
 
         listener.getLogger().println(String.format("Setting status of %s to %s with url %s and message: '%s'", sha1, state, url, message));
         if (context != null) {
-            listener.getLogger().println(String.format("Using conext: " + context));
+            listener.getLogger().println(String.format("Using context: " + context));
         }
         try {
             repo.createCommitStatus(sha1, state, url, message, context);

src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.groovy

@@ -1,4 +1,3 @@
-// Namespaces
 xml = namespace("http://www.w3.org/XML/1998/namespace")
 j = namespace("jelly:core")
 f = namespace("/lib/form")
@@ -61,6 +60,7 @@ f.advanced() {
 }
 f.advanced(title: _("Trigger Setup")) {
   f.entry(title: _("Trigger Setup")) {
-    f.hetero_list(items: instance.extensions, name: "extensions", oneEach: "true", hasHeader: "true", descriptors: descriptor.getExtensionDescriptors()) 
+    f.hetero_list(items: instance == null ? null : instance.extensions, 
+        name: "extensions", oneEach: "true", hasHeader: "true", descriptors: descriptor.getExtensionDescriptors()) 
   }
 }

src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.jelly.tmp

@@ -6,6 +6,15 @@
     <f:checkbox />
   </f:entry>
   <f:advanced>
+    <f:entry title="${%Shared secret}" field="secret">
+      <f:password />
+    </f:entry>
+    <f:entry title="${%Success message}" field="msgSuccess">
+      <f:textarea default="${descriptor.msgSuccess}"/>
+    </f:entry>
+    <f:entry title="${%Failure message}" field="msgFailure">
+      <f:textarea default="${descriptor.msgFailure}"/>
+    </f:entry>
 	<f:entry title="${%Trigger phrase}" field="triggerPhrase">
 	  <f:textbox />
 	</f:entry>

src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/global.groovy

@@ -1,4 +1,3 @@
-// Namespaces
 xml = namespace("http://www.w3.org/XML/1998/namespace")
 j = namespace("jelly:core")
 f = namespace("/lib/form")

src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMergeTest.java

@@ -90,7 +90,7 @@ public class GhprbPullRequestMergeTest {
     private final String mergeComment = "merge";
 
     private final Integer pullId = 1;
-    
+
     private Map<String, Object> triggerValues;
 
     @Before