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

JWT compares client clock with server clock - design error #1698

Closed
benbucksch opened this issue Dec 11, 2015 · 15 comments
Closed

JWT compares client clock with server clock - design error #1698

benbucksch opened this issue Dec 11, 2015 · 15 comments

Comments

@benbucksch
Copy link

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.

@wagnerand
Copy link
Member

This is a fatal misdesign of the protocol. You simply cannot compare client clock with server clocks.

To be fair, most multi-factor authentication systems rely on TOTP (time-based!)

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.

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.

@benbucksch
Copy link
Author

You can add multiple owners/developers (accounts) to your add-on

I didn't know, thank you, we'll do that.

most multi-factor authentication systems rely on TOTP (time-based!)

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.
http://bofh.ntk.net/BOFH/0000/bastard04.php

"Now I'm in a REALLY good mood. I think I just might write that script to make saving impossible on rogue at random times like I've been thinking about" ...
"the earth's magnetic field is particularly strong today."

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

@kumar303
Copy link
Contributor

You simply cannot compare client clock with server clocks.

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.

@benbucksch
Copy link
Author

The client can just sync their clock with NTP as stated in Olympia's API docs.

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.

JWTs (and many other signature based authentication) will need to rely on timestamps to prevent replay attacks.

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.

@kumar303
Copy link
Contributor

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.)

Theoretical concerns aside, JWT is completely unnecessary here at AMO.

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.

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.

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.

@benbucksch
Copy link
Author

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.

Theoretical concerns aside, JWT is completely unnecessary here at AMO.

JWTs ... 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.

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.

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?

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.

@kumar303
Copy link
Contributor

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.

there's no widespread standard protocol...that does this

SSL/TLS certs rely on client clocks for certificate validation (same concept, same problem).

If you don't trust SSL with a password

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.

It took me 2 days (!) to find the problem.

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.

@benbucksch
Copy link
Author

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.

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.

I think we can return a more informative authentication failure message.

Yup. That's #1697

SSL/TLS certs rely on client clocks for certificate validation (same concept, same problem).

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.

I think another problem here is that our clock skew allowance is too short, which we can fix.

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.

@wagnerand
Copy link
Member

most multi-factor authentication systems rely on TOTP (time-based!)

Not on the client clock, no.

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.

@kumar303
Copy link
Contributor

The problem here is that you're enforcing the client time on a second precision

It allows for 5 seconds of drift currently but I filed https://github.com/mozilla/olympia/issues/1498 to give it more leeway.

@kumar303
Copy link
Contributor

Until the leeway is added, the error message should be more obvious now (see mozilla/addons-server#1514)

@benbucksch
Copy link
Author

What is preventing you from syncing your clock using NTP or something similar? (time.apple.com on Mac, etc.)

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:
http://changelogs.ubuntu.com/changelogs/pool/main/n/ntp/ntp_4.2.6.p3+dfsg-1ubuntu3.8/changelog
20 security bugs in 2015 alone, including 4 critical ones. ntp and friends run as root.
This is why I don't do ntp syncs. They are a security risk, just for setting the clock. Sad but true

@kumar303
Copy link
Contributor

kumar303 commented Feb 4, 2016

Oh, huh, good point. I hadn't looked into this. Actually, I see that Hanno in our services group recommends to use tlsdate.

@benbucksch
Copy link
Author

Interesting, thanks.

@kumar303
Copy link
Contributor

kumar303 commented Feb 4, 2016

Thanks for this tip. I adjusted our exception message to no longer recommend NTP :) mozilla/addons-server#1617

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

No branches or pull requests

3 participants