kmaragon / Konscious.Security.Cryptography

MIT License
216 stars 20 forks source link

Get Argon2 version number #30

Open outkst opened 5 years ago

outkst commented 5 years ago

Continuing from closed Issue #19 — Can you please provide a way to programmatically obtain the version number of the current Argon2 implementation being used? From page 20 of the Argon2 documentation, the current version 1.3 is stored as a byte in the form of 0x13, and either this would be kept the same or like some other ports of Argon2 would simply become an integer (e.g. 19).

C.1 v.1.3 • The blocks are XORed with, not overwritten in the second pass and later; • The version number byte is now 0x13.

This number would be stored inside of the database in case the algorithm is ever changed and breaks compatibility with older versions.

kmaragon commented 5 years ago

This is a very good point. Though I would argue that rather than simply returning a version byte somewhere, the library needs a facility for selecting a version and acting accordingly. i.e. a constructor parameter specifying a version to use, with the default value being the latest version. Then a getter that just returns the version that the class was constructed with would do what you're asking. I think this is an important layer of abstraction that this library is missing. I will work on a non-breaking change to do that under the hood and maybe use the opportunity to support 1.2(.x)... While of course still generating 1.3 hashes by default.

SparkDustJoe commented 5 years ago

I would argue that the library should be able to deduce what version was used to encode a string and be able to validate any version. But should only produce latest version encoded strings. Raw hashes where no information is available is more tricky, and an optional variable to specify a version may still be appropriate, again for verify only. If older versions are needed to produce on either side (raw or encoded) a legacy class marked deprecated would work to delineate the older versions... But that's just my take on the matter.

On Thu, Feb 7, 2019, 16:29 Keef Aragon <notifications@github.com wrote:

This is a very good point. Though I would argue that rather than simply returning a version byte somewhere, the library needs a facility for selecting a version and acting accordingly. i.e. a constructor parameter specifying a version to use, with the default value being the latest version. Then a getter that just returns the version that the class was constructed with would do what you're asking. I think this is an important layer of abstraction that this library is missing. I will work on a non-breaking change to do that under the hood and maybe use the opportunity to support 1.2(.x)... While of course still generating 1.3 hashes by default.

ghost commented 5 years ago

Are there any plans to support encoding/decoding (i.e. $argon2i$v=19$m=65536,t=2,p=4$c29tZXNhbHQ$RdescudvJCsgt3ub+b+dWRWJTmaaJObG)?

outkst commented 5 years ago

@mrfrankbell I think the string encoding is the eventual goal here—once a Version property is introduced. The string encoding will allow the storage of the version number, cost options, and salt alongside the hash just like in the original implementation.

$argon2i$v=19$m=65536,t=2,p=4$c29tZXNhbHQ$RdescudvJCsgt3ub+b+dWRWJTmaaJObG

The string encoding above can then be obtained in this implementation by doing something like the following:

CalculateArgon2iHash("password"); // implicit CalculateArgon2iHash("password", "somesalt", Argon2Version.Newest); // same as below CalculateArgon2iHash("password", "somesalt", 19); // same as above CalculateArgon2iHash("password", "somesalt", 19, 65536, 2, 4, 32); // explicit

// NEW ENUM
public enum Argon2Version {
    Unknown = 0,
    V13 = 19, // 0x13
    V10 = 16, // 0x10
    Newest = V13
}
public static string CalculateArgon2iHash(string plaintextPassword, byte[] salt = null, Argon2Version version = Argon2Version.Newest, int? memorySize = null, int? iterations = null, int? parallelism = null, int? hashLength = null) {
    memorySize = memorySize ?? MEMORY_SIZE;
    iterations = iterations ?? ITERATIONS;
    parallelism = parallelism ?? DEGREE_OF_PARALLELISM;
    salt = salt ?? GenerateSalt(SALT_LENGTH);
    hashLength = hashLength ?? HASH_LENGTH;

    string encodedPassword;

    using (Argon2i argon2 = new Argon2i(Encoding.UTF8.GetBytes(plaintextPassword)))
    {
        argon2.Version = version; // NEW ENUM
        argon2.MemorySize = (int)memorySize;
        argon2.Iterations = (int)iterations;
        argon2.DegreeOfParallelism = (int)parallelism;
        argon2.Salt = salt;

        encodedPassword = String.Format("${0}$v={1}$m={2},t={3},p={4}${5}${6}",
            argon2.GetType().Name.ToLower(),
            argon2.Version, // NEW ENUM
            argon2.MemorySize,
            argon2.Iterations,
            argon2.DegreeOfParallelism,
            Convert.ToBase64String(argon2.Salt),
            Convert.ToBase64String(argon2.GetBytes((int)hashLength))
        );
    }

    return encodedPassword; // $argon2i$v=19$m=65536,t=2,p=4$c29tZXNhbHQ$RdescudvJCsgt3ub+b+dWRWJTmaaJObG
}
kmaragon commented 5 years ago

I have no intention of adding support for the stringification of the argon2 hash as in the argon2 reference command-line program (as opposed to the core library). You'll also notice that the command line program doesn't support various attributes laid out in the spec... But I do have every intention of making the version number a property that can be used for someone to do so - as well as to control how the hash is generated. But generally, keep in mind that the most secure use of this (or argon2 in-general) would be to keep the pieces entirely separately.

For example, if you are generating an API Key, you could embed the Iterations directly into the API key at the edge layer in the one time you give it to the client. And then simply validate it min < x < max before plugging it into the algorithm to validate the keys. And then you could use a key service i.e. AWS KMS to assign specific KnownSecret (one of those attributes laid out in the spec that the command line program doesn't support) values to subsets of keys. Embedding these into the final hash undermine good security design without asking the engineer to go and parse out the output to extract the pieces rather than store them.

The GetBytes API is also just that - an API to get bytes. It will be up to the developer to figure out where the pieces go to reconstruct those bytes. But I also don't intend to make the above code sample impossible to implement. Nor do I plan to make it impossible to validate old hashes as the versions are updated (as it is now). So I will be adding version as well as backwards compatibility... As soon as I have time to spin back up my .NET environment.