Add Argon2 KDF support. #9444

ckalina · 2019-07-23T16:56:28Z

Checklist

tests are added or updated

Hello,

this is an initial RFC pull request aiming at getting Argon2 support in OpenSSL. Currently only single threaded Argon2 mode is enforced. I tried to keep as close to reference implementation (which is dual licensed under the CC0 License and the Apache 2.0 License and as such should be compatible with OpenSSL) as possible to aid in future backports.

I'd appreciate any feedback you could provide (especially regarding threading).

Related: #4091

Thank you!

Best regards,
Cestmir

mattcaswell

Started to look at this, but ran out of time. Here's the comments I had so far.

crypto/argon2/argon2.c

kroeckx · 2019-07-23T17:26:36Z

Can you clarify why you think we should add this to OpenSSL? We only try to add new algorithms that have been standardized, or where we see a clear need from the community.

mbroz · 2019-07-23T17:48:36Z

For some discussion read issue #4091.

The RFC draft is active, authors want to publish it (see P-H-C/phc-winner-argon2/issues/271) and the last process state change ftom the last week.

We use Argon2 in disk encryption LUKS2 format, that format is actually default in several distros already (Fedora, Debian, Arch, RHEL8, ...).
Our default crypto backend is OpenSSL and using bundled Argon2 library (or separate libargon2) is just a workaround, I would definitely prefer to rely on the OpenSSL internal implementation.

kroeckx · 2019-07-23T19:19:46Z

I'm not sure how to best go with thread support. We currently have knowledge of threads, but it's very limited. We currently don't do have support for creating our own threads. If thread support is needed, we should add support for starting and stopping threads, and have the KDF argon2 implementation start and stop the thread itself. But I assume that is just a minor detail we can deal with later after the rest of the code is there and we can just run it sequentially at the start.

mbroz · 2019-07-23T19:45:26Z

The thread support is one of the "cost" parameter of the KDF algorithm, I am afraid the function is not really usable without supporting native threads. (Yes, it supports sequential execution, but then it will take much longer time if thread cost is set.)

For Argon the threading support should be quite simple - it needs to create "thread cost" number of threads, then it synchronously performs the parallel caclulation and all threads are joined.

The original libargon in fact includes simple portable wrapper around posix and windows threads, I guess something like that could be used here.

kroeckx · 2019-07-24T01:38:10Z

We also support platforms that don't support threads, so something will need to be written for that case anyway.

mattcaswell · 2019-07-24T08:34:48Z

We also support platforms that don't support threads, so something will need to be written for that case anyway.

Or possibly we just don't support Argon2 on such a platform

t8m · 2019-07-24T09:38:42Z

Another option in regards to threads would be to have the calling application setup the threads itself and call something like PerformThreadWork() function from them. This would avoid having to add thread library calls from OpenSSL, however it would make the use of Argon from applications much more complicated.

t-j-h · 2019-07-25T08:55:15Z

Note this is submission of code where the copyright is not owned by the submitter.
-1 blocker as per our CLA policy.

Thanks,

ckalina · 2019-07-25T09:08:30Z

Thank you all for expressing your views and comments! I will force push the minimal changes requested in a short while, as well as try to sort out the appropriate ICLA.

That said, I would be delighted if we could come to a compromise regarding thread usage in Argon2 -- this is really the crucial bit of Argon2 design and the family of algorithms suffers significant performance drawbacks without threading enabled.

The reference implementation comes with threading enabled and it is a small matter backporting them to OpenSSL, using appropriate OpenSSL-provided thread wrappers. OpenSSL already compiles (at least on GNU/Linux) with the appropriate flags, so either of these approaches poses no change to build process as such (can't speak for Windows just yet). So far, two proposals were made in terms of threading:

calling application itself sets up threads
add thread management (creation, joining, termination, ...) to OpenSSL itself

While I have no issue with implementing the either of the two approaches myself as part of this backport, it doesn't fall upon me to determine which of these are ideal for OpenSSL design-wise; I'd be very grateful if we could start to discuss which way ought to be implemented (or if this is a topic more appropriate for a mailing list, we can discuss it there).

Thank you!
Cestmir

mattcaswell · 2019-07-25T10:19:46Z

How about:

OpenSSL does its own thread management, with an option of setting a callback to enable the application to override it and the supply the threads directly

paulidale · 2019-07-25T10:23:24Z

suffers significant performance drawbacks without threading enabled

Isn't this the entire point of KDFs? 😁

mbroz · 2019-07-25T10:43:37Z

suffers significant performance drawbacks without threading enabled

Isn't this the entire point of KDFs? grin

Not if an attacker can use faster threaded version 🙄
Also as an additional benefit, we can now generate out-of-memory if memory-hard cost is too high! (joking, but unfortunately it happens sometimes) 😁

mbroz · 2019-07-26T08:45:16Z

Note this is submission of code where the copyright is not owned by the submitter.
-1 blocker as per our CLA policy.

@t-j-h So the original authors should send ICLA now, or what is the exaxt process? I can try to contact them...

t-j-h · 2019-07-26T09:30:24Z

We need ICLAs at least from the copyright owners (which are the original authors) - see https://www.openssl.org/policies/cla.html for those details.

And they need to submit the code themselves or indicate to the OMC that you are authorised to submit this code on their behalf.

For your own changes we need at least an ICLA and if you are employed then a CCLA.

t8m · 2019-07-26T11:58:30Z

IMO OMC should think about giving a general exception to a code that is licensed with CC0 - this license allows you to do anything with the code including licensing it under any license even a license incompatible with Apache 2.0 license.

I understand that allowing any code with compatible license without ICLA is something OMC does not want but the CC0 should be effectively the same as the authors signing the ICLA for you so that should be taken differently.

kroeckx · 2019-07-26T13:46:02Z

If something is licensed under the CC0 (or is in the public domain), or really any other license, I don't see how we can claim a different license. Maybe we can say someting like it's based on CC0 licenced software, with a link to were to find the original.

t8m · 2019-07-26T14:03:28Z

You can take the code and integrate it in any work which is licensed under any license. Yes, the parts of the code that were not modified will truly still have the CC0 license but that does not mean you have to mention that anywhere, the CC0 license does not require you to do that.

mattcaswell · 2019-07-26T14:10:41Z

You can take the code and integrate it in any work which is licensed under any license.

Exactly. CC0 is effectively public domain. You can do anything you like with it, including relicense and remove any existing comments about licensing.

Yes, the parts of the code that were not modified will truly still have the CC0 license but that does not mean you have to mention that anywhere, the CC0 license does not require you to do that.

I don't think even this is true. Once integrated into some other product under new licence terms it no longer has the old licence terms at all. Of course the original work can still be obtained under those terms - but not the integrated work.

t8m · 2019-07-26T14:24:51Z

I don't think even this is true. Once integrated into some other product under new licence terms it no longer has the old licence terms at all. Of course the original work can still be obtained under those terms - but not the integrated work.

I did not mean that the integrated work is still licensed under CC0 or that you somehow have rights to ask the copyright owner of the integrated work to extract the original CC0 work from the integrated one. I just meant that the fact that you integrate some CC0 work into something bigger under some arbitrary license does not mean that the original (unmodified parts) work somehow loses the CC0 license.

mbroz · 2019-07-29T20:17:31Z

We need ICLAs at least from the copyright owners (which are the original authors) - see https://www.openssl.org/policies/cla.html for those details.

And they need to submit the code themselves or indicate to the OMC that you are authorised to submit this code on their behalf.

For your own changes we need at least an ICLA and if you are employed then a CCLA.

@t-j-h According to email, the authors seem to agree with the ICLA, so I hope they can send it. But I am quite not sure how they can "submit the code themselves", could you please be more specific? Or send an email (where?) would be enough?

mattcaswell · 2019-07-30T09:00:44Z

@t-j-h According to email, the authors seem to agree with the ICLA, so I hope they can send it. But I am quite not sure how they can "submit the code themselves", could you please be more specific? Or send an email (where?) would be enough?

It is sufficient for them to indicate to us in some way that they agree to you submitting this code on their behalf. That could be in the email they send us with their ICLA, or by commenting on this PR (note that all the authors would need to do this).

t-j-h · 2019-07-30T09:04:09Z

If the authors provide the ICLA that is what we need - along with some indication you are approved to submit on their behalf. The form of communicating that approval isn't fixed - but the need for an ICLA from the authors under our policy is. Hopefully that is clearer.

If they include a comment when they send in their ICLA noting you will be submitting that then it makes it easier for us to match things up.

Thanks,

t-j-h · 2019-07-30T09:11:34Z

@mattcaswell for reference - public domain has a very explicit meaning - and claiming copyright on a submission makes it non-public domain. If the author's intent was to make the contribution public domain then the form in which they have done that is problematic - it actually takes care to make something public domain.

Public domain refers to items that are not protected by copyright, trademark, patent, or any other intellectual property laws - which means any statement or assertion of copyright, trademark, patent, or and other IP law defeats that intent.

The explicit claim of copyright in the LICENSE file works against the reference implementation actually being in the public domain. It might be the authors intent - but it isn't the effect of the license as stated.

paulidale · 2019-07-30T09:19:27Z

~~My understanding is that anyone can use public domain property without hassle. This seems to fit the spirit of the project well.~~

As far as I understand, nobody can claim copyright over anything in the public domain. Of course, the USA has bizarre IP laws.

mbroz · 2019-08-04T08:54:59Z

@mattcaswell @t-j-h I hope all ICLA forms were sent, do we still missing anything from the CLA side? (If anything is missing for @ckalina-rh as a submitter, it will be fixed promptly as we can talk internally in Red Hat.)

mattcaswell · 2019-08-05T09:59:09Z

@mbroz - I sent you and email with the current CLA status, i.e. we're still missing one CLA from the original authors. We also need the CLA for @ckalina-rh.

crypto/argon2/blake2b.c

ckalina · 2019-10-09T20:54:03Z

@levitte These thread*.[ch] files were previously located in crypto/thread_*.c and include/openssl/crypto.h, however, I found that to be really cumbersome. At first, I explored the possibility of moving them to a subfolder (say, crypto/thread; much like it's now in async), however, as async is somewhat related I opted to keep them in one place. Would you prefer I put them back/move to their own subfolder?

ckalina · 2019-10-09T21:03:36Z

@levitte As for moving the whole of Argon2 to providers. It is certainly possible. My reasons for keeping it separate were:

I am planning to include several optimized versions after the generic Argon2 implementation is merged. Currently, the KDF providers are all (if I remember correctly, please correct me if I am wrong) restricted to a single file. Distinct files would make it easier to navigate.
There could be multiple providers (MAC, KDF; all just wrappers on top of crypto/argon2).

What do you think?

mbroz · 2020-01-17T13:47:07Z

Hi,
@levitte could we please get some guidance here? I think it is up to OpenSSL maintainers now to say which way it should be implemented (for me, Cestmir's arguments make sense and I know he have already patform-optimized code ready, so it is not just a plain question)).

For the Argon2 RFC, draft already moved to next step, I think just final minor change review are blocking the standard to become final, see https://mailarchive.ietf.org/arch/msg/cfrg/5JvKGs91HEO3kXqbck5W6Itz1SQ

mattcaswell · 2020-01-17T14:20:51Z

Throwing in my 2 cents and responding to a couple of the comments above:

These thread*.[ch] files were previously located in crypto/thread_*.c and include/openssl/crypto.h, however, I found that to be really cumbersome. At first, I explored the possibility of moving them to a subfolder (say, crypto/thread; much like it's now in async), however, as async is somewhat related I opted to keep them in one place. Would you prefer I put them back/move to their own subfolder?

I really don't think they belong in the async directory. I'd prefer a new crypto/thread sub-directory.

As for moving the whole of Argon2 to providers...

To me moving it into providers in the long term makes sense. Algorithms are currently implemented under "crypto/", and the provider implementations under "provider/implementations" where the latter use the code in the former. The reason for this structure is historic. Prior to OpenSSL 3.0 all the algorithms were implemented in libcrypto. Now we are moving all of that code to providers. The Argon2 implementation is somewhat different in that it is the first new crypto implementation which was never in the old libcrypto.

However I don't think it right to hold this PR up while we figure out how we should be structuring things moving forwards. That really should be the subject of a different PR. Therefore I'd suggest that, for now at least, all the low-level code should remain under crypto.

Some algorithms (e.g., Argon2 KDF) are designed to be executed in parallel. This patch introduces support for threading to OpenSSL. Threading support is implemented in two ways: - Internal threading: OpenSSL creates threads itself - External threading: user-application spawns workers for OpenSSL to use (POSIX threads only) Threading must be explicitly enabled by the user application. Support for signal masking is also introduced. Signed-off-by: Čestmír Kalina <ckalina@redhat.com>

Blake2 message digest, unlike MAC, does not allow for a variable sized output. Allow for an empty-key blake2 MAC. To be used within Argon2 implementation. Signed-off-by: Čestmír Kalina <ckalina@redhat.com>

Signed-off-by: Čestmír Kalina <ckalina@redhat.com>

ckalina · 2020-02-07T08:59:04Z

Status overview:

Preemptive multitasking support moved from sync to crypto/thread.
Argon2 provider interface changed to mirror interim master branch changes.
[TBD] Two distinct approaches to preemptive threading were implemented. Which approach (if any, or both?) will be adopted by OpenSSL? The fundamental difference between the two propsed approaches is: OpenSSL does thread creation itself x OpenSSL is given a pool of workers started outside of OpenSSL, doesn't create any further threads.
[FutureWork] Argon2 remains in crypto/argon2. The whole of implementation to be moved to providers (fully) in a follow up PR.
[FutureWork] In a follow up MR, provide aarch64 optimized Argon2 variant.

With that, I'd like to re-open the discussion of threading support that this patchset requires/introduces -- specifically which approach will land in OpenSSL (I have no objections to using both of them/one or the other) -- and I'd be very grateful for your comments. We already had a good discussion regarding the pros and cons of the two approaches above, but nothing final.

Thank you!

richsalz · 2020-02-07T13:41:02Z

Since the consensus is to focus on FIPS for 3.0, is it likely this will be happening for the next release?

ckalina · 2020-02-07T15:47:28Z

@richsalz code-wise Argon2 as such is ready to be merged, there are no outstanding requests for changes. One outstanding thing to resolve is centered around threading, as that was the primary concern expressed in the discussion above. To recap [1], two different conceptual approaches arose and both of them were implemented; so all that remains is to decide which do we want in OpenSSL or whether we wish to use both. So either one needs to be removed from the code, or we can simply support both in which case there is no action required.

I would be very glad if there'd be time enough to resolve this last point to have it upstreamed,so that I could follow up with aarch64 optimization PR.

[1] One mechanism leaves OpenSSL free to create threads, the other relies on externally provided workers. Neither is enabled by default and requires explicit calls to enable. If both are enabled, the former is the preferred method.

ckalina · 2020-03-03T15:36:37Z

[TBD] Two distinct approaches to preemptive threading were implemented. Which approach (if any, or both?) will be adopted by OpenSSL? The fundamental difference between the two propsed approaches is: OpenSSL does thread creation itself x OpenSSL is given a pool of workers started outside of OpenSSL, doesn't create any further threads.

Has there been any development on this, please? Is there anything that's in your opinion missing or anything to rework?

Thanks!

t8m · 2020-03-03T15:51:53Z

@levitte @mattcaswell ping - I would really like to move forward with this PR but we need the decision on the approach for thread creation. My personal preference is to leave it up to OpenSSL to create the threads as that is much simpler and avoids needs for new API calls. Of course on platforms without thread support it will fallback to linear computation.

mattcaswell

I have started to review this, but ran out of time. My detailed comments so far are below.

I am confused by all the ASYNC plumbing in the threading code which doesn't seem necessary (or I don't understand what you are attempting to do). I also think this PR needs to be split into two. The underlying threading code needs to be pulled out as a separate PR, so that we can properly focus on just that (and the review of that code may involve different people to those that review the argon2 code). My suspicion is that once we have an agreement on the underlying threading API, getting agreement on the argon2 bit should be relatively straight forward.

In general I like the idea of having both threading models available to end users. I need to think more about the specific details of how it is implemented - and seeing that pulled out as a separate PR will help with that. I also think we need to see much more testing focused just on the threading aspect. I foresee problems if we don't get this right.

mattcaswell · 2020-03-03T15:42:26Z

crypto/argon2/argon2_api.c

+
+void ossl_dealloc(uint8_t *memory, size_t bytes)
+{
+    if (bytes)


bytes > 0

mattcaswell · 2020-03-03T15:45:05Z

crypto/argon2/argon2_api.c

+    context->out = OPENSSL_zalloc(context->outlen+1);
+    if (context->out == NULL) {
+        return ARGON2_MEMORY_ALLOCATION_ERROR;
+    }


Don't use {} for single line ifs. Here and numerous other instances below.

mattcaswell · 2020-03-03T15:47:34Z

crypto/argon2/argon2_api.c

+    }
+
+    /* 3. Initialization: Hashing inputs, allocating memory, filling first
+     * blocks. */


Use multi-line comment style:

/* * Comment here line 1 * Comment line 2 */

mattcaswell · 2020-03-03T15:49:03Z

include/internal/argon2.h

+    argon2_type type;
+
+    uint32_t flags; /* array of bool options */
+} argon2_context;


Is this intermediate type necessary? Why not just:

typedef struct { ... } ARGON2_CTX;

mattcaswell · 2020-03-03T15:52:33Z

include/internal/argon2.h

+  Argon2_d = 0,
+  Argon2_i = 1,
+  Argon2_id = 2
+} argon2_type;


We're quite inconsistent about enum types, but I personally prefer uppercase: ARGON2_TYPE

mattcaswell · 2020-03-03T17:03:54Z

crypto/thread/thread_api.c

+int CRYPTO_THREAD_exit(CRYPTO_THREAD_RETVAL retval)
+{
+    if (CRYPTO_THREAD_EXTERN_enabled == 1 && ASYNC_is_capable()) {
+        CRYPTO_THREAD_INTERN_exit(retval);


I think you mean CRYPTO_THREAD_EXTERN_exit() here?

mattcaswell · 2020-03-03T17:07:33Z

crypto/thread/thread_external.c

+
+    task = (CRYPTO_THREAD_TASK*)arg;
+
+    if ((currjob = ASYNC_get_current_job()) == NULL)


Why store this in currjob if we never then refer to currjob again?

mattcaswell · 2020-03-03T17:17:47Z

crypto/thread/thread_external.c

+        return 0;
+
+    ASYNC_block_pause();
+    task->retval = task->task(task->data);


I am confused by what you are attempting to achieve with all this ASYNC code. You seem to spend a lot of effort setting up an ASYNC job, only to then block pauses during it, which defeats the object. Why do you need to do anything with ASYNC at all?

mattcaswell · 2020-03-03T17:19:46Z

crypto/thread/thread_external.c

+static struct list    CRYPTO_THREAD_EXTERN_task_done;
+static CRYPTO_MUTEX   CRYPTO_THREAD_EXTERN_task_lock;
+static CRYPTO_CONDVAR CRYPTO_THREAD_EXTERN_task_cond_create;
+static CRYPTO_CONDVAR CRYPTO_THREAD_EXTERN_task_cond_finish;


All of these global variables should be stored in the OPENSSL_CTX instead, and then we should pass the OPENSSL_CTX to the various thread functions that need it.

mattcaswell · 2020-03-03T17:26:39Z

test/evp_kdf_test.c

@@ -160,6 +160,41 @@ static int test_kdf_pbkdf2(void)
    return ret;
 }

+#ifndef OPENSSL_NO_ARGON2
+static int test_kdf_argon2(void)


Testing seems very light. In particular I would expect to see separate tests for all the new threading plumbing that this PR puts in.

richsalz · 2020-03-03T17:35:58Z

crypto/argon2/argon2_api.c

+
+    *memory = OPENSSL_zalloc(bytes);
+
+    return memory != NULL;


shouldn't that be "*memory" ?

kroeckx · 2020-03-03T17:41:33Z

My preference is also that OpenSSL creates the threads itself.

richsalz

some real things, many nits. not fully reviewed.

richsalz · 2020-03-03T17:36:48Z

crypto/argon2/argon2_api.c

+    if (datalen > UINT32_MAX)
+        return 0;
+
+    if (data) {


explicit NULL test

richsalz · 2020-03-03T17:37:18Z

crypto/argon2/argon2_api.c

+        return result;
+    }
+
+    if (Argon2_d != context->type && Argon2_i != context->type &&


linebreak before the conditional. indent the second line an extra shift-width.

richsalz · 2020-03-03T17:39:04Z

crypto/argon2/argon2_api.c

+    /* 3. Initialization: Hashing inputs, allocating memory, filling first
+     * blocks. */
+    result = initialize(&instance, context);
+


should really get rid of the extra blank line here, so things look like:

foo = calculate... if (ARGON2_OK != foo) ...

The "constantvariable" construct is very uncommon in openssl code.

richsalz · 2020-03-03T17:39:56Z

crypto/argon2/argon2_api.c

+
+int ARGON2_Final(uint8_t *md, ARGON2_CTX *c)
+{
+    if (md == NULL || c == NULL) {


In general OpenSSL doesn'st test for NULL parameters. This is not a universal opinion, others will have to comment.

richsalz · 2020-03-03T17:40:46Z

crypto/argon2/blake2b.c

+{
+    int ret = 0;
+
+    EVP_MAC *mac;


initialize these variables to NULL then you can have just a single "goto" label.

richsalz · 2020-03-03T17:43:27Z

crypto/argon2/blake2b.h

+    if (is_endian.little) {
+        uint64_t w;
+        memcpy(&w, src, sizeof(w));
+        return w;


same as above.

richsalz · 2020-03-03T17:44:22Z

crypto/argon2/blake2b.h

+        uint8_t *p = (uint8_t *)dst;
+        int i;
+
+        for (i = 0; i < 4; i++)


is "4" (and the "8" below) right? I guess so, but it looks ... not quite portable.

richsalz · 2020-03-03T17:44:56Z

crypto/argon2/blake2b.h

+    return x + y + 2 * xy;
+}
+
+# ifndef G


you use about the ifndef ? Maybe just undef if it already exists?

richsalz · 2020-03-03T17:45:46Z

crypto/argon2/core.h

@@ -0,0 +1,225 @@
+/*
+ * Argon2 reference source code package - reference C implementations
+ *


Presumably this has changes to fit into openssl. The copyright should look like other contribute-code copyrights.

richsalz · 2020-03-03T17:46:20Z

crypto/argon2/core.h

+
+/*************************Argon2 core functions********************************/
+
+/* Allocates memory to the given pointer, uses the appropriate allocator as


See @mattcaswell's comment about multi-line comments.

richsalz · 2020-03-03T17:47:59Z

If OpenSSL is going to create the threads, then an application should be able to specify a limit so that it doesn't get DoS'd.

kroeckx · 2020-03-03T18:15:17Z

If OpenSSL is going to create the threads, then an application should be able to specify a limit so that it doesn't get DoS'd.

If we go that way, we need to add rate limits for EVP_* calls too. I don't think it's up to the library to do that.

richsalz · 2020-03-03T18:19:32Z

If we go that way, we need to add rate limits for EVP_* calls too.

I don't understand this.

ckalina · 2020-06-24T09:25:22Z

This MR has been split into #12255 and #12256.
Notably, support for OpenSSL CTX introduced, maximal limit on in-OpenSSL created threads added. Aside that, Argon2 moved to providers and threading was also slightly reworked. Closing as there are new PRs.

openssl-machine added the hold: cla required The contributor needs to submit a license agreement label Jul 23, 2019

mattcaswell requested changes Jul 23, 2019

View reviewed changes

crypto/argon2/argon2.c Outdated Show resolved Hide resolved

crypto/argon2/argon2.c Outdated Show resolved Hide resolved

crypto/argon2/argon2.c Outdated Show resolved Hide resolved

crypto/argon2/argon2.c Outdated Show resolved Hide resolved

t-j-h added the hold: need omc decision The OMC needs to make a decision label Jul 25, 2019

FdaSilvaYY reviewed Aug 6, 2019

View reviewed changes

ckalina added 2 commits February 7, 2020 00:53

Blake2 empty-key MAC support

b28c534

Blake2 message digest, unlike MAC, does not allow for a variable sized output. Allow for an empty-key blake2 MAC. To be used within Argon2 implementation. Signed-off-by: Čestmír Kalina <ckalina@redhat.com>

ckalina force-pushed the argon2 branch from 71ac960 to 2591a9e Compare February 6, 2020 23:58

Introduce Argon2 KDF

9204f65

Signed-off-by: Čestmír Kalina <ckalina@redhat.com>

ckalina force-pushed the argon2 branch from 2591a9e to 9204f65 Compare February 7, 2020 00:27

mbroz mentioned this pull request Feb 7, 2020

Any plans to RFC document? P-H-C/phc-winner-argon2#271

Open

SparkDustJoe mentioned this pull request Feb 18, 2020

V5 [Proposal only] keybase/triplesec#72

Closed

mattcaswell requested changes Mar 3, 2020

View reviewed changes

richsalz reviewed Mar 3, 2020

View reviewed changes

crypto/argon2/argon2_api.c

*memory = OPENSSL_zalloc(bytes);

return memory != NULL;

Copy link

Contributor

richsalz Mar 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't that be "*memory" ?

richsalz reviewed Mar 3, 2020

View reviewed changes

ranisalt mentioned this pull request Mar 20, 2020

Why is argon2id not the default? ranisalt/node-argon2#252

Closed

t8m added the branch: master Merge to master branch label Apr 20, 2020

ckalina mentioned this pull request Jun 24, 2020

KDF: Introduce Argon2i, Argon2d, Argon2id #12256

Closed

2 tasks

ckalina closed this Jun 24, 2020

johndoe31415 mentioned this pull request Jul 21, 2020

Support for Argon2 KDF #4091

Closed

ckalina deleted the argon2 branch March 15, 2022 00:15


		task = (CRYPTO_THREAD_TASK*)arg;

		if ((currjob = ASYNC_get_current_job()) == NULL)


		/***********************Argon2 core functions******************************/

		/* Allocates memory to the given pointer, uses the appropriate allocator as

Add Argon2 KDF support. #9444

Add Argon2 KDF support. #9444

Conversation

ckalina commented Jul 23, 2019

Checklist

mattcaswell left a comment

Choose a reason for hiding this comment

kroeckx commented Jul 23, 2019

mbroz commented Jul 23, 2019

kroeckx commented Jul 23, 2019

mbroz commented Jul 23, 2019

kroeckx commented Jul 24, 2019

mattcaswell commented Jul 24, 2019

t8m commented Jul 24, 2019

t-j-h commented Jul 25, 2019

ckalina commented Jul 25, 2019

mattcaswell commented Jul 25, 2019

paulidale commented Jul 25, 2019

mbroz commented Jul 25, 2019

mbroz commented Jul 26, 2019

t-j-h commented Jul 26, 2019

t8m commented Jul 26, 2019

kroeckx commented Jul 26, 2019 via email

t8m commented Jul 26, 2019

mattcaswell commented Jul 26, 2019

t8m commented Jul 26, 2019

mbroz commented Jul 29, 2019

mattcaswell commented Jul 30, 2019

t-j-h commented Jul 30, 2019

t-j-h commented Jul 30, 2019

paulidale commented Jul 30, 2019 • edited

mbroz commented Aug 4, 2019

mattcaswell commented Aug 5, 2019

ckalina commented Oct 9, 2019 • edited

ckalina commented Oct 9, 2019

mbroz commented Jan 17, 2020

mattcaswell commented Jan 17, 2020

ckalina commented Feb 7, 2020 • edited

richsalz commented Feb 7, 2020

ckalina commented Feb 7, 2020

ckalina commented Mar 3, 2020

t8m commented Mar 3, 2020

mattcaswell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kroeckx commented Mar 3, 2020 via email

richsalz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richsalz commented Mar 3, 2020

kroeckx commented Mar 3, 2020 via email

richsalz commented Mar 3, 2020

ckalina commented Jun 24, 2020

paulidale commented Jul 30, 2019 •

edited

ckalina commented Oct 9, 2019 •

edited

ckalina commented Feb 7, 2020 •

edited