From 4ffae5d862346bec5726ab5eccbfbf819870472b Mon Sep 17 00:00:00 2001 From: AJ Martinez Date: Sat, 18 Mar 2023 18:05:46 -0700 Subject: [PATCH] Store keypairs in profiles, do signature verification for all splitscreen players --- src/d_clisrv.c | 62 +++++++++++++++++++++++++++++++++++------------- src/d_clisrv.h | 10 ++++---- src/d_main.c | 34 -------------------------- src/d_netfil.c | 7 +----- src/k_profiles.c | 27 +++++++++++++++++++-- src/k_profiles.h | 5 ++++ src/p_saveg.c | 4 ++++ 7 files changed, 85 insertions(+), 64 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index c2d078b34..29501d37c 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -157,8 +157,8 @@ char connectedservername[MAXSERVERNAME]; /// \todo WORK! boolean acceptnewnode = true; -uint8_t lastReceivedKey[MAXNETNODES][32]; -uint8_t lastSentChallenge[MAXNETNODES][32]; +uint8_t lastReceivedKey[MAXNETNODES][MAXSPLITSCREENPLAYERS][32]; +uint8_t lastSentChallenge[MAXNETNODES][MAXSPLITSCREENPLAYERS][32]; boolean serverisfull = false; //lets us be aware if the server was full after we check files, but before downloading, so we can ask if the user still wants to download or not tic_t firstconnectattempttime = 0; @@ -834,25 +834,45 @@ static boolean CL_SendJoin(void) memcpy(&netbuffer->u.clientcfg.availabilities, R_GetSkinAvailabilities(false, false), MAXAVAILABILITY*sizeof(UINT8)); - uint8_t signature[64]; - crypto_eddsa_sign(signature, secret_key, awaitingChallenge, 32); + // Don't leak old signatures from prior sessions. + memset(&netbuffer->u.clientcfg.challengeResponse, 0, sizeof(((clientconfig_pak *)0)->challengeResponse)); - if (crypto_eddsa_check(signature, public_key, awaitingChallenge, 32) != 0) - I_Error("Couldn't verify own key?"); + for (i = 0; i <= splitscreen; i++) + { + uint8_t signature[64]; + profile_t *localProfile = PR_GetLocalPlayerProfile(i); - // Testing - // memset(signature, 0, sizeof(signature)); + if (cv_lastprofile[0].value == 0) // GUESTS don't have keys + { + memset(signature, 0, 64); + } + else + { + crypto_eddsa_sign(signature, localProfile->secret_key, awaitingChallenge, 32); + if (crypto_eddsa_check(signature, localProfile->public_key, awaitingChallenge, 32) != 0) + I_Error("Couldn't self-verify key associated with player %d, profile %d.\nProfile data may be corrupted.", i, cv_lastprofile[i].value); // I guess this is the most reasonable way to catch a malformed key. + } - memcpy(&netbuffer->u.clientcfg.challengeResponse, signature, sizeof(signature)); + // Testing + // memset(signature, 0, sizeof(signature)); + + memcpy(&netbuffer->u.clientcfg.challengeResponse[i], signature, sizeof(signature)); + } return HSendPacket(servernode, false, 0, sizeof (clientconfig_pak)); } static boolean CL_SendKey(void) { + int i; netbuffer->packettype = PT_CLIENTKEY; - memcpy(netbuffer->u.clientkey.key, public_key, sizeof(public_key)); + memset(netbuffer->u.clientkey.key, 0, sizeof(((clientkey_pak *)0)->key)); + for (i = 0; i <= splitscreen; i++) + { + // GUEST profiles have all-zero keys. This will be handled at the end of the challenge process, don't worry about it. + memcpy(netbuffer->u.clientkey.key[i], PR_GetProfile(cv_lastprofile[i].value)->public_key, 32); + } return HSendPacket(servernode, false, 0, sizeof (clientkey_pak) ); } @@ -3694,7 +3714,7 @@ static void Got_AddPlayer(UINT8 **p, INT32 playernum) players[newplayernum].splitscreenindex = splitscreenplayer; players[newplayernum].bot = false; - memcpy(players[newplayernum].public_key, lastReceivedKey[node], sizeof(public_key)); + memcpy(players[newplayernum].public_key, lastReceivedKey[node][splitscreenplayer], sizeof(players[newplayernum].public_key)); playerconsole[newplayernum] = console; splitscreen_original_party_size[console] = @@ -4069,8 +4089,6 @@ static void HandleConnect(SINT8 node) // Testing // memset(netbuffer->u.clientcfg.challengeResponse, 0, sizeof(netbuffer->u.clientcfg.challengeResponse)); - int sigcheck = crypto_eddsa_check(netbuffer->u.clientcfg.challengeResponse, lastReceivedKey[node], lastSentChallenge[node], 32); - if (bannednode && bannednode[node].banid != SIZE_MAX) { const char *reason = NULL; @@ -4152,13 +4170,12 @@ static void HandleConnect(SINT8 node) SV_SendRefuse(node, va(M_GetText("Too many people are connecting.\nPlease wait %d seconds and then\ntry rejoining."), (joindelay - 2 * cv_joindelay.value * TICRATE) / TICRATE)); } - else if (netgame && node != 0 && sigcheck != 0) - { - SV_SendRefuse(node, M_GetText("Signature verification failed.")); - } else { + int sigcheck; boolean newnode = false; + char allZero[32]; + memset(allZero, 0, 32); for (i = 0; i < netbuffer->u.clientcfg.localplayers - playerpernode[node]; i++) { @@ -4168,6 +4185,17 @@ static void HandleConnect(SINT8 node) SV_SendRefuse(node, "Bad player name"); return; } + + if (memcmp(lastReceivedKey[node], allZero, 32)) // We're a GUEST and the server throws out our keys anyway. + sigcheck = 0; // Always succeeds. Yes, this is a success response. C R Y P T O + else + sigcheck = crypto_eddsa_check(netbuffer->u.clientcfg.challengeResponse[i], lastReceivedKey[node][i], lastSentChallenge[node][i], 32); + + if (netgame && node != 0 && sigcheck != 0) + { + SV_SendRefuse(node, M_GetText("Signature verification failed.")); + return; + } } memcpy(availabilitiesbuffer, netbuffer->u.clientcfg.availabilities, sizeof(availabilitiesbuffer)); diff --git a/src/d_clisrv.h b/src/d_clisrv.h index 43e9bf540..2c2ffaef0 100644 --- a/src/d_clisrv.h +++ b/src/d_clisrv.h @@ -256,7 +256,7 @@ struct clientconfig_pak UINT8 mode; char names[MAXSPLITSCREENPLAYERS][MAXPLAYERNAME]; UINT8 availabilities[MAXAVAILABILITY]; - uint8_t challengeResponse[64]; + uint8_t challengeResponse[MAXSPLITSCREENPLAYERS][64]; } ATTRPACK; #define SV_SPEEDMASK 0x03 // used to send kartspeed @@ -353,12 +353,12 @@ struct filesneededconfig_pak struct clientkey_pak { - char key[32]; + char key[MAXSPLITSCREENPLAYERS][32]; } ATTRPACK; struct serverchallenge_pak { - char secret[32]; + char secret[MAXSPLITSCREENPLAYERS][32]; } ATTRPACK; // @@ -460,8 +460,8 @@ extern UINT16 software_MAXPACKETLENGTH; extern boolean acceptnewnode; extern SINT8 servernode; extern char connectedservername[MAXSERVERNAME]; -extern uint8_t lastReceivedKey[MAXNETNODES][32]; -extern uint8_t lastSentChallenge[MAXNETNODES][32]; +extern uint8_t lastReceivedKey[MAXNETNODES][MAXSPLITSCREENPLAYERS][32]; +extern uint8_t lastSentChallenge[MAXNETNODES][MAXSPLITSCREENPLAYERS][32]; void Command_Ping_f(void); extern tic_t connectiontimeout; diff --git a/src/d_main.c b/src/d_main.c index 8bfe889cb..a33125acd 100644 --- a/src/d_main.c +++ b/src/d_main.c @@ -159,10 +159,6 @@ INT32 eventhead, eventtail; boolean dedicated = false; -// For identity negotiation with netgame servers -uint8_t public_key[32]; -uint8_t secret_key[64]; - // // D_PostEvent // Called by the I/O functions when input is detected @@ -1715,36 +1711,6 @@ void D_SRB2Main(void) ACS_Init(); CON_SetLoadingProgress(LOADED_ACSINIT); - // TODO: This file should probably give a fuck about command line params, - // or not be stored next to the EXE in a way that allows people to unknowingly send it to others. - static char keyfile[16] = "rrid.dat"; - - static uint8_t seed[32]; - csprng(seed, 32); - crypto_eddsa_key_pair(secret_key, public_key, seed); - - int sk_size = sizeof(secret_key); - int pk_size = sizeof(public_key); - int totalsize = sk_size + pk_size; - - if (FIL_ReadFileOK(keyfile)) - { - UINT8 *readbuffer = NULL; - UINT16 lengthRead = FIL_ReadFile(keyfile, &readbuffer); - if (readbuffer == NULL || lengthRead != totalsize) - I_Error("Malformed keyfile"); - memcpy(secret_key, readbuffer, sk_size); - memcpy(public_key, readbuffer + sk_size, pk_size); - } - else - { - uint8_t keybuffer[totalsize]; - memcpy(keybuffer, secret_key, sk_size); - memcpy(keybuffer + sk_size, public_key, pk_size); - if (!FIL_WriteFile(keyfile, keybuffer, totalsize)) - I_Error("Couldn't open keyfile"); - } - //------------------------------------------------ COMMAND LINE PARAMS // this must be done after loading gamedata, diff --git a/src/d_netfil.c b/src/d_netfil.c index ee686f230..5b693b59a 100644 --- a/src/d_netfil.c +++ b/src/d_netfil.c @@ -1318,12 +1318,7 @@ void PT_ClientKey(INT32 node) { clientkey_pak *packet = (void*)&netbuffer->u.clientkey; - // TODO - // Stage 1: Exchange packets with no verification of their contents - // Stage 2: Exchange packets with a check, but no crypto - // Stage 3: The crypto part (YOU ARE HERE) - - memcpy(lastReceivedKey[node], packet->key, 32); + memcpy(lastReceivedKey[node], packet->key, sizeof(lastReceivedKey[node])); netbuffer->packettype = PT_SERVERCHALLENGE; diff --git a/src/k_profiles.c b/src/k_profiles.c index 75f2e77b4..acb993414 100644 --- a/src/k_profiles.c +++ b/src/k_profiles.c @@ -18,6 +18,8 @@ #include "k_profiles.h" #include "z_zone.h" #include "r_skins.h" +#include "monocypher/monocypher.h" +#include "stun.h" // List of all the profiles. static profile_t *profilesList[MAXPROFILES+1]; // +1 because we're gonna add a default "GUEST' profile. @@ -41,6 +43,16 @@ profile_t* PR_MakeProfile( new->version = PROFILEVER; + memset(new->secret_key, 0, sizeof(secret_key)); + memset(new->public_key, 0, sizeof(public_key)); + + if (!guest) + { + static uint8_t seed[32]; + csprng(seed, 32); + crypto_eddsa_key_pair(new->secret_key, new->public_key, seed); + } + strcpy(new->profilename, prname); new->profilename[sizeof new->profilename - 1] = '\0'; @@ -238,8 +250,10 @@ void PR_SaveProfiles(void) for (i = 1; i < numprofiles; i++) { - // Names. + // Names and keys, all the string data up front WRITESTRINGN(save.p, profilesList[i]->profilename, PROFILENAMELEN); + WRITESTRINGN(save.p, profilesList[i]->public_key, sizeof(((profile_t *)0)->public_key)); + WRITESTRINGN(save.p, profilesList[i]->secret_key, sizeof(((profile_t *)0)->secret_key)); WRITESTRINGN(save.p, profilesList[i]->playername, MAXPLAYERNAME); // Character and colour. @@ -329,8 +343,10 @@ void PR_LoadProfiles(void) // Version. (We always update this on successful forward step) profilesList[i]->version = PROFILEVER; - // Names. + // Names and keys, all the identity stuff up front READSTRINGN(save.p, profilesList[i]->profilename, PROFILENAMELEN); + READSTRINGN(save.p, profilesList[i]->public_key, sizeof(((profile_t *)0)->public_key)); + READSTRINGN(save.p, profilesList[i]->secret_key, sizeof(((profile_t *)0)->secret_key)); READSTRINGN(save.p, profilesList[i]->playername, MAXPLAYERNAME); // Character and colour. @@ -550,3 +566,10 @@ profile_t *PR_GetPlayerProfile(player_t *player) return NULL; } + +profile_t *PR_GetLocalPlayerProfile(INT32 player) +{ + if (player >= MAXSPLITSCREENPLAYERS) + return NULL; + return PR_GetProfile(cv_lastprofile[player].value); +} \ No newline at end of file diff --git a/src/k_profiles.h b/src/k_profiles.h index f0ae7e287..16af39687 100644 --- a/src/k_profiles.h +++ b/src/k_profiles.h @@ -59,6 +59,9 @@ struct profile_t // Profile header char profilename[PROFILENAMELEN+1]; // Profile name (not to be confused with player name) + uint8_t public_key[32]; // Netgame authentication + uint8_t secret_key[64]; // TODO: Is it a potential vuln to have keys in memory? + // Player data char playername[MAXPLAYERNAME+1]; // Player name char skinname[SKINNAMESIZE+1]; // Default Skin @@ -156,6 +159,8 @@ SINT8 PR_ProfileUsedBy(profile_t *p); profile_t *PR_GetPlayerProfile(player_t *player); +profile_t *PR_GetLocalPlayerProfile(INT32 player); + #ifdef __cplusplus } // extern "C" #endif diff --git a/src/p_saveg.c b/src/p_saveg.c index fdbcb48de..3781925ca 100644 --- a/src/p_saveg.c +++ b/src/p_saveg.c @@ -407,6 +407,8 @@ static void P_NetArchivePlayers(savebuffer_t *save) WRITEUINT8(save->p, players[i].sliptideZipDelay); WRITEUINT16(save->p, players[i].sliptideZipBoost); + WRITESTRINGN(save->p, players[i].public_key, 32); + // respawnvars_t WRITEUINT8(save->p, players[i].respawn.state); WRITEUINT32(save->p, K_GetWaypointHeapIndex(players[i].respawn.wp)); @@ -787,6 +789,8 @@ static void P_NetUnArchivePlayers(savebuffer_t *save) players[i].sliptideZipDelay = READUINT8(save->p); players[i].sliptideZipBoost = READUINT16(save->p); + READSTRINGN(save->p, players[i].public_key, 32); + // respawnvars_t players[i].respawn.state = READUINT8(save->p); players[i].respawn.wp = (waypoint_t *)(size_t)READUINT32(save->p);