Skip to content
This repository has been archived by the owner on May 21, 2022. It is now read-only.

Carry bug in crypto/elliptic may affect this library #216

Closed
kevinburke opened this issue May 24, 2017 · 5 comments
Closed

Carry bug in crypto/elliptic may affect this library #216

kevinburke opened this issue May 24, 2017 · 5 comments

Comments

@kevinburke
Copy link
Contributor

kevinburke commented May 24, 2017

A description of the problem may be found here: golang/go#20040 (comment)

The comments on the issue suggest that Go 1.7 below 1.7.6 and Go 1.8 below 1.8.2 are vulnerable. I'm not sure of the details; it sounds like you need to be a professional cryptographer to evaluate them, which is an argument for reducing complexity in libraries that implement those primitives. Per Chris Broadfoot (the Go release director):

If you're using the elliptic package directly, such as working with JWTs, then you probably want to update. If you're not, then wait for Go 1.8.3, which should be released tomorrow.

This library imports crypto/ecdsa, which imports crypto/elliptic.

I've addressed this in my fork as follows: kevinburke#1

@dlsniper
Copy link

@kevinburke so the bug is in Go but you want a professional cryptographer to evaluate this library but not Go?

@kevinburke
Copy link
Contributor Author

kevinburke commented May 25, 2017

I don't really follow... Two of the people in the discussion at the link are professional cryptographers. They were asked about the impact, and concluded "TLS as implemented by the Go standard library is not really exploitable," but "If you're using the elliptic package directly, such as working with JWTs, then you probably want to update [to 1.8.2]."

This package uses crypto/elliptic via crypto/ecdsa, which seems to suggest this library could be vulnerable. I'm unsure about that conclusion, though, and it sounds like you'd need to be familiar with the details of elliptic cryptography to figure it out for certain. Still, it seems like end users would want to care if their elliptic curve JWT could be trivially bypassed.

Going on the attack when I point out reasonable issues is a little odd.

@dlsniper
Copy link

I'm sorry, I didn't mean to sound like an attack, but read the issue yourself:

  • there's an issue in Go 1.8.1 and below
  • users are highly encouraged to upgrade to 1.8.2+
  • based on the fix the library, there's nothing that the user of the library needs to do in order to fix this
  • the solution suggested is to remove functionality from the library or to hire a professional cryptographer to evaluate a bug in Go (or the crypto/* packages as far as I can understand)
  • there's a link to a solution who's saying: I've solved this issue by not touching the package that was patched in Go at all
  • the quote in the second message says:
"If you're using the elliptic package directly, such as working with JWTs, then you probably want to update [to 1.8.2]."

I know you are totally against JWT and trying to promote this library as being a bad one and the whole standard as being bad, but leaving that aside, who's attacking what now?

@kevinburke
Copy link
Contributor Author

there's nothing that the user of the library needs to do in order to fix this

Right, I guess reading the comments on the issue, people who are using this library in particular should hurry to update to Go 1.8.2. It seems like adding an issue to the library is a good way to notify people they should update.

the solution suggested is to remove functionality from the library or to hire a professional cryptographer to evaluate a bug in Go

I was trying to suggest that trying to figure out whether this library is exploitable using the crypto/elliptic error is beyond the average library user's capability. I might not have written very clearly on that point.

Not being able to determine whether your library is vulnerable or not is an argument for two things:

  • asking people to upgrade to a Go version that isn't exploitable, no matter what
  • using libraries that are less complex and have fewer dependencies

Both seem reasonable. In the previous description, I didn't say anything good or bad about JWT.

@dgrijalva
Copy link
Owner

I added a comment in the README that mentions this.

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

No branches or pull requests

3 participants