Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid Signature #8

Closed
freewil opened this issue Mar 1, 2014 · 7 comments
Closed

Invalid Signature #8

freewil opened this issue Mar 1, 2014 · 7 comments

Comments

@freewil
Copy link
Contributor

freewil commented Mar 1, 2014

GitHub webhook POSTs are being rejected with the message Invalid Signature. It seems like GitHub may have recently changed their API, because the POSTs do not contain a X-Hub-Signature for Strider to verify.

I have tried deleting the service hooks and re-adding them from the strider project config page to no avail.

@jaredly
Copy link
Member

jaredly commented Mar 1, 2014

Hmm the github docs still say it will do that... http://developer.github.com/v3/repos/hooks/#example

@niallo
Copy link
Member

niallo commented Mar 1, 2014

May be worth reporting to github support. I have seen API breakages in the
past, but they fixed them extremely quickly.

On Friday, February 28, 2014, Jared Forsyth notifications@github.com
wrote:

Hmm the github docs still say it will do that...
http://developer.github.com/v3/repos/hooks/#example

Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-36417158
.

Niall O'Higgins
W: http://niallohiggins.com
E: n@niallo.me
T: @niallohiggins

@freewil
Copy link
Contributor Author

freewil commented Mar 2, 2014

It seems this is a bug on the @github side. There is a case where GitHub will not send the X-Hub-Signature request header.

  1. First of all, GitHub is rejecting my SSL certificate for my Strider install (not sure why, Chrome thinks it's cool).

  2. Strider creates the webhook through GitHub's API with the following config (I've added content_type and insecure_ssl in my testing using the documented GitHub defaults):

...
  "config": {
    "url": "https://strider.example.com/user/repo/api/github/webhook",
    "secret": "mysecret",
    "content_type": "form",
    "insecure_ssl": 0
  }
...
  1. As soon as the webhook is added, GitHub sends off a ping event, which never hits the server because GitHub appears to be rejecting the server certificate.

github-ssl-reject

  1. I then disable the SSL verification through the project settings on GitHub (which I assume should simply change the value of insecure_ssl from 0 to 1).

github-disable-ssl-verification

  1. Now, after disabling the SSL verification, the request from GitHub will hit Strider, but the requests no longer have the X-Hub-Signature header.
    github-no-sig

Conclusion:

  1. it seems to be a bug on GitHub's side that X-Hub-Signature request headers are not sent with requests after disabling ssl verification if it was created originally with it on.

  2. GitHub should make SSL verification rejections displayed more clearly, rather than just showing a response of 0 bytes.

  3. Strider does not properly handle ping events from GitHub that are sent immediately after enabling the webhook (it tries to treat them as commits).

POST /[redacted]/api/github/hook 200 120ms - 18b
2 Mar 08:51:46 - info: got a body: { payload: '{"zen":"Favor focus over features.","hook_id": "redacted"}' }
2 Mar 08:51:46 - error: TypeError: Cannot call method 'indexOf' of undefined
    at pushJob (/secure_fs/strider/strider/node_modules/strider-github/lib/webhooks.js:122:19)
    at startFromCommit (/secure_fs/strider/strider/node_modules/strider-github/lib/webhooks.js:42:16)
    at receiveWebhook (/secure_fs/strider/strider/node_modules/strider-github/lib/webhooks.js:216:3)
    at callbacks (/secure_fs/strider/strider/node_modules/express/lib/router/index.js:164:37)
    at projectProvider (/secure_fs/strider/strider/lib/middleware.js:107:3)
    at callbacks (/secure_fs/strider/strider/node_modules/express/lib/router/index.js:164:37)
    at Promise.<anonymous> (/secure_fs/strider/strider/lib/middleware.js:201:5)
    at Promise.<anonymous> (/secure_fs/strider/strider/node_modules/mongoose/node_modules/mpromise/lib/promise.js:162:8)
    at Promise.EventEmitter.emit (events.js:95:17)
    at Promise.emit (/secure_fs/strider/strider/node_modules/mongoose/node_modules/mpromise/lib/promise.js:79:38)

@niallo
Copy link
Member

niallo commented Mar 2, 2014

Excellent detective work and analysis! We should fix the big with the
initial ping github web hook.

On Saturday, March 1, 2014, Sean Lavine notifications@github.com wrote:

It seems this is a bug on the @github https://github.com/github side.
There is a case where GitHub will not send the X-Hub-Signature request
header.

  1. First of all, GitHub is rejecting my SSL certificate for my Strider
    install (not sure why, Chrome thinks it's cool).

  2. Strider creates the webhook through GitHub's API with the following
    config (I've added content_type and insecure_ssl in my testing using the
    documented GitHub defaults):

...
"config": {
"url": "https://strider.example.com/user/repo/api/github/webhook",
"secret": "mysecret",
"content_type": "form",
"insecure_ssl": 0
}...

  1. As soon as the webhook is added, GitHub sends off a ping event, which
    never hits the server because GitHub appears to be rejecting the server
    certificate.

[image: github-ssl-reject]https://f.cloud.github.com/assets/716621/2303879/047e5c74-a1ee-11e3-9cdc-ff686606e266.png

  1. I then disable the SSL verification through the project settings on
    GitHub (which I assume should simply change the value of insecure_sslfrom
    0 to 1).

[image: github-disable-ssl-verification]https://f.cloud.github.com/assets/716621/2303881/6e574458-a1ee-11e3-99c7-28d0572b7b39.png

  1. Now, after disabling the SSL verification, the request from GitHub will
    hit Strider, but the requests no longer have the X-Hub-Signature
    [image: github-no-sig]https://f.cloud.github.com/assets/716621/2303888/3fa141b2-a1ef-11e3-8c9e-f3b4f53eac22.jpg

Conclusion:

  1. it seems to be a bug on GitHub's side that X-Hub-Signature request
    headers are not sent with requests after disabling ssl verification if it
    was created originally with it on.

  2. GitHub should make SSL verification rejections displayed more clearly,
    rather than just showing a response of 0 bytes.

  3. Strider does not properly handle ping events from GitHub that are sent
    immediately after enabling the webhook (it tries to treat them as commits).

POST /[redacted]/api/github/hook 200 120ms - 18b
2 Mar 08:51:46 - info: got a body: { payload: '{"zen":"Favor focus over features.","hook_id": "redacted"}' }
2 Mar 08:51:46 - error: TypeError: Cannot call method 'indexOf' of undefined
at pushJob (/secure_fs/strider/strider/node_modules/strider-github/lib/webhooks.js:122:19)
at startFromCommit (/secure_fs/strider/strider/node_modules/strider-github/lib/webhooks.js:42:16)
at receiveWebhook (/secure_fs/strider/strider/node_modules/strider-github/lib/webhooks.js:216:3)
at callbacks (/secure_fs/strider/strider/node_modules/express/lib/router/index.js:164:37)
at projectProvider (/secure_fs/strider/strider/lib/middleware.js:107:3)
at callbacks (/secure_fs/strider/strider/node_modules/express/lib/router/index.js:164:37)
at Promise. (/secure_fs/strider/strider/lib/middleware.js:201:5)
at Promise. (/secure_fs/strider/strider/node_modules/mongoose/node_modules/mpromise/lib/promise.js:162:8)
at Promise.EventEmitter.emit (events.js:95:17)
at Promise.emit (/secure_fs/strider/strider/node_modules/mongoose/node_modules/mpromise/lib/promise.js:79:38)

Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-36450348
.

Niall O'Higgins
W: http://niallohiggins.com
E: n@niallo.me
T: @niallohiggins

@freewil freewil closed this as completed Mar 8, 2014
@laurentj
Copy link

Hi,
I contacted the support of Github for this problem and here their response:

we just checked at it seems that we run over the "secret" parameter if you "Disable SSL verification" in your settings. As a result, when you disable ssl verification via settings, you no longer get the "X-Hub-Signature" header. We're working on a fix for that problem so that the "secret" parameter is preserved. I'm sorry for the trouble with this -- I'll get back to you once the fix is deployed.

A workaround you can use now is to remove your hook and set it up again. However, instead of using the "Disable SSL verification" button in settings, you can disable ssl verification using the API by editing the configuration of that hook:

http://developer.github.com/v3/repos/hooks/#create-a-hook
http://developer.github.com/v3/repos/hooks/#edit-a-hook

If you edit the config so that you both specify the hook's secret and set insecure_ssl to "1", then ssl verification should be disabled and you should get the "X-Hub-Signature" header with deliveries.

You could also just wait for us to deploy the fix and then you should be able to re-create the hook and just use the "Disable SSL verification" button in your settings.

@laurentj
Copy link

Hi,

Github has fixed the issue appearing when ssl verification is disabled:

We rolled out a change which should have fixed this issue. Could you please try removing your hook and recreating it, as we discussed? After you re-create it, clicking "Disable SSL verification" should leave the "secret" attribute intact so you should continue to receive the X-Hub-Signature header with deliveries.

Everything works for me now.

@jaredly
Copy link
Member

jaredly commented Mar 22, 2014

thanks for following up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants