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

Illegal token detection before jose.Decode() #4

Closed
pandex opened this issue Aug 5, 2015 · 11 comments
Closed

Illegal token detection before jose.Decode() #4

pandex opened this issue Aug 5, 2015 · 11 comments

Comments

@pandex
Copy link

pandex commented Aug 5, 2015

Could I get a quick confirmation that in

token, err := jose.Encrypt(someString, jose.RSA_OAEP, jose.A256GCM, publicKey)

token is string and NOT base64?

I'm trying to see if there's a quick way to pre-disqualify a potentially malformed token BEFORE having to pay the price of doing

 j, err := jose.Decode(token, privateKey)

like you can with a base64 coded string:

 _, err := base64.StdEncoding.DecodeString(tokenString) 

for space/illegal chars/length=multiple of 4/etc.

Any ideas?

@dvsekhvalnov
Copy link
Owner

Hi @pandex ,

  1. yes, you never deal with compact serialization itself with jose2go. It takes your plain payload as string (can be json or whatever else). And returns always plain payload back.
  2. I don't think you should care about valid base64 strings, if base64.StdEncoding fails within jose.Decode(...) call it will panic before attempting any further actions (now i think we probably can reconsider to return err instead, unsure why i did panic initially).
  3. Also you may be interested in two-phase validation, checkout docs: https://github.com/dvsekhvalnov/jose2go#two-phase-validation if you want to examine headers/content before actual decoding. And/or use dynamic verification keys.

@pandex
Copy link
Author

pandex commented Aug 5, 2015

In what I am doing, all operations happen in microseconds or low double-digit milliseconds, except token decoding, which can take up to triple-digit milliseconds.

So the idea is to avoid it by doing a near cost-free token length, presence of space, etc. check prior to decode. No point decoding a malformed token string, which I assume is in constant time.

I am in fact using exactly the same two-phase validation code you pointed out. However, I have to assume the token received in the Authorization Bearer is from a bad actor and headers may have been bogus. Hence the current effort to see if there's anything I'm neglecting.

@dvsekhvalnov
Copy link
Owner

So, help me understand. You want to check something before base64 decoding happened, but after token have been spliced into parts?

@pandex
Copy link
Author

pandex commented Aug 5, 2015

It happens in this order:

// This is where the cheap validation occurs

if !isValidTokenString(tokenString) {
    log.Println("MALFORMED")
}

// MAY BE a valid token, so decode further

keyBytes, err1 := ioutil.ReadFile(baseDir + "demo.rsa") // get PRIVATE key
if err1 != nil {
    log.Println(err1)
}

privateKey, err2 := Rsa.ReadPrivate(keyBytes)  
if err2 != nil {
    log.Println(err2)
}

j, err3 := jose.Decode(tokenString, privateKey)
if err3 != nil { 
    log.Println(err3) 
}

And encryption is via:

token, err := jose.Encrypt(string(s), jose.RSA_OAEP, jose.A256GCM, publicKey)

where s is the JWT Claims json.

@dvsekhvalnov
Copy link
Owner

can you post isValidTokenString(..) source?

@pandex
Copy link
Author

pandex commented Aug 5, 2015

Sure. it's a simple thing right now:

func isValidTokenString(s string) bool {
    l := len(s)
    if l < minTokenLen || l > maxTokenLen {
        fmt.Println("LENGTH ERROR") 
        return false
    }
    if strings.IndexAny(s, " ") != -1 {
       fmt.Println("SPACE ERROR") 
       return false
    }
// base64 check: THIS DID NOT WORK
// _, err := base64.StdEncoding.DecodeString(s)
// if err != nil {
//  fmt.Println("ENCODING ERROR")
//  return false
// }
    return true // string MAY BE a valid token

}

@dvsekhvalnov
Copy link
Owner

I'm not sure what is minTokenLen and maxTokenLen ? How can you know max?

base64 decode failing because:

  1. token should be spliced to parts by splitting on '.' This is what compact package is doing.
  2. it is base64 url safe encoded. You can use base64url package inside jose2go. But honestly it doesn't make much sense to me, because jose.Decode(..) will do exactly same. No reason to do decoding twice, it is for sure performance penalty.

So, what feature are you asking for? :) I really can't see anything new right now. You can perfectly do len constraints and illegal chars test outside of library. And Base64 check will be performed by library itself anyway.

@pandex
Copy link
Author

pandex commented Aug 5, 2015

Sorry, minTokenLen/maxTokenLen are my own constants, defined elsewhere.

I'm OK with base64 checking not working, it'd have been a quick check of multiple things in one swoop, but I'll do it outside the package. My assumption that any change of the string would invalidate it was wrong. Live and learn. So no new request, today. :)

@pandex
Copy link
Author

pandex commented Aug 5, 2015

I just have to figure out where the missing 5th part of the token is (in code that had been working fine for weeks) tomorrow. Thanks.

@dvsekhvalnov
Copy link
Owner

Hi @pandex , safe to close issue?

@pandex
Copy link
Author

pandex commented Aug 13, 2015

Sorry, please close it, all's good.

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

2 participants