Rs256 jwt signature verification #640
Conversation
topdown/topdown_test.go
Outdated
| var exp interface{} | ||
| exp = ast.String(p.result) | ||
| if p.err != "" { | ||
| exp = errors.New(p.err) |
There was a problem hiding this comment.
Did you mean to exercise this above?
topdown/tokens_verify.go
Outdated
| } | ||
| token := string(astToken) | ||
| if !strings.Contains(token, ".") { | ||
| return nil, errors.New("encoded JWT has no period separators") |
There was a problem hiding this comment.
I don't know if this error case is useful since we split the string and check below. I would probably remove it.
topdown/tokens_verify.go
Outdated
| err = rsa.VerifyPKCS1v15(publicKey, crypto.SHA256, getInputSHA(headerPayload), []byte(signature)) | ||
|
|
||
| if err != nil { | ||
| return ast.String("RS256 JWT signature verification failed"), nil |
There was a problem hiding this comment.
Is there a reason we're not returning the error here?
There was a problem hiding this comment.
Returning an error now
topdown/tokens_verify.go
Outdated
|
|
||
| cert, err := x509.ParseCertificate(block.Bytes) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to parse PEM certificate %v", err) |
There was a problem hiding this comment.
See errors.Wrapf for returning wrapped errors. This way the embedded error formatting is more consistent. For example, I would use errors.Wrapf(err, "PEM parse error").
topdown/tokens_verify.go
Outdated
| return base64.StdEncoding.DecodeString(data) | ||
| } | ||
|
|
||
| // encode encodes given byte array to base64url string |
There was a problem hiding this comment.
Can this be replaced with base64.URLEncoding.EncodeToString?
There was a problem hiding this comment.
Yes it can. Do we want to ?
There was a problem hiding this comment.
Yes, let's use the standard library implementation instead.
topdown/tokens_verify.go
Outdated
| return hasher.Sum(nil) | ||
| } | ||
|
|
||
| // decode decodes base64url string to byte array |
There was a problem hiding this comment.
Can this be replaced with base64.URLEncoding.Decode?
There was a problem hiding this comment.
I need conversion from string to []byte . So "Decode" won't help.
There was a problem hiding this comment.
I think base64.URLEncoding.DecodeString will work.
ashutosh-narkar
changed the title
ashutosh-narkar
changed the title
Fixes open-policy-agent#421 removed blank line updated test added command info documentation wrap the error messages used buitin URL decode method moved verify token code in tokens module
ashutosh-narkar
force-pushed
the
rs256-jwt-verify
branch
from
cbd55f6 to
f89402b
Compare
A builtin to verify RS256 JWT signatures
Issue - #421
Fixes #421