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

Webhook signature checking is skipped if incoming webhook has no signature

    XMLWordPrintable

Details

    Description

      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
        }
      }
      

      Attachments

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

        Issue Links

          Activity

            pelletierjon 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?

            pelletierjon 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?

            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_issue_link 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

            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?

            nv035674 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?

            People

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

              Dates

                Created:
                Updated:
                Resolved: