Skip to content

Commit

Permalink
Verify signatures of JWTs when authorizing
Browse files Browse the repository at this point in the history
This commit upgrades the authoriser to pass all the metadata necessary
to verify received JWTs, assuming that they were signed by this node.

(Policy authors are free to verify tokens issued from other sources
and ignore the material passed to the policy.)

Policies can now call `io.jwt.decode_verify` as in the ns_anon policy
to verify submitted JWTs.
  • Loading branch information
simonwo committed Feb 13, 2024
1 parent b58e8c8 commit 666ce09
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 28 deletions.
3 changes: 1 addition & 2 deletions pkg/authz/policies/policy_ns_anon.rego
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,7 @@ token_namespaces := ns if {
startswith(authHeader, "Bearer ")
accessToken := trim_prefix(authHeader, "Bearer ")

# TODO: verify signature
[header, claims, sig] := io.jwt.decode(accessToken)
[valid, header, claims] := io.jwt.decode_verify(accessToken, input.constraints)
ns := claims["ns"]
}

Expand Down
43 changes: 38 additions & 5 deletions pkg/authz/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package authz

import (
"bytes"
"crypto/rsa"
"embed"
"encoding/json"
"errors"
"fmt"
"io"
Expand All @@ -11,6 +13,7 @@ import (
"strings"

"github.com/bacalhau-project/bacalhau/pkg/lib/policy"
"github.com/lestrrat-go/jwx/jwk"
"github.com/samber/lo"
)

Expand All @@ -21,7 +24,10 @@ import (
const AuthzAllowRule = "bacalhau.authz.allow"

type policyAuthorizer struct {
policy *policy.Policy
policy *policy.Policy
keyset string
nodeID string

allowQuery policy.Query[authzData, bool]
}

Expand All @@ -34,20 +40,39 @@ type httpData struct {
Body string `json:"body"`
}

type tokenData struct {
Keyset string `json:"cert"`
Issuer string `json:"iss"`
Audience string `json:"aud"`
}

type authzData struct {
HTTP httpData `json:"http"`
HTTP httpData `json:"http"`
Constraints tokenData `json:"constraints"`
}

//go:embed policies/*.rego
var policies embed.FS

// PolicyAuthorizer can authorize users by calling out to an external Rego
// policy containing logic to make decisions about who should be authorized.
func NewPolicyAuthorizer(authzPolicy *policy.Policy) Authorizer {
return &policyAuthorizer{
func NewPolicyAuthorizer(authzPolicy *policy.Policy, key *rsa.PublicKey, nodeID string) Authorizer {
p := &policyAuthorizer{
policy: authzPolicy,
nodeID: nodeID,
allowQuery: policy.AddQuery[authzData, bool](authzPolicy, AuthzAllowRule),
}

if key != nil {
keys := jwk.NewSet()
keys.Add(lo.Must(jwk.New(key)))

var keyset strings.Builder
lo.Must0(json.NewEncoder(&keyset).Encode(keys))
p.keyset = keyset.String()
}

return p
}

// Authorize runs the loaded policy and provides a structure representing the
Expand Down Expand Up @@ -81,6 +106,14 @@ func (authorizer *policyAuthorizer) Authorize(req *http.Request) (Authorization,
Headers: req.Header,
Body: body.String(),
},
// Metadata that can be used to verify the JWT, if it was signed by this
// requester node (which does not have to be the case – users can submit
// tokens signed elsewhere as long as the policy verifies them)
Constraints: tokenData{
Keyset: authorizer.keyset,
Issuer: authorizer.nodeID,
Audience: authorizer.nodeID,
},
}

approved, err := authorizer.allowQuery(req.Context(), in)
Expand All @@ -93,4 +126,4 @@ var AlwaysAllowPolicy = lo.Must(policy.FromFS(policies, "policies/policy_test_al

// AlwaysAllow is an authorizer that will always permit access, irrespective of
// the passed in data, which is useful for testing.
var AlwaysAllow = NewPolicyAuthorizer(AlwaysAllowPolicy)
var AlwaysAllow = NewPolicyAuthorizer(AlwaysAllowPolicy, nil, "")
48 changes: 34 additions & 14 deletions pkg/authz/policy_ns_anon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ package authz

import (
"bytes"
"crypto"
"crypto/rand"
"crypto/rsa"
"net/http"
"testing"

Expand All @@ -23,14 +26,25 @@ const (
NamespaceCancellable uint8 = 0b1000
)

func getJWTWithNamespace(t *testing.T, namespace string, perms uint8) string {
token, err := jwt.NewWithClaims(jwt.GetSigningMethod("none"), jwt.MapClaims{
"ns": map[string]uint8{namespace: perms},
}).SignedString(jwt.UnsafeAllowNoneSignatureType)
func getJWTWithNamespace(t *testing.T, signingKey crypto.PrivateKey, namespace string, perms uint8) string {
token, err := jwt.NewWithClaims(jwt.GetSigningMethod(jwt.SigningMethodRS256.Name), jwt.MapClaims{
"aud": []string{"test-node"},
"iss": "test-node",
"ns": map[string]uint8{namespace: perms},
}).SignedString(signingKey)
require.NoError(t, err)
return token
}

var (
sameKey = func(t *testing.T, in crypto.PrivateKey) crypto.PrivateKey { return in }
newKey = func(t *testing.T, in crypto.PrivateKey) crypto.PrivateKey {
newKey, err := rsa.GenerateKey(rand.Reader, 2048)
require.NoError(t, err)
return newKey
}
)

func TestAppliesAnonymousNamespacePolicy(t *testing.T) {
cases := []struct {
name string
Expand All @@ -40,29 +54,35 @@ func TestAppliesAnonymousNamespacePolicy(t *testing.T) {
token_perms uint8
method string
path string
getSignerKey func(*testing.T, crypto.PrivateKey) crypto.PrivateKey
checker func(require.TestingT, bool, ...interface{})
}{
{"allow valid job submission",
"test", "test", "test", NamespaceWritable, http.MethodPut, "/api/v1/orchestrator/jobs", require.True},
"test", "test", "test", NamespaceWritable, http.MethodPut, "/api/v1/orchestrator/jobs", sameKey, require.True},
{"deny job submit to unwritable namespace",
"test", "test", "test", NamespaceReadable, http.MethodPut, "/api/v1/orchestrator/jobs", require.False},
"test", "test", "test", NamespaceReadable, http.MethodPut, "/api/v1/orchestrator/jobs", sameKey, require.False},
{"deny job submit to alternative namespace",
"other", "other", "test", NamespaceWritable, http.MethodPut, "/api/v1/orchestrator/jobs", require.False},
"other", "other", "test", NamespaceWritable, http.MethodPut, "/api/v1/orchestrator/jobs", sameKey, require.False},
{"allow valid job read",
"test", "test", "test", NamespaceReadable, http.MethodGet, "/api/v1/orchestrator/jobs", require.True},
"test", "test", "test", NamespaceReadable, http.MethodGet, "/api/v1/orchestrator/jobs", sameKey, require.True},
{"deny job read to unreadable namespace",
"test", "test", "test", NamespaceWritable, http.MethodGet, "/api/v1/orchestrator/jobs", require.False},
"test", "test", "test", NamespaceWritable, http.MethodGet, "/api/v1/orchestrator/jobs", sameKey, require.False},
{"deny job read to alternative namespace",
"other", "other", "test", NamespaceReadable, http.MethodGet, "/api/v1/orchestrator/jobs", require.False},
"other", "other", "test", NamespaceReadable, http.MethodGet, "/api/v1/orchestrator/jobs", sameKey, require.False},
{"allow reading other APIs without token",
"other", "other", "test", NamespaceNoPermission, http.MethodGet, "/api/v1/orchestrator/nodes", require.True},
"other", "other", "test", NamespaceNoPermission, http.MethodGet, "/api/v1/orchestrator/nodes", sameKey, require.True},
{"deny writing other APIs",
"other", "other", "test", NamespaceNoPermission, http.MethodDelete, "/api/v1/orchestrator/nodes", require.False},
"other", "other", "test", NamespaceNoPermission, http.MethodDelete, "/api/v1/orchestrator/nodes", sameKey, require.False},
{"deny signed by wrong key",
"test", "test", "test", NamespaceWritable, http.MethodPut, "/api/v1/orchestrator/jobs", newKey, require.False},
}

key, err := rsa.GenerateKey(rand.Reader, 2048)
require.NoError(t, err)

policy, err := policy.FromFS(policies, "policies/policy_ns_anon.rego")
require.NoError(t, err)
authorizer := NewPolicyAuthorizer(policy)
authorizer := NewPolicyAuthorizer(policy, &key.PublicKey, "test-node")

for _, testcase := range cases {
t.Run(testcase.name, func(t *testing.T) {
Expand All @@ -82,7 +102,7 @@ func TestAppliesAnonymousNamespacePolicy(t *testing.T) {
require.NoError(t, err)

if testcase.token_perms > 0 {
token := getJWTWithNamespace(t, testcase.token_namespace, testcase.token_perms)
token := getJWTWithNamespace(t, testcase.getSignerKey(t, key), testcase.token_namespace, testcase.token_perms)
request.Header.Add("Authorization", "Bearer "+token)
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/authz/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func TestAllowsWhenPolicySaysAllow(t *testing.T) {
policy, err := policy.FromFS(policies, "policies/policy_test_allow.rego")
require.NoError(t, err)

authorizer := NewPolicyAuthorizer(policy)
authorizer := NewPolicyAuthorizer(policy, nil, "")
require.NotNil(t, authorizer)

result, err := authorizer.Authorize(request)
Expand All @@ -32,7 +32,7 @@ func TestDeniesWhenPolicySaysDeny(t *testing.T) {
policy, err := policy.FromFS(policies, "policies/policy_test_deny.rego")
require.NoError(t, err)

authorizer := NewPolicyAuthorizer(policy)
authorizer := NewPolicyAuthorizer(policy, nil, "")
require.NotNil(t, authorizer)

result, err := authorizer.Authorize(request)
Expand All @@ -44,7 +44,7 @@ func TestPolicyEvaluatedAgainstHTTPRequest(t *testing.T) {
policy, err := policy.FromFS(policies, "policies/policy_test_http.rego")
require.NoError(t, err)

authorizer := NewPolicyAuthorizer(policy)
authorizer := NewPolicyAuthorizer(policy, nil, "")
require.NotNil(t, authorizer)

goodRequest, err := http.NewRequest(http.MethodGet, "/api/v1/hello", nil)
Expand Down
2 changes: 1 addition & 1 deletion pkg/lib/policy/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ type Query[Input, Output any] func(ctx context.Context, input Input) (Output, er
// certain input type and returns a function that will execute the query when
// given input of that type.
func AddQuery[Input, Output any](runner *Policy, rule string) Query[Input, Output] {
opts := append(runner.modules, rego.Query("data."+rule), scryptFn)
opts := append(runner.modules, rego.Query("data."+rule), scryptFn, rego.StrictBuiltinErrors(true))
query := lo.Must(rego.New(opts...).PrepareForEval(context.Background()))

return func(ctx context.Context, t Input) (Output, error) {
Expand Down
7 changes: 6 additions & 1 deletion pkg/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,11 @@ func NewNode(
return nil, err
}

signingKey, err := pkgconfig.GetClientPublicKey()
if err != nil {
return nil, err
}

serverVersion := version.Get()
// public http api server
serverParams := publicapi.ServerParams{
Expand All @@ -177,7 +182,7 @@ func NewNode(
Port: config.APIPort,
HostID: config.NodeID,
Config: config.APIServerConfig,
Authorizer: authz.NewPolicyAuthorizer(authzPolicy),
Authorizer: authz.NewPolicyAuthorizer(authzPolicy, signingKey, config.NodeID),
Headers: map[string]string{
apimodels.HTTPHeaderBacalhauGitVersion: serverVersion.GitVersion,
apimodels.HTTPHeaderBacalhauGitCommit: serverVersion.GitCommit,
Expand Down
4 changes: 2 additions & 2 deletions webui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@
"storybook": "storybook dev -p 6006",
"build-storybook": "storybook build",
"build": "yarn build:dev",
"build:dev": "tsc && NODE_ENV=development vite build",
"build:prod": "tsc && NODE_ENV=production vite build",
"build:dev": "npx tsc && NODE_ENV=development vite build",
"build:prod": "npx tsc && NODE_ENV=production vite build",
"start": "vite",
"serve": "yarn start"
},
Expand Down

0 comments on commit 666ce09

Please sign in to comment.