From 3a151863c8c4c03cfa7b1d5b516dbf29c16bcdda Mon Sep 17 00:00:00 2001 From: Jim Kalafut Date: Tue, 25 Jun 2019 16:52:11 -0700 Subject: [PATCH 1/3] Add option to allow verbose OIDC logging --- path_oidc.go | 10 ++++++++++ path_role.go | 12 +++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/path_oidc.go b/path_oidc.go index c633daaa..2ca7b781 100644 --- a/path_oidc.go +++ b/path_oidc.go @@ -2,6 +2,7 @@ package jwtauth import ( "context" + "encoding/json" "fmt" "net/url" "strings" @@ -136,6 +137,9 @@ func (b *jwtAuthBackend) pathCallback(ctx context.Context, req *logical.Request, if !ok { return logical.ErrorResponse(errTokenVerification + " No id_token found in response."), nil } + if role.VerboseOIDCLogging { + b.Logger().Debug("OIDC provider response", "ID token", rawToken) + } // Parse and verify ID Token payload. allClaims, err := b.verifyOIDCToken(ctx, config, role, rawToken) @@ -161,6 +165,12 @@ func (b *jwtAuthBackend) pathCallback(ctx context.Context, req *logical.Request, logFunc("error reading /userinfo endpoint", "error", err) } + if role.VerboseOIDCLogging { + if c, err := json.Marshal(allClaims); err == nil { + b.Logger().Debug("OIDC provider response", "claims", string(c)) + } + } + if err := validateBoundClaims(b.Logger(), role.BoundClaims, allClaims); err != nil { return logical.ErrorResponse("error validating claims: %s", err.Error()), nil } diff --git a/path_role.go b/path_role.go index 3b57d2a3..a6d8bddf 100644 --- a/path_role.go +++ b/path_role.go @@ -4,10 +4,11 @@ import ( "context" "errors" "fmt" - "gopkg.in/square/go-jose.v2/jwt" "strings" "time" + "gopkg.in/square/go-jose.v2/jwt" + "github.com/hashicorp/go-sockaddr" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/helper/parseutil" @@ -128,6 +129,9 @@ authenticate against this role`, Type: framework.TypeCommaStringSlice, Description: `Comma-separated list of allowed values for redirect_uri`, }, + "verbose_oidc_logging": { + Type: framework.TypeBool, + }, }, ExistenceCheck: b.pathRoleExistenceCheck, Operations: map[logical.Operation]framework.OperationHandler{ @@ -199,6 +203,7 @@ type jwtRole struct { GroupsClaim string `json:"groups_claim"` OIDCScopes []string `json:"oidc_scopes"` AllowedRedirectURIs []string `json:"allowed_redirect_uris"` + VerboseOIDCLogging bool `json:"verbose_oidc_logging"` } // role takes a storage backend and the name and returns the role's storage @@ -279,6 +284,7 @@ func (b *jwtAuthBackend) pathRoleRead(ctx context.Context, req *logical.Request, "groups_claim": role.GroupsClaim, "allowed_redirect_uris": role.AllowedRedirectURIs, "oidc_scopes": role.OIDCScopes, + "verbose_oidc_logging": role.VerboseOIDCLogging, }, } @@ -386,6 +392,10 @@ func (b *jwtAuthBackend) pathRoleCreateUpdate(ctx context.Context, req *logical. role.BoundSubject = boundSubject.(string) } + if verboseOIDCLoggingRaw, ok := data.GetOk("verbose_oidc_logging"); ok { + role.VerboseOIDCLogging = verboseOIDCLoggingRaw.(bool) + } + if boundCIDRs, ok := data.GetOk("bound_cidrs"); ok { parsedCIDRs, err := parseutil.ParseAddrs(boundCIDRs) if err != nil { From e2893346bce35f62eb880eb2d058e9003cfdb489 Mon Sep 17 00:00:00 2001 From: Jim Kalafut Date: Tue, 25 Jun 2019 21:54:18 -0700 Subject: [PATCH 2/3] Fix test --- path_role_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/path_role_test.go b/path_role_test.go index 250dbca4..c4ca280c 100644 --- a/path_role_test.go +++ b/path_role_test.go @@ -530,6 +530,7 @@ func TestPath_Read(t *testing.T) { "expiration_leeway": int64(500), "not_before_leeway": int64(500), "clock_skew_leeway": int64(100), + "verbose_oidc_logging": false, } req := &logical.Request{ From e814a3fedd62fd1ccf2c574ddad6dfee26ade2aa Mon Sep 17 00:00:00 2001 From: Jim Kalafut Date: Mon, 1 Jul 2019 09:51:00 -0700 Subject: [PATCH 3/3] Add warnings to verbose_oidc_logging update --- path_oidc.go | 2 ++ path_role.go | 12 ++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/path_oidc.go b/path_oidc.go index 2ca7b781..c127bba7 100644 --- a/path_oidc.go +++ b/path_oidc.go @@ -168,6 +168,8 @@ func (b *jwtAuthBackend) pathCallback(ctx context.Context, req *logical.Request, if role.VerboseOIDCLogging { if c, err := json.Marshal(allClaims); err == nil { b.Logger().Debug("OIDC provider response", "claims", string(c)) + } else { + b.Logger().Debug("OIDC provider response", "marshalling error", err.Error()) } } diff --git a/path_role.go b/path_role.go index a6d8bddf..3a2719d2 100644 --- a/path_role.go +++ b/path_role.go @@ -131,6 +131,9 @@ authenticate against this role`, }, "verbose_oidc_logging": { Type: framework.TypeBool, + Description: `Log received OIDC tokens and claims when debug-level logging is active. +Not recommended in production since sensitive information may be present +in OIDC responses.`, }, }, ExistenceCheck: b.pathRoleExistenceCheck, @@ -469,12 +472,17 @@ func (b *jwtAuthBackend) pathRoleCreateUpdate(ctx context.Context, req *logical. return logical.ErrorResponse("ttl should not be greater than max_ttl"), nil } - var resp *logical.Response + resp := &logical.Response{} if role.MaxTTL > b.System().MaxLeaseTTL() { - resp = &logical.Response{} resp.AddWarning("max_ttl is greater than the system or backend mount's maximum TTL value; issued tokens' max TTL value will be truncated") } + if role.VerboseOIDCLogging { + resp.AddWarning(`verbose_oidc_logging has been enabled for this role. ` + + `This is not recommended in production since sensitive information ` + + `may be present in OIDC responses.`) + } + // Store the entry. entry, err := logical.StorageEntryJSON(rolePrefix+roleName, role) if err != nil {