General response, as I don't know the details of the .Net cryptography libraries.
Like I said: I don't care about protocol encryption, only secure authentication.
At least I know that there's no way in hell anyone can get a user's password, or even hash, because now it isn't being sent across.
What do you mean "can he read data on your client"?
From what you've mentioned, it sounds like your threat model is an attacker trying to steal client credentials:
- Passively listening on the network
- Actively interfering with the network, e.g. a man in the middle
You don't appear to be trying to solve the problem where the attacker has compromised the server or the client itself, or where the user is socially engineered by a determined attacker. Is that accurate?
I'm afraid your code would fail review based on the first comment:
Not sure why, but both keys must derive from the same master for decryption to work.
On the other hand, at least it is honest!
I'd be worried about your use of GUIDs for nonces and challenges - they are not cryptographically secure.
The server appears to require the client's private key, which probably means you haven't thought through how this will work in practise.
It isn't clear if you're doing much basic input validation. I cannot see the ProcessedPacket class, but it looks like in places you're willing to pass values read from the network directly into array allocations. In the cases I saw, these values they are bytes, so there isn't likely to be a serious DOS problem. However, if you are doing that for multi-byte values anywhere (ReadPascalString, perhaps?) you might allow a malicious client to make you to do an awful lot of work just by sending packets with large usernames / GUID strings.
And for comments purely on the code, you appear to have mixed client and server specific code together, which makes everything quite confusing. It is difficult to remember which variables and functions are going to be used by which peer, and you're doing unnecessary work initialising some variables that won't (or at least, shouldn't) be used. The common code could be moved to a helper class, and then the client and server can have their own classes. The code is heavily tied to the network, which complicates testing.
Ideally, you might be able to write test cases in isolation (hypothetical code):
public void testStuff()
{
byte [] serverKey = MatskCrypto.loadPrivateKey("server.key");
MatskServer server = new MatskServer(serverKey);
string storedPassword = MatskAuth.hashWithSalt("top_secret", "delicious_salt");
server.addClientCredentials("somebody@example.com", storedPassword);
byte [] clientKey = MatskCrypto.loadPrivateKey("client.key");
MatskClient client = new MatskClient(clientKey);
byte [] validCipherText = client.prepareAuthenticationRequest("somebody@example.com", "top_secret");
assertTrue(server.verifyAuthenticationRequest(cipherText));
byte [] badUsername = client.prepareAuthenticationRequest("nobody@example.com", "top_secret");
assertFalse(server.verifyAuthenticationRequest(cipherText));
byte [] badPassword = client.prepareAuthenticationRequest("somebody@example.com", "bottom_secret");
assertFalse(server.verifyAuthenticationRequest(cipherText));
}