Store keypairs in profiles, do signature verification for all splitscreen players

src/d_clisrv.c

@@ -157,8 +157,8 @@ char connectedservername[MAXSERVERNAME];
 /// \todo WORK!
 boolean acceptnewnode = true;
 
-uint8_t lastReceivedKey[MAXNETNODES][32];
+uint8_t lastReceivedKey[MAXNETNODES][MAXSPLITSCREENPLAYERS][32];
-uint8_t lastSentChallenge[MAXNETNODES][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];
+	// Don't leak old signatures from prior sessions.
-	crypto_eddsa_sign(signature, secret_key, awaitingChallenge, 32);
+	memset(&netbuffer->u.clientcfg.challengeResponse, 0, sizeof(((clientconfig_pak *)0)->challengeResponse));
 
-	if (crypto_eddsa_check(signature, public_key, awaitingChallenge, 32) != 0)
+	for (i = 0; i <= splitscreen; i++)
-		I_Error("Couldn't verify own key?");
+	{
+		uint8_t signature[64];
+		profile_t *localProfile = PR_GetLocalPlayerProfile(i);
 
-	// Testing
+		if (cv_lastprofile[0].value == 0) // GUESTS don't have keys
-	// memset(signature, 0, sizeof(signature));
+		{
+			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));

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 lastReceivedKey[MAXNETNODES][MAXSPLITSCREENPLAYERS][32];
-extern uint8_t lastSentChallenge[MAXNETNODES][32];
+extern uint8_t lastSentChallenge[MAXNETNODES][MAXSPLITSCREENPLAYERS][32];
 
 void Command_Ping_f(void);
 extern tic_t connectiontimeout;

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,

src/d_netfil.c

@@ -1318,12 +1318,7 @@ void PT_ClientKey(INT32 node)
 {
 	clientkey_pak *packet = (void*)&netbuffer->u.clientkey;
 
-	// TODO
+	memcpy(lastReceivedKey[node], packet->key, sizeof(lastReceivedKey[node]));
-	// 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);
 
 	netbuffer->packettype = PT_SERVERCHALLENGE;
 

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);
+}

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

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);