SRP6 calculating M1, M2 incorrectly

[jimm98y]

I was trying to use BouncyCastle SRP6 on the server side for HomeKit and I found out that I can only generate B and S, but then M1 and M2 are calculated in BouncyCastle like:

M1 = H(A | B | S)
M2 = H(A | M1 | S)
K = H(S)

While they should be calculated as follows according to RFC2945:

K = H(S)
M1 = H(H(N) XOR H(g) | H(I) | s | A | B | K)
M2 = H(A | M1 | K)

In C#, I ended up using the following code:

private SecureRandom _random = new SecureRandom();
private readonly IDigest _digest = new Sha512Digest();
private Srp6Server _server;
private byte[] _N;
private byte[] _g;
private byte[] _s;
private byte[] _I;
private byte[] _B;
private byte[] _calculatedM1;
private byte[] _K;
private byte[] _A;

public byte[] GenerateSalt()
{
    byte[] s = new byte[16];
    _random.NextBytes(s);
    return s;
}

public byte[] GenerateServerCredentials(byte[] bN, byte[] bg, byte[] salt, string userName, string password)
{
    _N = bN;
    _g = bg;
    _s = salt;
    _I = Encoding.UTF8.GetBytes(userName);
    
    byte[] P = Encoding.UTF8.GetBytes(password);
    BigInteger N = new BigInteger(1, _N);
    BigInteger g = new BigInteger(1, _g);
    Srp6GroupParameters group = new Srp6GroupParameters(N, g);
    Srp6VerifierGenerator gen = new Srp6VerifierGenerator();

    gen.Init(group, _digest);

    BigInteger v = gen.GenerateVerifier(_s, _I, P);
    _server = new Srp6Server();
    _server.Init(group, v, _digest, _random);

    BigInteger B = _server.GenerateServerCredentials();
    _B = B.ToByteArrayUnsigned();
    return _B;
}

public bool VerifyClientEvidenceMessage(byte[] A, byte[] clientM1)
{
    //  BouncyCastle is using simplified calculation of M1 and M2 using only A, B and S.
    //_ = _server.CalculateSecret(new BigInteger(1, A));
    //return _server.VerifyClientEvidenceMessage(new BigInteger(1, clientM1));

    // let's calculate it using the correct formula: M1 = H(H(N) XOR H(g) | H(I) | s | A | B | K)
    _A = A;
    BigInteger S = _server.CalculateSecret(new BigInteger(1, A));
    _K = SHA(S.ToByteArrayUnsigned());
    _calculatedM1 = SHA(CONCAT(XOR(SHA(_N), SHA(_g)), CONCAT(SHA(_I), CONCAT(_s, CONCAT(A, CONCAT(_B, _K))))));
    bool result = _calculatedM1.SequenceEqual(clientM1);
    return result;
}

public byte[] CalculateServerEvidenceMessage()
{
    //return _server.CalculateServerEvidenceMessage().ToByteArrayUnsigned();
    return SHA(CONCAT(_A, CONCAT(_calculatedM1, _K)));
}

public byte[] CalculateSessionKey()
{
    //return _server.CalculateSessionKey().ToByteArrayUnsigned();   
    return _K;
}

private static byte[] CONCAT(byte[] a, byte[] b)
{
    return a.Concat(b).ToArray();
}

private byte[] SHA(byte[] bytes)
{
    _digest.Reset();
    _digest.BlockUpdate(bytes, 0, bytes.Length);
    byte[] rv = new byte[_digest.GetDigestSize()]; 
    _digest.DoFinal(rv, 0);
    return rv;
}

private static byte[] XOR(byte[] a, byte[] b)
{
    for (int i = 0; i < a.Length; i++)
    {
        a[i] ^= b[i];
    }
    return a;
}

This issue is not just related to C#, I can see the same code in Java as well. The current BouncyCastle implementation is working only when BouncyCastle is being used on both sides - client and the server.

[peterdettman]

Our implementation is for RFC 5054.

[jimm98y]

Yes, but RFC 5054 it does not specify the algorithm for calculating M1 and M2, or at least I haven't found it there. It delegates M1/M2 to RFC 2945 and it just states that failing to decrypt the finished message performs the same function as M1/M2 in RFC 2945. If I understand it correctly, M1/M2 are not even being used in RFC 5054, but BouncyCastle has the methods for calculating them. And I haven't found anything related to the calculation of M1/M2 using the algorithm currently implemented by BouncyCastle:

M1 = H(A | B | S)
M2 = H(A | M1 | S)

May I ask where does it come from, if not from RFC 5054/RFC 2945?

[cipherboy]

@jimm98y I was able to hunt this down a little more. It looks like in 1999ish (according to here -- though other commentary points to a later date, perhaps 2002+), Thomas Wu proposed the SRP protocol with computation discussed here. Before (or perhaps after) standardization though (by the same author), it was changed to the version currently present in RFC 2945 (linked above), also sometimes called SRP-3 (perhaps?).

Wikipedia gives this clue about the introduction of our algorithm:

Alternatively, in a password-only proof the calculation of K can be skipped and the shared S proven with:

(with algorithm matching what is in Bouncy Castle, without citations).

The JavaDocs of a third-party package give us a clue as to citation for the reduced form:

The evidence messages 'M1' and 'M2' are computed according to Tom Wu's paper "SRP-6: Improvements and refinements to the Secure Remote Password protocol", table 5, from 2002.

Indeed, reading RFC 5054 linked by Peter above says:

The version of SRP used here is sometimes referred to as "SRP-6"
[SRP-6]. This version is a slight improvement over "SRP-3", which
was described in [SRP] and [SRP-RFC]. For convenience, this document
and [SRP-RFC] include the details necessary to implement SRP-6;
[SRP-6] is cited for informative purposes only.

Though, notably SRP-6 cites what is, presumably, the same paper mentioned by the JavaDocs and the Stanford link above. And, note my bolding: SRP-RFC links to RFC 2945. I believe Peter is thus correct and RFC 5054 is poorly cited/written: it is not RFC 2945 compatible and should be as implemented here in BC. This also matches with the file naming (SRP6).

(However, I do agree, RFC 5054 is very unclear about these differences and lacks the information it states it should have.)

Thus, you would be correct (as per your PR) in proposing alternative interfaces while retaining compatibility for existing users.

---

However, it (incorrectly) looks like there might be a third variant:

K = H(S)
M1 = H(A | B | K)

as discussed in this rust crate: https://docs.rs/srp/latest/srp/ -- though no source for this variant is mentioned on the README...

As far as I can tell, this variant isn't used in the code base: https://github.com/RustCrypto/PAKEs/blob/master/srp/src/server.rs#L149-L155 does not seem to include K = H(S) and instead includes S directly, aligning with Bouncy Castle's and RFC 5054's implementation. So, this crate should be compatible with our code.

[jimm98y]

Wow, nice work! Thank you very much for this research, it's really interesting to see how many variants of SRP are out there. Digging a little bit more into HAP-nodejs revealed they also had to modify an existing implementation to make it HAP (RFC 2945) compatible: zarmack/fast-srp@24a32ff