GitHub webhooks: check signature #2493
Conversation
francois2metz
force-pushed
the
plugin/gh_mac_validation
branch
from
dcf29bb
to
68d6d6d
Compare
| @@ -29,6 +33,13 @@ func (gh *GithubWebhook) eventHandler(w http.ResponseWriter, r *http.Request) { | |||
| w.WriteHeader(http.StatusBadRequest) | |||
| return | |||
| } | |||
|
|
|||
| if gh.Secret != "" && !checkSignature(gh.Secret, data, r.Header["X-Hub-Signature"][0]) { | |||
There was a problem hiding this comment.
This will panic if the header X-Hub-Signature doesn't exist. It's recommended to use r.Header.Get("X-Hub-Signature") instead.
Same thing with the X-Github-Event up above. I know it's pre-existing code, but would probably be good time to fix it.
|
Note: I tested that in production: with and without secret, and with a non matching secret. |
| data, err := ioutil.ReadAll(r.Body) | ||
| if err != nil { | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
||
| if gh.Secret != "" && !checkSignature(gh.Secret, data, r.Header.Get("X-Hub-Signature")) { | ||
| log.Printf("I! Fail to check the github webhook signature\n") |
| return hmac.Equal([]byte(signature), []byte(generateSignature(secret, data))) | ||
| } | ||
|
|
||
| func generateSignature(secret string, data []byte) string { |
There was a problem hiding this comment.
instead of generating the signature on every github event, you should generate it just once and then cache for future calls
There was a problem hiding this comment.
The generation is dependant of the body of the event. Not sure what I can cache. The string to byte op?
|
Can you add this to the changelog? |
francois2metz
force-pushed
the
plugin/gh_mac_validation
branch
from
1994ba3
to
cad84e6
Compare
|
Rebased and updated the changelog and the readme. |
Fix #1661
I added the check of the signature to github webhooks. If no secret is set in the config file, no check is performed (make it backward compatible).
Required for all PRs: