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

Code comment for jwt.ParseRSAPublicKeyFromPEM incorrect #119

Closed
twocs opened this issue Nov 4, 2021 · 13 comments
Closed

Code comment for jwt.ParseRSAPublicKeyFromPEM incorrect #119

twocs opened this issue Nov 4, 2021 · 13 comments

Comments

@twocs
Copy link
Contributor

twocs commented Nov 4, 2021

The code comment for rsa_utils.go says it's for a PKCS1 or PKCS8 public key (ParseRSAPublicKeyFromPEM parses a PEM encoded PKCS1 or PKCS8 public key ParseRSAPublicKeyFromPEM parses a PEM encoded PKCS1 or PKCS8 public key), but the function uses x509.ParsePKIXPublicKey instead. I was unable to get a PKCS1 public key despite the code comment, and basically just copied the entire function and swapped it from x509.ParsePKIXPublicKey to x509.ParsePKCS1PublicKey, which works. I am not clear but it seems that it's not matching the code comment and it took me quite some time to figure out why I couldn't parse my RSA Public Key (it wasn't accepted on jwt.io).

I suspect that the problem is not with the code comment, but with the implementation. Or am I doing something wrong that the parsing of a pem with PKCS1 encoding doesn't work using x509.ParsePKIXPublicKey but does work with x509.ParsePKCS1PublicKey?

func ParseRSAPublicKeyFromPEM(key []byte) (*rsa.PublicKey, error) {

// ParseRSAPublicKeyFromPEM parses a PEM encoded PKCS1 or PKCS8 public key
func ParseRSAPublicKeyFromPEM(key []byte) (*rsa.PublicKey, error) {
	var err error

	// Parse PEM block
	var block *pem.Block
	if block, _ = pem.Decode(key); block == nil {
		return nil, ErrKeyMustBePEMEncoded
	}

	// Parse the key
	var parsedKey interface{}
	if parsedKey, err = x509.ParsePKIXPublicKey(block.Bytes); err != nil {
		if cert, err := x509.ParseCertificate(block.Bytes); err == nil {
			parsedKey = cert.PublicKey
		} else {
			return nil, err
		}
	}

If we compare to

func ParseRSAPrivateKeyFromPEM(key []byte) (*rsa.PrivateKey, error) {

private key explicitly tries both functions.

	if parsedKey, err = x509.ParsePKCS1PrivateKey(block.Bytes); err != nil {
		if parsedKey, err = x509.ParsePKCS8PrivateKey(block.Bytes); err != nil {
			return nil, err
		}
	}
@oxisto
Copy link
Collaborator

oxisto commented Nov 5, 2021

I think the comment of this function is wrong. If you look into the implementation of ParsePKIXPublicKey it actually refers to ParsePKCS1PublicKey if it fails to parse the key.

https://cs.opensource.google/go/go/+/refs/tags/go1.17.3:src/crypto/x509/x509.go;drc=ec4efa42089415b0427a4d30b317cfd7e4a0fe75;l=58

So if you want to support both, we probably need to do something similar to ParseRSAPrivateKeyFromPEM.

Would you be interesting in supplying a PR to support this?

@twocs
Copy link
Contributor Author

twocs commented Nov 5, 2021

Thanks for the feedback. I'll submit a PR.

@mfridman
Copy link
Member

mfridman commented Nov 5, 2021

What if we leveraged this crypto/ssh.ParseRawPrivateKey function for parsing a private key? Which does the pem decoding and then switches on the type.

https://pkg.go.dev/golang.org/x/crypto/ssh#ParseRawPrivateKey

Source code: https://cs.opensource.google/go/x/crypto/+/089bfa56:ssh/keys.go;l=1117-1145

@oxisto
Copy link
Collaborator

oxisto commented Nov 5, 2021

What if we leveraged this crypto/ssh.ParseRawPrivateKey function for parsing a private key? Which does the pem decoding and then switches on the type.

https://pkg.go.dev/golang.org/x/crypto/ssh#ParseRawPrivateKey

Source code: https://cs.opensource.google/go/x/crypto/+/089bfa56:ssh/keys.go;l=1117-1145

It's the public key parsing we need :)

@mfridman
Copy link
Member

mfridman commented Nov 5, 2021

What if we leveraged this crypto/ssh.ParseRawPrivateKey function for parsing a private key? Which does the pem decoding and then switches on the type.
https://pkg.go.dev/golang.org/x/crypto/ssh#ParseRawPrivateKey
Source code: https://cs.opensource.google/go/x/crypto/+/089bfa56:ssh/keys.go;l=1117-1145

It's the public key parsing we need :)

Ugh, apologies, should have read the issue a bit closer. 😊

twocs added a commit to twocs/jwt that referenced this issue Nov 6, 2021
@twocs
Copy link
Contributor Author

twocs commented Nov 6, 2021

The test of jwt.ParseRSAPublicKeyFromPEM uses file https://github.com/golang-jwt/jwt/blob/main/test/sample_key.pub

Switching to x509.ParsePKCS1PublicKey results in the following failure:
2021/11/06 20:49:32 x509: failed to parse public key (use ParsePKIXPublicKey instead for this key format)

Looks like the unit test relies on PKIX public key so need to fix both the comment and add PKCS1 support.

@lknite
Copy link

lknite commented Mar 10, 2024

I was lucky to have found sample code for my use case, keycloak, here.

Without that, odds are I'd still be trying to get things working. (not enough comments to understand for a new user)

When I post a token at jwt.io it says everything is good to go, signature verified, but this library seems to require a key (certificate). Why is it jwt.io is ok with verifying the signature but this library is unable to without a key? Is this key just the standard onprem ca root certificate?

@oxisto
Copy link
Collaborator

oxisto commented Mar 11, 2024

Closed by #120

@oxisto oxisto closed this as completed Mar 11, 2024
@oxisto
Copy link
Collaborator

oxisto commented Mar 11, 2024

I was lucky to have found sample code for my use case, keycloak, here.

Without that, odds are I'd still be trying to get things working. (not enough comments to understand for a new user)

When I post a token at jwt.io it says everything is good to go, signature verified, but this library seems to require a key (certificate). Why is it jwt.io is ok with verifying the signature but this library is unable to without a key? Is this key just the standard onprem ca root certificate?

We are currently working on a documentation site: https://golang-jwt.github.io/jwt/usage/create/.

@twocs
Copy link
Contributor Author

twocs commented Mar 13, 2024

I was lucky to have found sample code for my use case, keycloak, here.

Without that, odds are I'd still be trying to get things working. (not enough comments to understand for a new user)

When I post a token at jwt.io it says everything is good to go, signature verified, but this library seems to require a key (certificate). Why is it jwt.io is ok with verifying the signature but this library is unable to without a key? Is this key just the standard onprem ca root certificate?
#119 (comment)

Would be great to have a full example of best practices for that in the docs. However, JWT supports a lot of encryption algorithms, and there are also a number of potential security vulnerabilities (99% of OWASP JWT for Java is applicable to this library). In my experience, JWT is deceptively complicated for something so simple: cryptographically signed claims.

Actually jwt.io is also looking for the keys: on https://jwt.io and set the Algorithm to RS256 (matches your Keycloak config) then look at the Verify Signature box (in Decoded section). It has two fields, the PUBLIC KEY and the PRIVATE KEY. You should not use the default signature fields from https://jwt.io, you should generate your own. I have found that certificates can be tricky, because they can be encoded (e.g. as a PEM). The jwt function jwt.ParseRSAPublicKeyFromPEM can be used to convert a .pem file into the RSA Public Key that's then used for the decoding.

As for certificates, I am not sure exactly if there's a one-size-fits-all solution. Either you have a certificate already and want to use it for JWT signing (maybe your ca root certificate). Or you can generate one, e.g. openssl genrsa -out jwtRSA256-private.pem 2048 in the terminal. Or in golang, we can leverage libraries such as "crypto/rsa", "crypto/rand" and "crypto/x509"; for example, to generate a private key:

privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
publicKey = privateKey.PublicKey

@nicolascampbell
Copy link

nicolascampbell commented May 12, 2024

I was lucky to have found sample code for my use case, keycloak, here.
Without that, odds are I'd still be trying to get things working. (not enough comments to understand for a new user)
When I post a token at jwt.io it says everything is good to go, signature verified, but this library seems to require a key (certificate). Why is it jwt.io is ok with verifying the signature but this library is unable to without a key? Is this key just the standard onprem ca root certificate?
#119 (comment)

Would be great to have a full example of best practices for that in the docs. However, JWT supports a lot of encryption algorithms, and there are also a number of potential security vulnerabilities (99% of OWASP JWT for Java is applicable to this library). In my experience, JWT is deceptively complicated for something so simple: cryptographically signed claims.

Actually jwt.io is also looking for the keys: on https://jwt.io and set the Algorithm to RS256 (matches your Keycloak config) then look at the Verify Signature box (in Decoded section). It has two fields, the PUBLIC KEY and the PRIVATE KEY. You should not use the default signature fields from https://jwt.io, you should generate your own. I have found that certificates can be tricky, because they can be encoded (e.g. as a PEM). The jwt function jwt.ParseRSAPublicKeyFromPEM can be used to convert a .pem file into the RSA Public Key that's then used for the decoding.

As for certificates, I am not sure exactly if there's a one-size-fits-all solution. Either you have a certificate already and want to use it for JWT signing (maybe your ca root certificate). Or you can generate one, e.g. openssl genrsa -out jwtRSA256-private.pem 2048 in the terminal. Or in golang, we can leverage libraries such as "crypto/rsa", "crypto/rand" and "crypto/x509"; for example, to generate a private key:

privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
publicKey = privateKey.PublicKey

I found this super useful. Had problems generating a key pair that was accepted. I was trying with ssh-keygen (with key format pem -m pem) and I couldnt make it work.
Maybe would be nice to have a "How to generate the signing keys?" in the README

@mfridman
Copy link
Member

If we were to add "guides" or "how to" section to the documentation website, would that help?

https://golang-jwt.github.io/jwt/

@nicolascampbell
Copy link

For sure.

Maybe something like this?

Generating signing keys

To generate a key pair that will be properly parsed by ParseRSAPublicKeyFromPEM/ParseRSAPrivateKeyFromPEM you can create them with openssl.
This is an example with RSA but you can change the algorithm used.

# create private key
openssl genrsa -out signing-private.pem 2048
# extract public key from private key
openssl rsa -in signing-private.pem -outform PEM -pubout -out signing-public.pem

If you want to use them in your code (remember to choose the correct alg):

key, _ := os.ReadFile("your-path/signing-public.pem")
var rsaKey *rsa.PublicKey
if rsaKey, err = jwt.ParseRSAPublicKeyFromPEM(key); err != nil {
fmt.Printf("Unable to parse RSA public key: %v\n", err)
}

Now that I am here this page could also do with an example:
https://golang-jwt.github.io/jwt/usage/parse/

key, _ := os.ReadFile("your-path/signing-public.pem")
var rsaKey *rsa.PublicKey
if rsaKey, err = jwt.ParseRSAPublicKeyFromPEM(key); err != nil {
fmt.Printf("Unable to parse RSA public key: %v\n", err)
}
token, err := jwt.Parse(tokenString, func(token *jwt.Token) (interface{}, error) {
  // Don't forget to validate the alg is what you expect:
  if _, ok := token.Method.(*jwt.SigningMethodRSA); !ok {
    return nil, fmt.Errorf("Unexpected signing method: %v", token.Header["alg"])
  }
 // You return the already parsed RSA key to the KeyFunc. It will use that key to check the signature
  return rsaKey, nil
})

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

5 participants