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

Webhook signature checking is skipped if incoming webhook has no signature

      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
          github.jpg
          593 kB
        2. jenkins.jpg
          jenkins.jpg
          38 kB

          [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 ]
          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 ]
          Nathan Vahrenberg made changes -
          Attachment New: jenkins.jpg [ 41176 ]
          Nathan Vahrenberg made changes -
          Attachment New: github.jpg [ 41177 ]
          Jesse Glick made changes -
          Labels New: security
          Jesse Glick made changes -
          Remote Link New: This issue links to "github-plugin #188 (Web Link)" [ 24885 ]
          Jesse Glick made changes -
          Released As New: https://github.com/jenkinsci/github-plugin/releases/tag/v1.29.0
          Resolution New: Fixed [ 1 ]
          Status Original: In Review [ 10005 ] New: Resolved [ 5 ]

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

              Created:
              Updated:
              Resolved: