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

authorizing principal from http header "username" instead of from jwt-user claim allows malicious impersonation #372

Closed
stephengaudet opened this issue Jun 2, 2023 · 0 comments
Labels
bug Something isn't working jwt

Comments

@stephengaudet
Copy link
Contributor

for the "jwt authZ per PKH feature", Signatory is not authorizing the principal name in the jwt claim "user", rather, it is authorizing the principal name found in the http header "username". this allows for user A to request signing from user B's key.

steps to reproduce:
1.configure 2 jwt users like so:

	c.Server.Jwt = JwtConfig{Users: map[string]*JwtUserData{"username1": {Password: "password1", Secret: secret},
		"username2": {Password: "password2", Secret: secret}}}

2.configure an active key that only authorizes username1 but not username2 like so:
c.Tezos[pkh1].JwtUsers = []string{"username1"}
3.get a bearer token for username2 like so:

	h = [][]string{{"Content-Type", "application/json"}, {"username", "username2"}, {"password", "password2"}}
	code, bytes = request(login, "", h)
	require.Equal(t, 201, code)
	token2 := string(bytes)

4.request signing from the PKH allowed for only username1 not username2. use username2's token, but, put "username1" in the username http header:

	h = [][]string{{"Content-Type", "application/json"}, {"username", "username1"}, {"Authorization", "Bearer " + token2}}
	code, bytes = request(url1, message, h)
	assert.Equal(t, 403, code)
	assert.Contains(t, string(bytes), "user `username2' is not authorized to access "+pkh1) 

expected:
http error 403, username2 is not authorized to access pkh1.

actual:
successful sign request with username1's key

failing integration test name: TestPerPkh

@stephengaudet stephengaudet added bug Something isn't working jwt labels Jun 2, 2023
AbineshECAD added a commit that referenced this issue Jun 7, 2023
stephengaudet added a commit that referenced this issue Jun 13, 2023
* JWT as a middleware authenticator

* Nil check for JWT

* user credentials verification

* Integration test for JWT

* signature invalid issue fixed

* Removed unwanted debug changes

* Removes debug prints

* test code refactor

* Documentation for JWT

* Doc page

* Spelling correction

* Doc corrections

* Spell correction

* Spell correction

* config read issues fixed

* Sample config added to document

* Doc correction

* Spell corrections

* Client authorization sidebar change in docs

* Client authorization sidebar change in docs

* Client authorization sidebar change in docs

* SidebarHeader code alignment change

* token expiry optional

* token expiry description added

* doc update on tok_expiry

* minor changes as per discussion

* doc tab corrections

* tok_exp changes as per discussion

* minor variable name changes

* credentials check logic updated

* sg add an integration test for jwt

* login endpoint added

* login serve issue fix

* fix jwt integration test

* add more jwt tests

* username and password error messages match

* unit test case for Login handler

* AuthHandler UT

* JWT and authorized_keys are mutually exclusive, can't use both

* sg-integrationtest add bad input test

* JWT users per PKH & credentials rotation

* Unit tests

* nil check for jwt users list

* UT refactor

* fix integration test

* per PKH config change

* error messages got JWT label

* secert validation changes

* fix jwt unit test new secret length constraint

* fix jwt unit test new secret length constraint

* fix jwt unit test

* fix integration test - new secret constraint

* add (failing) jwt password rotation integration test

* secret & password validation changes

* add jwt per pkh integration tests

* new and old user cred accepted in parallel till the exp time

* integration test - add password complexity 16 characters

* integrationtest - fix password rotation testcase - requires new secret

* fix for issue #372

* error message added

* fix broken test

* fix broken test

* old expiry logic change

* fix password rotation integration test

* tidy test

* small jwt doc improvements

* jwt doc fix typo

* bump octez version in integration tests

* update octez version in integration tests

---------

Co-authored-by: stephengaudet <stephen.gaudet@shaw.ca>
Co-authored-by: stephengaudet <32783698+stephengaudet@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working jwt
Projects
None yet
Development

No branches or pull requests

1 participant