-
Notifications
You must be signed in to change notification settings - Fork 41
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
JWT compares client clock with server clock - design error #1698
Comments
To be fair, most multi-factor authentication systems rely on TOTP (time-based!)
You can add multiple owners/developers (accounts) to your add-on. I am fairly sure this would enable you to have multiple users being able to use the signing API for your add-on. |
I didn't know, thank you, we'll do that.
Not on the client clock, no. OAuth generates a token on the server, and that token includes the server time as expiry, so the server compares server to server time. JWT is the first I ever see where the client generates the token with expiry itself. Most protocols deliberately avoid comparing server to client time, for exactly this reason. I debugged this for hours during 2 days. I would never have thought the clock would influence my code. This made me remember the "Bastard Operator From Hell mozilla/addons-server#4" story that I read a decades ago.
This bug here was the closest to this BOFH story I ever got in my engineering life. :-) The clock drift... FWIW, I checked the spec, the "iat" is supposed to be the "time of issue", so a clock drifting in the fast direction is bound to cause errors. This is really a mis-designed spec. Author Microsoft, BTW |
Sure you can. The client can just sync their clock with NTP as stated in Olympia's API docs. JWTs (and many other signature based authentication) will need to rely on timestamps to prevent replay attacks. I'm closing this because NTP syncing should be sufficient to deal with this. |
No, it cannot. Only root can change the clock. The JWT client cannot do that. You cannot assume that all people and all computers have perfect clocks, or all have NTP sync enabled. That's a ridiculous assumption.
As I said above, other protocols compare server time with server time. No widespread protocol ever compares client time with server time. If any of them did, they ran into the same problem as here and were fixed. |
I think it's reasonable to expect that a client who calls our API has a clock that is in sync with the world. Pretty much all APIs rely on a synchronized client clock. Oauth 1.0, for example, requires your clock to be in sync. A quick search shows that Twitter's API requires the client's clock to be in sync, just as an example. In the story of your original bug report you mention that the problem was your computer. What is preventing you from syncing your clock using NTP or something similar? (time.apple.com on Mac, etc.)
JWTs provide enhanced security for API access because instead of passing a token that might be intercepted and re-used by an attacker (like a password or a session cookie), you only pass a signature, which cannot be re-used. Oauth 1.0 works just the same and is the authentication mechanism for many APIs out there. JWT is a little simpler than Oauth.
You shouldn't have to share your API credentials with anyone. AMO supports team members for add-ons (go to Manage Authors and License). For each assigned team member, their personal API credentials should allow them to push updates to the shared add-on. Please open a bug if that isn't working. |
I am just implementing OAuth in a project, and OAuth 2.0 definitely uses server clock vs. server clock, if the token considers clock at all. How the token is generated and verified is entirely the server's problem. There isn't even any way to consider the client clock in the protocol. I never claimed that there are no APIs that do this. I'm sure there are plenty of web devs that did things like that. I specifically said there's no widespread standard protocol - meaning HTTP, IMAP, SMTP or similar - that does this, and intentionally so, for good reason, namely experience. To take just one example, HTTP cache headers do not send the client time, although that would be an obvious choice. Rather, they take an ETag from the server and echo back the same ETag. That's a little bit more effort, but crucial to make things work reliably. The protocol is specifically designed to avoid comparing client with server time.
I've covered that in my initial report. If you don't trust SSL with a password - even a dedicated password for this upload purpose only -, then you have a problem in many other places, e.g. the developer login on AMO with password.
It took me 2 days (!) to find the problem. The purpose of the bug report is that I want to prevent that somebody else falls in the same trap and having to suffer as I did. |
Oauth 2.0 can still suffer from client clock skew. Maybe we're talking about different things? When I say "client clock skew" I mean the clock of the server that the client is connecting to our API from.
SSL/TLS certs rely on client clocks for certificate validation (same concept, same problem).
It's a lot harder to trust SSL/TLS when the client is not a web browser. For example, a lot of standard Python libs will make requests without checking certs by default. This is generally why APIs typically don't let you connect simply with a username and password. Using JWTs really isn't about a mistrust of SSL though. After some kind of authentication (even if it were presenting a username + password) we would need a way to maintain a token for the session. If we use a signature approach (which requires sync'd clocks) then we don't have to maintain a local database of valid tokens -- we can just check the signature.
Yeah, that sucks. I think we can return a more informative authentication failure message. I think another problem here is that our clock skew allowance is too short, which we can fix. |
The issue with JWT is that you take the absolute client time (e.g. "2016-01-20 20:12:23") and compare it with the absolute server time. That must go wrong, as soon as client and server disagree about what time is it, which will happen. That's the conceptual problem that this bug is about. OAuth 2.0 doesn't suffer from that. As implied in the doc that you quoted, it will never compare client time with server time, but only client time with client time and server time with server time, e.g. for expiry. That's fairly sane and a lot safer. Even that can fail (even though much less likely), and the doc you mentioned shows how to deal with that (i.e. handle the expiry error code). In other words, that doc is in line with what I was saying.
Yup. That's #1697
OK, but there, we're in the year or month range, at most days. We're talking date here, not time of day. My clock was 90 seconds fast. When I was in vacation and came back, my computer's clock was 20 minutes fast (which I quickly noticed manually and corrected). That's still fine for SSL purposes.
Exactly. Thank you. The problem here is that you're enforcing the client time on a second precision. As a JWT client, I am not allowed to set iat even 1 second in the future, or your server will bail. So a clock being only 2 seconds (!) fast will make things fail. If you're using client time at all, you should allow it to be 1 hour fast or late. |
Google's TOTP does that, for example. I have a private server which uses that mechanism and its clock was off by a minute or so. It did not accept any of the keys generated by the Google Authenticator app until I corrected the time on the server. |
It allows for 5 seconds of drift currently but I filed https://github.com/mozilla/olympia/issues/1498 to give it more leeway. |
Until the leeway is added, the error message should be more obvious now (see mozilla/addons-server#1514) |
Security. I just got another security update for ntp. The security bugs in ntp just don't stop. Take a look at this, how many in a year: |
Oh, huh, good point. I hadn't looked into this. Actually, I see that Hanno in our services group recommends to use tlsdate. |
Interesting, thanks. |
Thanks for this tip. I adjusted our exception message to no longer recommend NTP :) mozilla/addons-server#1617 |
xref issue #1697 is the hot fix
This bug here tries to get to the root of the problem.
Story:
We wrote a script to upload our builds to AMO as part of our build process.
The code was working for Neil, who wrote the script, but I just got "Invalid authentication credentials".
We were debugging it for hours and hours, suspecting shell argument passing and parsing, OS differences and what not. We compared credentials and confirmed they are identical. Still, the result was different.
During debugging, once, by coincidence, it worked. At a time when it should be expired.
What was the reason? My clock on my computer was fast.
I was screaming. Client server protocols must NEVER EVER compare client clocks with server clocks, because clocks are very often off. Even if it was originally set exactly right by the second, clocks always have a drift in one way or another. If you have 5 million users, you will surely have 500000 whose clocks are off by 1 minute, and 50000 users whose clocks are off more than that.
In my case, my clock was 95 seconds fast. That's not uncommon. I don't run ntpdate regularly, because ntp regularly has security problems, too.
This is a fatal misdesign of the protocol. You simply cannot compare client clock with server clocks. That something that engineers generally know from experience. Often hard lessons, like mine above. Sometimes worse lessons, when 5 million people were hit by this problem.
If this problem is inherent in the JWT protocol, then it was mis-designed and you can't use JWT, simple as that.
Please note that an RFC doesn't make it a standard. It's merely a proposal. RFC means "Request for comment". My experience shows that JWT is not suited for deployment, at all. Please replace it (please read on, alternative below).
Theoretical concerns aside, JWT is completely unnecessary here at AMO. For the AMO website, I have one single AMO account password, and because we're a team working on that extension, I need to share this password with several people in the organization. That's where your security weakness is.
Separating the scripts from the main password, what you tried to do here in JWT, was a wise move. Putting the main AMO password in build scripts would be dead stupid. However, instead of letting me generate JWT credentials on AMO, you could simply let me generate a second password which is specifically for the upload API, and works only there. Then let me send that using standard HTTP Auth when I upload builds.
Security-wise, that's equivalent. Whether I store a JWT user and JWT secret or a username and password is strictly same, it's just a different term for the same thing. Given that JWT can generate tokens from the user/secret at any time, it doesn't add any security. All JWT protects is the wire data, so it helps only when you mistrust HTTPS - but then you have the same trouble with my main AMO login in the browser, so it didn't help.
The advantage of using standard HTTP Auth is that it's a standard (and actual standard) and implemented in almost all HTTP clients. I wouldn't need funny python modules and import them into my build system. Security-wise, it would be the same.
The text was updated successfully, but these errors were encountered: