Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-48012

Webhook signature checking is skipped if incoming webhook has no signature

XMLWordPrintable

      Looking at this code https://github.com/jenkinsci/github-plugin/blob/68ceb5960549c6a5ce55c5288c7eaabbbb3719a2/src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java#L145

      This means that if a secret is configured but the webhook doesn't have a signature, the request is allowed. I would expect that is a secret is configured, any webhook without a signature should be rejected, i.e.:

      if(Optional.fromNullable(secret).isPresent()) {
  if(signHeader.isPresent()) {
    // Do the existing check
   } else {
    // fail the hook
  }
}

        1. github.jpg
          593 kB
          Nathan Vahrenberg
        2. jenkins.jpg
          38 kB
          Nathan Vahrenberg

          [JENKINS-48012] Webhook signature checking is skipped if incoming webhook has no signature

          Noah Kantrowitz created issue -
          Matthias Silbernagl made changes -
          Link New: This issue is duplicated by JENKINS-48762 [ JENKINS-48762 ]

          Jonathan Pelletier added a comment - - edited

          This seems to have been introduced by the "fix" of -JENKINS-37481- (commit 05ad7b42033)... I obviously don't know all the ramifications, but the description in -JENKINS-37481- isn't really clear about the webhook configuration after the upgrade. It sounds like the webhook did have a signature token and the plugin's configuration wasn't updated accordingly after the update from 1.20.0 to 1.21.0.

          All of this to say that I don't quite understand why this bug has been fixed this way. The /github-webhook/ endpoint is inbound, so the Jenkins configuration should have priority over what's coming from a request (imho), as it acts similarly to a whitelist. Otherwise, anyone can ping a Jenkins instance visible from the internet and omit this header and the request will be processed successfully. Maybe a distinct option in the UI to disable the security checks would be more explicit instead of using no credentials?

          Any idea when this can be fixed?

          Jonathan Pelletier added a comment - - edited This seems to have been introduced by the "fix" of - JENKINS-37481 - ( commit 05ad7b42033 )... I obviously don't know all the ramifications, but the description in - JENKINS-37481 - isn't really clear about the webhook configuration after the upgrade. It sounds like the webhook did have a signature token and the plugin's configuration wasn't updated accordingly after the update from 1.20.0 to 1.21.0. All of this to say that I don't quite understand why this bug has been fixed this way. The /github-webhook/ endpoint is inbound, so the Jenkins configuration should have priority over what's coming from a request (imho), as it acts similarly to a whitelist. Otherwise, anyone can ping a Jenkins instance visible from the internet and omit this header and the request will be processed successfully. Maybe a distinct option in the UI to disable the security checks would be more explicit instead of using no credentials? Any idea when this can be fixed?
          Jesse Glick made changes -
          Status Original: Open [ 1 ] New: In Progress [ 3 ]
          Jesse Glick made changes -
          Status Original: In Progress [ 3 ] New: In Review [ 10005 ]

          SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Matthias Silbernagl
          Path:
          src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java
          src/test/java/org/jenkinsci/plugins/github/test/HookSecretHelper.java
          src/test/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayloadTest.java
          http://jenkins-ci.org/commit/github-plugin/8bb18cd8f25f94ec97b3df25747a5ad7b8414e4d
          Log:
          JENKINS-48012 Require a X-Hub-Signature header when receiving a hook payload and if… (#188)

          • Require a X-Hub-Signature header when receiving a hook payload and if a secret is configured
          • Make it clear that the hook signature is only validated if a hook secret is specified in the GitHub plugin config

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Matthias Silbernagl Path: src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java src/test/java/org/jenkinsci/plugins/github/test/HookSecretHelper.java src/test/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayloadTest.java http://jenkins-ci.org/commit/github-plugin/8bb18cd8f25f94ec97b3df25747a5ad7b8414e4d Log: JENKINS-48012 Require a X-Hub-Signature header when receiving a hook payload and if… (#188) Require a X-Hub-Signature header when receiving a hook payload and if a secret is configured Make it clear that the hook signature is only validated if a hook secret is specified in the GitHub plugin config
          Nathan Vahrenberg made changes -
          Attachment New: jenkins.jpg [ 41176 ]
          Nathan Vahrenberg made changes -
          Attachment New: github.jpg [ 41177 ]

          Nathan Vahrenberg added a comment -

          Can you offer any advice on how to properly configure Jenkins to handle Github webhooks? After updating the Github Plugin to 1.29.0 our webhooks broke with the error indicating that a signature was expected but not received. In the central Jenkins configuration I have an admin token set up and I have Jenkins configured to automatically manage hooks, which works as expected before updating:

          The hook that is registered on Github doesn't have anything in the 'Secret' field - is that what Jenkins is expecting to see?

          Nathan Vahrenberg added a comment - Can you offer any advice on how to properly configure Jenkins to handle Github webhooks? After updating the Github Plugin to 1.29.0 our webhooks broke with the error indicating that a signature was expected but not received. In the central Jenkins configuration I have an admin token set up and I have Jenkins configured to automatically manage hooks, which works as expected before updating: The hook that is registered on Github doesn't have anything in the 'Secret' field - is that what Jenkins is expecting to see?
          Jesse Glick made changes -
          Labels New: security

            lanwen Kirill Merkushev
            coderanger Noah Kantrowitz
            Votes:
            1 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: