aidantwoods / go-paseto

Platform-Agnostic Security Tokens implementation in Golang.
https://pkg.go.dev/aidanwoods.dev/go-paseto
MIT License
282 stars 16 forks source link

Keep getting "bad signature" from parsing #7

Closed jcheng-devops closed 1 year ago

jcheng-devops commented 1 year ago

I'm doing some test locally

After my client logs in, and the backend generate signed variable and send back to client in frontend with the following code:

token := paseto.NewToken()
token.SetIssuer("test@example.com")
token.SetIssuedAt(time.Now())
token.SetNotBefore(time.Now())
token.SetExpiration(time.Now().Add(time.Minute + 30))

secretKey, err := paseto.NewV4AsymmetricSecretKeyFromBytes([]byte(<Some random 64 characters>))

publicKey := secretKey.Public().ExportHex() // DO share this one
signed := token.V4Sign(secretKey, nil)

The auth middleware I have to check if key/token valid:

pubKey, err := paseto.NewV4AsymmetricPublicKeyFromHex(publicKey)
parser := paseto.NewParser()
parsedToken, err := parser.ParseV4Public(pubKey, signedToken, nil)
if err != nil {
    log.Println("err: ", err)
    return "", errors.New(err.Error())
}

This is where I keep getting bad signature

Can anyone help me here?

One quick note: I notice two hex values are different between using

pubkey := secretKey.Public().ExportHex()

and

pubkey := hex.EncodeToString([]byte(os.Getenv("TOKEN_SYMMETRIC_PUBLIC_KEY")))

But I have tried both public keys to verify and both give me bad signature

aidantwoods commented 1 year ago

In the step:

secretKey, err := paseto.NewV4AsymmetricSecretKeyFromBytes([]byte(<Some random 64 characters>))

Are you importing a truly random set of 64 bytes, or is this a previously exported secret key that was generated?

jcheng-devops commented 1 year ago

In the step:

secretKey, err := paseto.NewV4AsymmetricSecretKeyFromBytes([]byte(<Some random 64 characters>))

Are you importing a truly random set of 64 bytes, or is this a previously exported secret key that was generated?

Oh, sorry, I should be more clear. I already have a hard-coded 64 characters in my .env file, and I just use os.Getenv()

aidantwoods commented 1 year ago

Was this a previously generated key that was exported, or is this a set of 64 bytes that you're wanting to use as a key?

jcheng-devops commented 1 year ago

Was this a previously generated key that was exported, or is this a set of 64 bytes that you're wanting to use as a key?

I'm not sure if I understand you correctly. It's a pre-generated key and it exists in .env file, it's just a string.

symmetricKey="1234567890G78PXWn9CLu7sR3PuDQn37tOBEfnS7h5cmfD7xKdnDf4TCRWYJcVD5"

Because it's a string so I convert it to byte as paseto.NewV4AsymmetricSecretKeyFromBytes([]byte(symmetricKey))

Don't worry about the key exposed here, it's not a real one

aidantwoods commented 1 year ago

The reason I ask is because an ed25519 key (at least in Go's private format) can't just be an arbitrary string of characters. The 64 bytes consist of a seed value (which is generally random) and a public key (which is mathematically related to the seed). If you pass in an arbitrary string of bytes/characters here, the public portion of the key won't be mathematically related to the seed, and so it won't be possible to validate signatures which were built on this value.

If you're wanting to pass in your own random bytes, you can use the seed constructors which accept 32 bytes to use as the seed (these will calculate the the public key/rest of the private key for you).

I'd also caution against using a string of printable characters even using the seed constructor in anything other than testing, because it'll significantly reduce the security of signatures by restricting the seed to only bytes which happen to be printable characters (instead it is better to import as hex if you need to store the key in a text file, so you can retain the full range of the passed in bytes).

Also, you've mentioned a symmetric key in the above, so just wanted to point out that we're currently discussing the asymmetric PASETO mode (which signs but does not encrypt the contents of the token). If you're actually wanting to use a symmetric key, "local" mode is suited for that (and will encrypt the token) πŸ™‚

jcheng-devops commented 1 year ago

I see. Thank you so much for the detailed explanation. I will test it out later again and keep you posted!

aidantwoods commented 1 year ago

Did you manage to get this working? I'm debating introspecting the private key to try to catch ones that embed a public key that is incorrect (which would catch this kind of issue).

jcheng-devops commented 1 year ago

Did you manage to get this working? I'm debating introspecting the private key to try to catch ones that embed a public key that is incorrect (which would catch this kind of issue).

I'm sorry I haven't tried it yet. It's been crazy busy on my end. I will try to find a time on Thursday night or the weekend. The latest would be the weekend, and I will keep you posted.

aidantwoods commented 1 year ago

No rush at all πŸ™‚

jcheng-devops commented 1 year ago

No rush at all πŸ™‚

Alright, I found some time to test it, and unfortunately, it's still not working. I listened to your suggestions. I converted the string to hex first before I do the rest of the code. Let me show you what I have tried.

hexString := "4737385058576e39434c7537735233507544516e3337744f4245666e53376835636d664437784b646e44663454435257594a6356443530465177704a694b6f4c"

secretKey, err := paseto.NewV4AsymmetricSecretKeyFromHex(hexString)
if err != nil {
    panic("secretKey length should be 64")
}
publicKey := secretKey.Public().ExportHex() // DO share this one
signed := token.V4Sign(secretKey, nil)

pubKey, err := paseto.NewV4AsymmetricPublicKeyFromHex(publicKey)
if err != nil {
    return "", errors.New("Paseto public key error")
}
parser := paseto.NewParser()
parsedToken, err := parser.ParseV4Public(pubKey, signedToken, nil)
if err != nil {
    log.Println("err: ", err)
    return "", errors.New(err.Error())
}
log.Println(string(parsedToken.ClaimsJSON()))

And I still get err: bad signature in parser.ParseV4Public() Please guide me on what I should try next :)

A side question, in your example in README, you have publicKey := secretKey.Public() // DO share this one, so after I did publicKey := secretKey.Public().ExportHex() like above, not only the signed token, but also I put the publicKey in 2 cookies and send it back to the browser, so when a user is trying to access APIs that require authMiddle, I grab the publicKey from the request in the cookie in my code and use paseto.NewV4AsymmetricPublicKeyFromHex() to get the pubKey and pass into parser.ParseV4Public(), this is the right logic to do, right?

Hopefully this makes sense. Please let me know if you have any questions!

aidantwoods commented 1 year ago

The value that you're using is still not a valid ed25519 private key πŸ˜„ The reason I suggested using hex was so that you could have a convenient string representation of the key in your file, but you still need the underlying value to be a valid key. Hope that makes sense!

If you need help generating a new valid private key, you can run this once and save the output to use as a key later:

paseto.NewV4AsymmetricSecretKey().ExportHex()

A side question, in your example in README, you have publicKey := secretKey.Public() // DO share this one, so after I did publicKey := secretKey.Public().ExportHex() like above, not only the signed token, but also I put the publicKey in 2 cookies and send it back to the browser, so when a user is trying to access APIs that require authMiddle, I grab the publicKey from the request in the cookie in my code and use paseto.NewV4AsymmetricPublicKeyFromHex() to get the pubKey and pass into parser.ParseV4Public(), this is the right logic to do, right?

Unless you need the client side to be able to inspect the values inside the token, or a party other than your server to be able to verify your token (which doesn't seem to be the case here?), I'd recommend instead using the symmetric (aka "local") PASETO mode.

For asymmetric modes, whilst the public key can be known by others, you still need the public key to be trustworthy (i.e. you get it directly from the service you expect to have signed the token, rather than consuming it at the same time as the token that you are verifying). The setup you've described would actually allow users to generate their own tokens using whatever fields they like, so long as they generate their own keypair and give you their public key.

To use an analogy: imagine that someone gave you a signed document and you wanted to make sure it was correct; instead of checking your own records to verify the signature, you asked the person that handed you this document to also tell you whether or not the signature was correct. If they wanted to trick you, they could simply say it was correct πŸ˜„ When you consume the public key (i.e. the thing that verifies the token) from the same place as you get the token, you're essentially doing this with the token.

jcheng-devops commented 1 year ago

The value that you're using is still not a valid ed25519 private key πŸ˜„ The reason I suggested using hex was so that you could have a convenient string representation of the key in your file, but you still need the underlying value to be a valid key. Hope that makes sense!

If you need help generating a new valid private key, you can run this once and save the output to use as a key later:

paseto.NewV4AsymmetricSecretKey().ExportHex()

A side question, in your example in README, you have publicKey := secretKey.Public() // DO share this one, so after I did publicKey := secretKey.Public().ExportHex() like above, not only the signed token, but also I put the publicKey in 2 cookies and send it back to the browser, so when a user is trying to access APIs that require authMiddle, I grab the publicKey from the request in the cookie in my code and use paseto.NewV4AsymmetricPublicKeyFromHex() to get the pubKey and pass into parser.ParseV4Public(), this is the right logic to do, right?

Unless you need the client side to be able to inspect the values inside the token, or a party other than your server to be able to verify your token (which doesn't seem to be the case here?), I'd recommend instead using the symmetric (aka "local") PASETO mode.

For asymmetric modes, whilst the public key can be known by others, you still need the public key to be trustworthy (i.e. you get it directly from the service you expect to have signed the token, rather than consuming it at the same time as the token that you are verifying). The setup you've described would actually allow users to generate their own tokens using whatever fields they like, so long as they generate their own keypair and give you their public key.

To use an analogy: imagine that someone gave you a signed document and you wanted to make sure it was correct; instead of checking your own records to verify the signature, you asked the person that handed you this document to also tell you whether or not the signature was correct. If they wanted to trick you, they could simply say it was correct πŸ˜„ When you consume the public key (i.e. the thing that verifies the token) from the same place as you get the token, you're essentially doing this with the token.

I finally understand what you meant haha, sorry I was confused by myself. I have tested paseto.NewV4AsymmetricSecretKey() and it works flawlessly :) I have a question. I also actually tried to use the local PASETO mode like you recommended.

key := paseto.NewV4SymmetricKey()
localSignedToken := token.V4Encrypt(key, nil)

And I sent the localSignedToken back to the client, how can I verify the token in my backend server when any user sends it back? I don't find any function that is something like V4Decrypt(), nor in the README. Could you guide me a bit?

aidantwoods commented 1 year ago

The function you're looking for is ParseV4Local on the parser πŸ™‚

I named the methods Parse... to keep it a little general, since each method will do the appropriate cryptographic operation (decrypt/verify) as well as rule validation on the claims for any supplied rules.

Possibly these names are now over generalised though, and might help if I renamed these to DecryptV4Local, VerifyV4Public etc...

jcheng-devops commented 1 year ago

The function you're looking for is ParseV4Local on the parser πŸ™‚

I named the methods Parse... to keep it a little general, since each method will do the appropriate cryptographic operation (decrypt/verify) as well as rule validation on the claims for any supplied rules.

Possibly these names are now over generalised though, and might help if I renamed these to DecryptV4Local, VerifyV4Public etc...

Got it got it. However, I have a bit question about the arguments passing ParseV4Local(); after we generate a new symmetric key, encrypt it with V4Encrypt(), and send the token to the customer. And then when the customer sends back the v4.local.xxxxx... token, it's a string type. And the function has 3 arguments as ParseV4Local(key paseto.V4SymmetricKey, tainted string, implicit []byte), I assume the token should be in the 2nd argument as tainted string, right? What about the first argument?

Sorry if I bring lots of questions, I'm still learning this new encrypted solution.

aidantwoods commented 1 year ago

Yes, the token should be the string tained argument. The first should be the same symmetric key you used to encrypt the token (you can import it again in the verification flow).

Sorry if I bring lots of questions, I'm still learning this new encrypted solution.

No problem at all, it is useful for me to understand where there may be gaps in my documentation πŸ™‚

jcheng-devops commented 1 year ago

you can import it again in the verification flow

Hmmmm, interesting, doesn't that mean I will have to store the symmetric key into the database first? And when it needs to verify, I will have to query the key corresponding to the user in the database?

No problem at all, it is useful for me to understand where there may be gaps in my documentation Thank you for your patience :)

Once I figure this out, I will have think a logic to do refresh token

aidantwoods commented 1 year ago

Hmmmm, interesting, doesn't that mean I will have to store the symmetric key into the database first? And when it needs to verify, I will have to query the key corresponding to the user in the database?

You'd need to store the symmetric key you use, yes, but this is true of both modes. The difference between the symmetric vs. asymmetric approach is that you only need to deal with one type of key in the symmetric approach (the same key is used to mint new tokens as is to verify them), whereas asymmetric you have one key to mint and another to verify.

The key doesn't necessarily need to be per user, it could be set at the application level instead (e.g. set it through an environment variable). Equally you could go so far as to generate a new key for each token you issue, so long as you have a means to store that key and a sensible way to look it up when it comes to verify the token; a key per user falls somewhere in the middle of these extremes. How you set this up has different security and operational trade-offs, and depending on your use case you may decide a different option is the better suited πŸ™‚

jcheng-devops commented 1 year ago

Hmmmm, interesting, doesn't that mean I will have to store the symmetric key into the database first? And when it needs to verify, I will have to query the key corresponding to the user in the database?

You'd need to store the symmetric key you use, yes, but this is true of both modes. The difference between the symmetric vs. asymmetric approach is that you only need to deal with one type of key in the symmetric approach (the same key is used to mint new tokens as is to verify them), whereas asymmetric you have one key to mint and another to verify.

The key doesn't necessarily need to be per user, it could be set at the application level instead (e.g. set it through an environment variable). Equally you could go so far as to generate a new key for each token you issue, so long as you have a means to store that key and a sensible way to look it up when it comes to verify the token; a key per user falls somewhere in the middle of these extremes. How you set this up has different security and operational trade-offs, and depending on your use case you may decide a different option is the better suited πŸ™‚

I figured out what you meant but I do have a quick question. I'm doing the key as an environment variable as you suggested, but wouldn't another user overwrite the value when the user signs in? Unless the run-time wouldn't affect the environment variable, but I find it hard to believe. I haven't had a chance to test it, but maybe you have some insight

aidantwoods commented 1 year ago

I figured out what you meant but I do have a quick question. I'm doing the key as an environment variable as you suggested, but wouldn't another user overwrite the value when the user signs in?

The decision to make here is how many keys you want to manage operationally. If using an environment variable, the approach would be to generate a key once for the app to use, and then to keep using that same key both to mint new tokens and to verify incoming ones.

If you wanted more granularity (such as a key per user), then you'd need a different approach to storing the keys than an environment variable (such as using a KMS or DB).

More granular keys can offer advantages (e.g. if you had a key per user, you'd be able to revoke all tokens for a user by deleting and regenerating their key), but are operationally it can be more difficult to manage than a single key.

jcheng-devops commented 1 year ago

I figured out what you meant but I do have a quick question. I'm doing the key as an environment variable as you suggested, but wouldn't another user overwrite the value when the user signs in?

The decision to make here is how many keys you want to manage operationally. If using an environment variable, the approach would be to generate a key once for the app to use, and then to keep using that same key both to mint new tokens and to verify incoming ones.

If you wanted more granularity (such as a key per user), then you'd need a different approach to storing the keys than an environment variable (such as using a KMS or DB).

More granular keys can offer advantages (e.g. if you had a key per user, you'd be able to revoke all tokens for a user by deleting and regenerating their key), but are operationally it can be more difficult to manage than a single key.

Thank you very much!! I have decided to use the granularity way (key for each user in DB) because later on, I will need a refresh token from the DB too (I have some idea, but not sure if it will work). I have learned so much from you, really appreciated it!! If you need me to close the question, please let me know. I might need to ask you about refresh token later ;P

aidantwoods commented 1 year ago

Happy to help! πŸ™‚ I'll convert this issue to a discussion, so feel free to add on if you have anything else