Generate more secure API tokens

[lunkwill42]

SonarCloud flagged this in a PR. The "vulnerability" was not introduced by the PR itself, which just updated the already existing code line's Python 2-compatible string handling to be Python 3-only:

https://sonarcloud.io/project/security_hotspots?id=Uninett_nav&pullRequest=2365&hotspots=AX-wyHGM2WXABqWMNA13

Although the seriousness of this flagged vulnerability is disputable, we can probably easily fix it. This is the affected function:

nav/python/nav/util.py

Lines 474 to 480 in cb9b3d4

	def auth_token():
	"""Generates a hash that can be used as an OAuth API token"""
	from django.conf import settings
	_hash = hashlib.sha1(six.text_type(uuid.uuid4()).encode('utf-8'))
	_hash.update(settings.SECRET_KEY.encode('utf-8'))
	return _hash.hexdigest()

The point of this isn't really to create a SHA-1 signature of anything, but to produce a sufficiently random string to persist as an authentication token for an API client. The resulting token string is stored in the NAV database and used to compare with tokens supplied by API clients.

secrets was introduced to the standard library in Python 3.6, and we should potentially just use something like secrets.token_hex(32) to generate this auth token. Changing the method used to issue new tokens should not affect existing valid tokens, since they are persisted in the database.

[stveit]

The library is prob fine, but we should consider the token size. I think its pretty conventional for secret tokens to be 256 bits in this day and age.

[lunkwill42]

The library is prob fine, but we should consider the token size. I think its pretty conventional for secret tokens to be 256 bits in this day and age.

Yes, calling token_hex() with nbytes=32, as suggested, will give a 256 bit result.
Another question is whether there is an actual point to including the site's SECRET_KEY setting in the hash, like the current implementation does...

[stveit]

Unless there is a good reason for it (e.g. if this was for generating hmac stuff), its probably best to not include it. If the goal is to have a random string then secrets.token_hex(32) achieves that.

[hmpf]

Yes to using the secrets-library! Interesting that it does not seem to be seed-able.

[lunkwill42]

Right-o. It seems that we are all in agreement that an implementation that just consists of return secrets.token_hex(32) is acceptable. I'm throwing this on the current sprintboard for anyone to grab :)