Reduced JWT tokens for HTTP login - #4967
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the existing random session key mechanism with JWT-based tokens for both in-game and HTTP login flows, refactors Base64 and HMAC utilities, and removes IP arguments from HTTP handlers.
- Refactor cryptographic helpers (
transformToSHA1,hmac) to use RAII and custom deleters - Implement URL-safe, no-padding Base64 encode/decode and add a new
sessionSecretconfig - Switch in-game and HTTP login to HS256 JWTs, update router and tests to drop IP parameters
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/tools.cpp | Introduce C_ptr/Deleter and refactor transformToSHA1/hmac |
| src/base64.cpp | Replace BIO-based Base64 with custom URL-safe, unpadded encoder/decoder |
| src/configmanager.h | Add SESSION_SECRET key to string_config_t |
| src/configmanager.cpp | Load/base64::decode the sessionSecret from Lua config |
| src/protocollogin.cpp | Generate in-game session tokens as JWT(HS256) |
| src/rsa.cpp | Add error check and exception on RSA decrypt failure |
| src/http/login.h | Update handle_login signature to remove IP parameter |
| src/http/login.cpp | Refactor HTTP login to JWT tokens, drop DB session insert/IP use |
| src/http/cacheinfo.h | Update handle_cacheinfo signature to remove IP parameter |
| src/http/cacheinfo.cpp | Refactor cacheinfo handler to drop IP |
| src/http/router.cpp | Adjust router to call handlers without IP |
| src/tests/test_base64.cpp | Update tests to expect unpadded Base64 output |
| src/http/tests/test_login.cpp | Update HTTP login tests to use new handler signature |
| src/http/tests/test_cacheinfo.cpp | Update cacheinfo tests to use new handler signature |
| CMakeLists.txt | Add vcpkg features, HTTP compile-def, libmysql/mariadb option |
| config.lua.dist | Introduce sessionSecret setting for JWT support |
Comments suppressed due to low confidence (3)
src/http/login.cpp:34
- [nitpick] The new JWT-based session key generation logic in handle_login lacks dedicated unit tests to verify the correct header.payload.signature structure and token expiration handling.
std::pair<status, json::value> tfs::http::handle_login(const json::object& body)
config.lua.dist:30
- Add a note clarifying that
sessionSecretshould be provided as a base64-encoded string (not plaintext), so users know to encode their secret before configuring.
sessionSecret = "c2VjcmV0" -- CHANGE THIS VALUE!
src/protocollogin.cpp:125
- Double base64-encoding the JWT inflates the token and may break client expectations; consider passing the raw
sessionKey(the header.payload.signature) directly instead of encoding it again.
output->addString(tfs::base64::encode(sessionKey));
I think I know why it happens. Session key is send encrypted using RSA. Tibia uses 1024-bit RSA key, which is 128 bytes, from which All bytes from forgottenserver/src/protocolgame.cpp Lines 367 to 411 in c519cc6 must fit together into 117 bytes. IDK how many bytes are there for session token. JWT won't work in this scenario. We need much shorter format ex. first format proposed here: 4 lines (no JSON, as it wastes too many characters for formatting):
I don't see any chance to put full client IP there. It would be possible, if we allow only IPv4, but with IPv6 it's too long. EDIT: |
|
I see no value in adding a checksum at the end. However, sending the account id + timestamp and signing those with HMAC should be safe enough to assume it isn't fake and still be secure. It's a thinner version of JWT without the fixed header + JSON boilerplate |
By checksum, I meant "4. SHA1 sign:", which is like HMAC-SHA1, but as I described, we must truncate it to 10-20 ASCII characters (from 40 in hexadecimal format). I didn't want to call it HMAC, because HMAC is known format and we will use something custom.
Don't forget about password. We need some hash of password to make session token fail to login into game, after password to account is changed. I checked how many bytes are used right now by Tibia 13.10, there are 50-70 free bytes (of 127 RSA-1024 bytes), even, if we allow character name with up to 30 letters. Let's go back with discussion about format to #4963 |
Pull Request Prelude
Changes Proposed
See #4963
Current issues:
Tibia client sends a malformed packet for login (does not decrypt), could be my client misbehaving.OTClient crashes hard because the token is too long for it's expectations.OTClient bugs printing that a message was received with the wrong checksum. Probably just a bug.