search

louislam / uptime-kuma

A fancy self-hosted monitoring tool
https://uptime.kuma.pet
MIT License
58.58k stars 5.28k forks source link

TOTP Window to small #2680

Closed julian-piehl closed 1 year ago

julian-piehl commented 1 year ago

⚠️ Please verify that this bug has NOT been raised before.

🛡️ Security Policy

Description

Smaller Window = More Secure. But not every system has the exact same timestamp. My server is nearly 1 minute in the future of my Windows pc. So I have to use the token in the first few seconds my authenticator shows it to me.

At first, I thought I locked my self out...

https://github.com/louislam/uptime-kuma/blob/d99d37898e6abc2753a3ae06e4dfff5c42e8cbd6/server/server.js#L113

I would suggest a value of 2-3. It's not too large, but should fix this problem for me (and I think others will have the same problem).

👟 Reproduction steps

Change your local or server time

👀 Expected behavior

More windows

😓 Actual Behavior

1 Window isn't enough

🐻 Uptime-Kuma Version

1.19.6

💻 Operating System and Arch

Ubuntu 20.04

🌐 Browser

Opera GX

🐋 Docker Version

Docker 20.10.22

🟩 NodeJS Version

No response

📝 Relevant log output

No response

louislam commented 1 year ago

640 It is part this pr.

Cc: @andreasbrett

julian-piehl commented 1 year ago

640 It is part this pr.

Cc: @andreasbrett

In this pr the window was set from notp default (6) to 1. I now suggest to higher it again, because 1 is too low for some systems.

The current window is +/- 1*30 seconds = 30 seconds I say it should be 2-3 so +/- 3*30 seconds = 1.5 minutes

andreasbrett commented 1 year ago

But not every system has the exact same timestamp. My server is nearly 1 minute in the future of my Windows pc.

My server and Windows machine sync their time via NTP and are in-sync within 1sec. AFAIK Windows machines do that by default. Maybe your server is not set up to NTP-sync its system time?

In this pr the window was set from notp default (6) to 1.

It was and for a reason. The linked PR states the RFC and its recommendation. I don't think uptime kuma should ignore that. Furthermore the libpam Google Authenticator module uses those +/- 30seconds and that seems to work for a much larger userbase.

The validation system should compare OTPs not only with the receiving timestamp but also the past timestamps that are within the transmission delay. A larger acceptable delay window would expose a larger window for attacks. We RECOMMEND that at most one time step is allowed as the network delay.

We RECOMMEND a default time-step size of 30 seconds. This default value of 30 seconds is selected as a balance between security and usability.

louislam commented 1 year ago

It was and for a reason. The linked PR states the RFC and its recommendation. I don't think uptime kuma should ignore that.

Good point, I almost forget that it was defined by RFC 6238. I would tent to keep the current config.

julian-piehl commented 1 year ago

My server and Windows machine sync their time via NTP and are in-sync within 1sec. AFAIK Windows machines do that by default. Maybe your server is not set up to NTP-sync its system time?

I checked again. My server isn't in the future, my Windows pc is in the past. Have to check that...

It was and for a reason. The linked PR states the RFC and its recommendation. I don't think uptime kuma should ignore that. Furthermore the libpam Google Authenticator module uses those +/- 30seconds and that seems to work for a much larger userbase.

When I encountered this problem I googled it and read, that the typical window for most applications should be 3, because the time can differ up to about 1 minute from device to device. But if the default by RFC is 1 then it should be alright... Just wanted to make sure others don't encounter this problem.

I will check what's wrong with my device and correct it. But thanks for the nice discussion :D