Skip to content

Commit

Permalink
feat: add support for rotating jwt keys
Browse files Browse the repository at this point in the history
This allows jwt_secret to have multiple ',' separated secrets. The
first/leftmost should be used to sign new JWTs. All of them are used
(starting from left/newest) to try to verify a JWT.

If the first secret is < 32 chars in length JWTs are disabled. If any
of the other secrets are < 32 chars, the configuration code causes the
software to exit. This prevents insecure (too short) secrets from
being used.

Updated doc examples and tests.
  • Loading branch information
rouilj committed Mar 14, 2024
1 parent a2918d4 commit fd4f209
Show file tree
Hide file tree
Showing 6 changed files with 255 additions and 37 deletions.
2 changes: 2 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ Features:
- the roundup-admin history command now dumps the journal entries
in a more human readable format. Use the raw option to get the older
machine parsible output. (John Rouillard)
- Multiple JWT secrets are supported to allow key rotation. See
an updated config.ini for details. (John Rouillard)


2023-07-13 2.3.0
Expand Down
9 changes: 7 additions & 2 deletions doc/rest.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2071,7 +2071,9 @@ only been tested with python3)::
claim['roles'] = newroles
else:
claim['roles'] = user_roles
secret = self.db.config.WEB_JWT_SECRET

# Sign with newest/first secret.
secret = self.db.config.WEB_JWT_SECRET[0]
myjwt = jwt.encode(claim, secret, algorithm='HS256')

# if jwt.__version__ >= 2.0.0 jwt.encode() returns string
Expand All @@ -2090,7 +2092,10 @@ only been tested with python3)::

myjwt = input['jwt'].value

secret = self.db.config.WEB_JWT_SECRET
secret = self.db.config.WEB_JWT_SECRET[0]

# only return decoded result if the newest signing key
# is used. Have older keys report an invalid signature.
try:
result = jwt.decode(myjwt, secret,
algorithms=['HS256'],
Expand Down
43 changes: 31 additions & 12 deletions roundup/cgi/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1111,23 +1111,42 @@ def authenticate_bearer_token(self, challenge):
self.setHeader("WWW-Authenticate", "Basic")
raise LoginError('Support for jwt disabled.')

secret = self.db.config.WEB_JWT_SECRET
if len(secret) < 32:

# If first ',' separated token is < 32, jwt is disabled.
# If second or later tokens are < 32 chars, the config system
# stops the tracker from starting so insecure tokens can not
# be used.
if len(self.db.config.WEB_JWT_SECRET[0]) < 32:
# no support for jwt, this is fine.
self.setHeader("WWW-Authenticate", "Basic")
raise LoginError('Support for jwt disabled by admin.')

try: # handle jwt exceptions
token = jwt.decode(challenge, secret,
algorithms=['HS256'],
audience=self.db.config.TRACKER_WEB,
issuer=self.db.config.TRACKER_WEB)
except jwt.exceptions.InvalidTokenError as err:
self.setHeader("WWW-Authenticate", "Basic, Bearer")
self.make_user_anonymous()
raise LoginError(str(err))
last_error = "Unknown error validating bearer token."

for secret in self.db.config.WEB_JWT_SECRET:
try: # handle jwt exceptions
token = jwt.decode(challenge, secret,
algorithms=['HS256'],
audience=self.db.config.TRACKER_WEB,
issuer=self.db.config.TRACKER_WEB)
return (token)

except jwt.exceptions.InvalidSignatureError as err:
# Try more signatures.
# If all signatures generate InvalidSignatureError,
# we exhaust the loop and last_error is used to
# report the final (but not only) InvalidSignatureError
last_error = str(err) # preserve for end of loop
except jwt.exceptions.InvalidTokenError as err:
self.setHeader("WWW-Authenticate", "Basic, Bearer")
self.make_user_anonymous()
raise LoginError(str(err))

# reach here only if no valid signature was found
self.setHeader("WWW-Authenticate", "Basic, Bearer")
self.make_user_anonymous()
raise LoginError(last_error)

return (token)

def determine_user(self, is_api=False):
"""Determine who the user is"""
Expand Down
41 changes: 33 additions & 8 deletions roundup/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,25 @@ class SecretNullableOption(NullableOption, SecretOption):
get = SecretOption.get
class_description = SecretOption.class_description

class ListSecretOption(SecretOption):
# use get from SecretOption
def get(self):
value = SecretOption.get(self)
return [x.lstrip() for x in value.split(',')]

class_description = SecretOption.class_description

def validate(self, options):
if self.name == "WEB_JWT_SECRET":
secrets = self.get()
invalid_secrets = [ x for x in secrets[1:] if len(x) < 32]
if invalid_secrets:
raise OptionValueError(
self, ", ".join(secrets),
"One or more secrets less then 32 characters in length\n"
"found: %s" % ', '.join(invalid_secrets))
else:
self.get()

class RedisUrlOption(SecretNullableOption):
"""Do required check to make sure known bad parameters are not
Expand Down Expand Up @@ -1437,14 +1456,20 @@ def str2value(self, value):
"(Note the default value changes every time\n"
" roundup-admin updateconfig\n"
"is run, so it must be explicitly set to a non-empty string.\n"),
(SecretNullableOption, "jwt_secret", "disabled",
"This is used to generate/validate json web tokens (jwt).\n"
"Even if you don't use jwts it must not be empty.\n"
"If less than 256 bits (32 characters) in length it will\n"
"disable use of jwt. Changing this invalidates all jwts\n"
"issued by the roundup instance requiring *all* users to\n"
"generate new jwts. This is experimental and disabled by\n"
"default. It must be persistent across application restarts.\n"),
(ListSecretOption, "jwt_secret", "disabled",
"This is used to sign/validate json web tokens\n"
"(JWT). Even if you don't use JWTs it must not be\n"
"empty. You can use multiple secrets separated by a\n"
"comma ','. This allows for secret rotation. The newest\n"
"secret should be placed first and used for signing. The\n"
"rest of the secrets are used for validating an old JWT.\n"
"If the first secret is less than 256 bits (32\n"
"characters) in length JWTs are disabled. If other secrets\n"
"are less than 32 chars, the application will exit. Removing\n"
"a secret from this list invalidates all JWTs signed with\n"
"the secret. JWT support is experimental and disabled by\n"
"default. The secrets must be persistent across\n"
"application restarts.\n"),
)),
("rdbms", (
(DatabaseBackend, 'backend', NODEFAULT,
Expand Down
Loading

0 comments on commit fd4f209

Please sign in to comment.