Skip to content

fix: allow 5 mins time drift #65

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.

Sign up for GitHub

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

Merged
levenleven merged 1 commit into from
Jul 2, 2024
Merged

fix: allow 5 mins time drift #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/chatops.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ module Chatops
threaded_and_channel: 2,
}.freeze

ALLOWED_TIME_SKEW_MINS = 5

def self.public_key
ENV[public_key_env_var_name]
end
Expand Down
4 changes: 2 additions & 2 deletions lib/chatops/controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,8 @@ def ensure_valid_chatops_signature
def ensure_valid_chatops_timestamp
@chatops_timestamp = request.headers["Chatops-Timestamp"]
time = Time.iso8601(@chatops_timestamp)
if !(time > 1.minute.ago && time < 1.minute.from_now)
return jsonrpc_error(-32803, 403, "Chatops timestamp not within 1 minute of server time: #{@chatops_timestamp} vs #{Time.now.utc.iso8601}")
if !(time > Chatops::ALLOWED_TIME_SKEW_MINS.minute.ago && time < Chatops::ALLOWED_TIME_SKEW_MINS.minute.from_now)
return jsonrpc_error(-32803, 403, "Chatops timestamp not within #{Chatops::ALLOWED_TIME_SKEW_MINS} minutes of server time: #{@chatops_timestamp} vs #{Time.now.utc.iso8601}")
end
rescue ArgumentError, TypeError
# time parsing or missing can raise these
Expand Down
2 changes: 1 addition & 1 deletion lib/chatops/controller/version.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module ChatopsController
VERSION = "5.2.0"
VERSION = "5.3.0"
end
12 changes: 6 additions & 6 deletions spec/lib/chatops/controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,9 @@ def rails_flexible_post(path, outer_params, jsonrpc_params = nil)
expect(response.status).to eq 403
end

it "doesn't allow requests more than 1 minute old" do
it "doesn't allow requests more than 5 minute old" do
nonce = SecureRandom.hex(20)
timestamp = 2.minutes.ago.utc.iso8601
timestamp = 6.minutes.ago.utc.iso8601
request.headers["Chatops-Nonce"] = nonce
request.headers["Chatops-Timestamp"] = timestamp
digest = OpenSSL::Digest::SHA256.new
Expand All @@ -188,12 +188,12 @@ def rails_flexible_post(path, outer_params, jsonrpc_params = nil)
request.headers["Chatops-Signature"] = "Signature keyid=foo,signature=#{signature}"
get :list
expect(response.status).to eq 403
expect(response.body).to include "Chatops timestamp not within 1 minute"
expect(response.body).to include "Chatops timestamp not within 5 minutes"
end

it "doesn't allow requests more than 1 minute in the future" do
it "doesn't allow requests more than 5 minute in the future" do
nonce = SecureRandom.hex(20)
timestamp = 2.minutes.from_now.utc.iso8601
timestamp = 6.minutes.from_now.utc.iso8601
request.headers["Chatops-Nonce"] = nonce
request.headers["Chatops-Timestamp"] = timestamp
digest = OpenSSL::Digest::SHA256.new
Expand All @@ -202,7 +202,7 @@ def rails_flexible_post(path, outer_params, jsonrpc_params = nil)
request.headers["Chatops-Signature"] = "Signature keyid=foo,signature=#{signature}"
get :list
expect(response.status).to eq 403
expect(response.body).to include "Chatops timestamp not within 1 minute"
expect(response.body).to include "Chatops timestamp not within 5 minutes"
end

it "does not add authentication to non-chatops routes" do
Expand Down