add JWT timestamp formatting workaround (#4270)

Rego will sometimes serialize integers to JSON with a decimal point and
exponent. I don't completely understand this behavior.

Add a workaround to headers.rego to convert the JWT "iat" and "exp"
timestamps to a string and back to an integer. This appears to cause
Rego to serialize these values as plain integers.

Add a unit test to verify this behavior. Also add a unit test that will
fail if the Rego behavior changes, making this workaround unnecessary.
This commit is contained in:
Kenneth Jenkins 2023-06-16 10:36:00 -07:00 committed by GitHub
parent aa90cd4bb7
commit e7703a1891
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 72 additions and 4 deletions

View file

@ -1,16 +1,24 @@
package evaluator package evaluator
import ( import (
"bytes"
"context" "context"
"encoding/base64"
"encoding/json"
"fmt"
"math" "math"
"strconv"
"strings"
"testing" "testing"
"time" "time"
"github.com/go-jose/go-jose/v3/jwt" "github.com/go-jose/go-jose/v3/jwt"
"github.com/open-policy-agent/opa/rego"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto" "google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/structpb" "google.golang.org/protobuf/types/known/structpb"
"google.golang.org/protobuf/types/known/timestamppb"
"github.com/pomerium/pomerium/authorize/internal/store" "github.com/pomerium/pomerium/authorize/internal/store"
"github.com/pomerium/pomerium/config" "github.com/pomerium/pomerium/config"
@ -67,6 +75,8 @@ func TestHeadersEvaluator(t *testing.T) {
return e.Evaluate(ctx, input) return e.Evaluate(ctx, input)
} }
iat := time.Unix(1686870680, 0)
t.Run("jwt", func(t *testing.T) { t.Run("jwt", func(t *testing.T) {
output, err := eval(t, output, err := eval(t,
[]proto.Message{ []proto.Message{
@ -75,7 +85,7 @@ func TestHeadersEvaluator(t *testing.T) {
"name": {Values: []*structpb.Value{ "name": {Values: []*structpb.Value{
structpb.NewStringValue("n1"), structpb.NewStringValue("n1"),
}}, }},
}}, }, IssuedAt: timestamppb.New(iat)},
}, },
&HeadersRequest{ &HeadersRequest{
Issuer: "from.example.com", Issuer: "from.example.com",
@ -86,7 +96,29 @@ func TestHeadersEvaluator(t *testing.T) {
}) })
require.NoError(t, err) require.NoError(t, err)
rawJWT, err := jwt.ParseSigned(output.Headers.Get("X-Pomerium-Jwt-Assertion")) jwtHeader := output.Headers.Get("X-Pomerium-Jwt-Assertion")
// Make sure the 'iat' and 'exp' claims can be parsed as an integer. We
// need to do some explicit decoding in order to be able to verify
// this, as by default json.Unmarshal() will make no distinction
// between numeric formats.
d := json.NewDecoder(bytes.NewReader(decodeJWSPayload(t, jwtHeader)))
d.UseNumber()
var jwtPayloadDecoded map[string]interface{}
err = d.Decode(&jwtPayloadDecoded)
require.NoError(t, err)
// The 'iat' claim is set from the session store.
assert.Equal(t, json.Number("1686870680"), jwtPayloadDecoded["iat"],
"unexpected 'iat' timestamp format")
// The 'exp' claim will vary with the current time, but we can still
// use Atoi() to verify that it can be parsed as an integer.
exp := string(jwtPayloadDecoded["exp"].(json.Number))
_, err = strconv.Atoi(exp)
assert.NoError(t, err, "unexpected 'exp' timestamp format")
rawJWT, err := jwt.ParseSigned(jwtHeader)
require.NoError(t, err) require.NoError(t, err)
var claims M var claims M
@ -188,3 +220,31 @@ func TestHeadersEvaluator(t *testing.T) {
assert.Equal(t, "Bearer ID_TOKEN", output.Headers.Get("Authorization")) assert.Equal(t, "Bearer ID_TOKEN", output.Headers.Get("Authorization"))
}) })
} }
func decodeJWSPayload(t *testing.T, jws string) []byte {
t.Helper()
// A compact JWS string should consist of three base64-encoded values,
// separated by a '.' character. The payload is the middle one of these.
// cf. https://www.rfc-editor.org/rfc/rfc7515#section-7.1
parts := strings.Split(jws, ".")
require.Equal(t, 3, len(parts))
payload, err := base64.RawURLEncoding.DecodeString(parts[1])
require.NoError(t, err)
return payload
}
// If this test fails with the message "workaround no longer needed", then the
// upstream serialization issue in Rego has been fixed, and we should be able
// to remove the to_number / format_int workaround from headers.rego (and
// delete this test).
func TestTimestampWorkaroundStillNeeded(t *testing.T) {
now := strconv.FormatInt(time.Now().Unix(), 10)
r := rego.New(rego.Query(fmt.Sprintf("json.marshal(%s + 0)", now)))
rs, err := r.Eval(context.Background())
require.NoError(t, err, "rego evaluation error")
require.Equal(t, 1, len(rs))
e := rs[0].Expressions
require.Equal(t, 1, len(e))
assert.NotEqual(t, now, e[0].Value, "workaround no longer needed")
}

View file

@ -90,6 +90,10 @@ jwt_payload_exp = v {
v = five_minutes v = five_minutes
} else = null } else = null
jwt_payload_exp_int = v {
v = to_number(format_int(jwt_payload_exp, 10))
} else = null
jwt_payload_iat = v { jwt_payload_iat = v {
# sessions store the issued_at on the id_token # sessions store the issued_at on the id_token
v = round(session.id_token.issued_at.seconds) v = round(session.id_token.issued_at.seconds)
@ -98,6 +102,10 @@ jwt_payload_iat = v {
v = round(session.issued_at.seconds) v = round(session.issued_at.seconds)
} else = null } else = null
jwt_payload_iat_int = v {
v = to_number(format_int(jwt_payload_iat, 10))
} else = null
jwt_payload_sub = v { jwt_payload_sub = v {
v = session.user_id v = session.user_id
} else = "" } else = ""
@ -133,8 +141,8 @@ base_jwt_claims := [
["iss", jwt_payload_iss], ["iss", jwt_payload_iss],
["aud", jwt_payload_aud], ["aud", jwt_payload_aud],
["jti", jwt_payload_jti], ["jti", jwt_payload_jti],
["exp", jwt_payload_exp], ["exp", jwt_payload_exp_int],
["iat", jwt_payload_iat], ["iat", jwt_payload_iat_int],
["sub", jwt_payload_sub], ["sub", jwt_payload_sub],
["user", jwt_payload_user], ["user", jwt_payload_user],
["email", jwt_payload_email], ["email", jwt_payload_email],