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

add time based MatchRule #267

Open
foosinn opened this issue Sep 17, 2018 · 5 comments
Open

add time based MatchRule #267

foosinn opened this issue Sep 17, 2018 · 5 comments

Comments

@foosinn
Copy link

foosinn commented Sep 17, 2018

Using a time based match rule would allow payload-hash-* secured hooks that are only valid for a configurable amount of time.

Using those would prevent a captured hook call once more, even if it has been payload-hash verified.

@foosinn foosinn changed the title Time based MatchRule add time based MatchRule Sep 17, 2018
@moorereason
Copy link
Collaborator

@foosinn,
If you're asking about rate-limiting hook executions, this is a duplicate of #120. It was closed because @adnanh didn't have time to implement it back then even though @antoineco seemed to suggest he might submit a PR. ?

This probably is a useful feature for many, but we need to flesh this out into a proposal. If we can agree on the proposal, we can label this issue as help wanted and hope someone has the time and inclination to implement it. So...

Proposed Feature

  • Add a rate-limit Hook property.
  • The value is a time.Duration string (will be parsed by time.ParseDuration). Example: 90s.
  • Used to rate limit the execution (not evaluation) of hook requests. Check rate limiting after trigger rule evaluation was successful.
  • HTTP response code during rate limiting will be 429 (http.StatusTooManyRequests).

Sample Configuration

[
  {
    "id": "webhook",
    "execute-command": "/home/adnan/redeploy-go-webhook.sh",
    "command-working-directory": "/home/adnan/go",
    "rate-limit": "90s",
    "trigger-rule":
    {
      "match":
      {
        "type": "ip-whitelist",
        "ip-range": "104.192.143.0/24"
      }
    }
  }
]

@antoineco
Copy link

Indeed, I wanted to implement a rate-limiting hook which seems to match the description, but never got an answer and eventually stopped using webhook (for other reasons), so I never got the motivation to invest the time.

@foosinn
Copy link
Author

foosinn commented Sep 18, 2018

No this is not about rate limiting.

I if did understand the docs correctly a person that manages to capture the webhook call can resend it anytime. Its possible to secure webhooks by using payload-hash-*, but this only verifies the content. If he sends the same content a potential attacker is able to rerun the webhook.

Or did i miss something?

@moorereason
Copy link
Collaborator

Sorry, I wasn't sure exactly what you meant. "Time based" sounds like rate limiting so assumed that's what you meant.

Can you please flesh out your request? Can you offer an initial proposal of exactly how this feature would be used and how it would work? How is it different from the general rate-limiting case?

@foosinn
Copy link
Author

foosinn commented Sep 24, 2018

  • The request has just a header field with the current unix time and is with a payload-hash-sha256 singed.
  • Webhook checks if the time is unix timestamp is not older than a user configurable timestamp and verifies the payload-hash.
  • If someone would resend that request the time is not matching anymore, even if the payload-hash is still correct.

This is not intended to do rate-limiting, its more like an additional access protection. Since we are using the payload hash the password itself is not submitted in the request.

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

No branches or pull requests

3 participants