Skip to content

Commit 2718bb0

Browse files
committed
Release the GVL lock when hashing and verifying
To align with the bcrypt gem, release Ruby's Global VM Lock when performing the intentionally slow computations for hashing and verifying passwords. To do this, we effectively serialise the required arguments into a struct and invoke rb_thread_call_without_gvl. Note we don't pass an unblocking function when we do so because Argon2 provides no API to cancel a hash mid-computation so the only safe thing to do is to let it complete (this matches bcrypt). To prevent memory leaks if encoding raises (which would longjmp before the encoded string is freed), we wrap it in an ensure block. Note we defensively use ARGON2_MISSING_ARGS as a sentinel result value in case the rb_thread_call_without_gvl call ever returns without running our nogvl function. With this change, mark the extension as safe to use with Ractors.
1 parent 690bd96 commit 2718bb0

2 files changed

Lines changed: 140 additions & 26 deletions

File tree

ext/argon2id/argon2id.c

Lines changed: 115 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include <ruby.h>
2+
#include <ruby/thread.h>
23
#include <stdint.h>
34

45
#include "argon2.h"
@@ -8,15 +9,87 @@
89
VALUE mArgon2id, cArgon2idError, cArgon2idPassword;
910
ID id_encoded;
1011

12+
struct hash_encoded_args {
13+
uint32_t t_cost;
14+
uint32_t m_cost;
15+
uint32_t parallelism;
16+
const char *pwd;
17+
size_t pwdlen;
18+
const char *salt;
19+
size_t saltlen;
20+
uint32_t outlen;
21+
char *encoded;
22+
size_t encodedlen;
23+
int result;
24+
};
25+
26+
static void *
27+
nogvl_hash_encoded(void *data)
28+
{
29+
struct hash_encoded_args *args = data;
30+
args->result = argon2id_hash_encoded(args->t_cost, args->m_cost,
31+
args->parallelism, args->pwd, args->pwdlen, args->salt, args->saltlen,
32+
args->outlen, args->encoded, args->encodedlen);
33+
34+
return NULL;
35+
}
36+
37+
struct verify_args {
38+
const char *encoded;
39+
const char *pwd;
40+
size_t pwdlen;
41+
int result;
42+
};
43+
44+
static void *
45+
nogvl_verify(void *data)
46+
{
47+
struct verify_args *args = data;
48+
args->result = argon2id_verify(args->encoded, args->pwd, args->pwdlen);
49+
50+
return NULL;
51+
}
52+
53+
struct hash_encoded_data {
54+
char *encoded;
55+
VALUE pwd;
56+
VALUE salt;
57+
struct hash_encoded_args args;
58+
};
59+
60+
static VALUE
61+
hash_encoded_body(VALUE arg)
62+
{
63+
struct hash_encoded_data *data = (struct hash_encoded_data *)arg;
64+
65+
rb_thread_call_without_gvl(nogvl_hash_encoded, &data->args, NULL, NULL);
66+
67+
if (data->args.result != ARGON2_OK) {
68+
rb_raise(cArgon2idError, "%s", argon2_error_message(data->args.result));
69+
}
70+
71+
return rb_str_new_cstr(data->encoded);
72+
}
73+
74+
static VALUE
75+
hash_encoded_finalize(VALUE arg)
76+
{
77+
struct hash_encoded_data *data = (struct hash_encoded_data *)arg;
78+
79+
RB_GC_GUARD(data->pwd);
80+
RB_GC_GUARD(data->salt);
81+
free(data->encoded);
82+
83+
return Qnil;
84+
}
85+
1186
static VALUE
1287
rb_argon2id_hash_encoded(VALUE klass, VALUE iterations, VALUE memory, VALUE threads, VALUE pwd, VALUE salt, VALUE hashlen)
1388
{
1489
uint32_t t_cost, m_cost, parallelism, outlen;
1590
size_t encodedlen;
1691
long saltlen;
17-
char * encoded;
18-
int result;
19-
VALUE hash;
92+
struct hash_encoded_data data;
2093

2194
UNUSED(klass);
2295

@@ -41,55 +114,71 @@ rb_argon2id_hash_encoded(VALUE klass, VALUE iterations, VALUE memory, VALUE thre
41114
}
42115

43116
encodedlen = argon2_encodedlen(t_cost, m_cost, parallelism, (uint32_t)saltlen, outlen, Argon2_id);
44-
encoded = malloc(encodedlen);
45-
if (!encoded) {
117+
data.encoded = malloc(encodedlen);
118+
if (!data.encoded) {
46119
rb_raise(rb_eNoMemError, "not enough memory to allocate for encoded password");
47120
}
48121

49-
result = argon2id_hash_encoded(t_cost, m_cost, parallelism, RSTRING_PTR(pwd), RSTRING_LEN(pwd), RSTRING_PTR(salt), RSTRING_LEN(salt), outlen, encoded, encodedlen);
50-
51-
if (result != ARGON2_OK) {
52-
free(encoded);
53-
rb_raise(cArgon2idError, "%s", argon2_error_message(result));
54-
}
55-
56-
hash = rb_str_new_cstr(encoded);
57-
free(encoded);
58-
59-
return hash;
122+
data.pwd = pwd;
123+
data.salt = salt;
124+
data.args.result = ARGON2_MISSING_ARGS;
125+
data.args.t_cost = t_cost;
126+
data.args.m_cost = m_cost;
127+
data.args.parallelism = parallelism;
128+
data.args.pwd = RSTRING_PTR(pwd);
129+
data.args.pwdlen = RSTRING_LEN(pwd);
130+
data.args.salt = RSTRING_PTR(salt);
131+
data.args.saltlen = RSTRING_LEN(salt);
132+
data.args.outlen = outlen;
133+
data.args.encoded = data.encoded;
134+
data.args.encodedlen = encodedlen;
135+
136+
return rb_ensure(hash_encoded_body, (VALUE)&data, hash_encoded_finalize, (VALUE)&data);
60137
}
61138

62139
static VALUE
63140
rb_argon2id_verify(VALUE self, VALUE pwd) {
64-
int result;
65141
VALUE encoded;
142+
struct verify_args args;
66143

67144
encoded = rb_ivar_get(self, id_encoded);
68145

69-
/* Coerce encoded and freeze it before coercing pwd: StringValue(pwd) may
70-
* call #to_str which can execute arbitrary Ruby and mutate the original
71-
* @encoded string via a held reference, bypassing verification. */
146+
/* Coerce encoded and freeze it before doing the same to pwd. The order here
147+
* is important to prevent pwd#to_str mutating encoded.
148+
*/
72149
StringValueCStr(encoded);
73150
encoded = rb_str_new_frozen(encoded);
74151
StringValue(pwd);
152+
pwd = rb_str_new_frozen(pwd);
75153

76-
result = argon2id_verify(RSTRING_PTR(encoded), RSTRING_PTR(pwd), RSTRING_LEN(pwd));
77-
if (result == ARGON2_OK) {
154+
args.result = ARGON2_MISSING_ARGS;
155+
args.encoded = RSTRING_PTR(encoded);
156+
args.pwd = RSTRING_PTR(pwd);
157+
args.pwdlen = RSTRING_LEN(pwd);
158+
159+
rb_thread_call_without_gvl(nogvl_verify, &args, NULL, NULL);
160+
161+
RB_GC_GUARD(encoded);
162+
RB_GC_GUARD(pwd);
163+
164+
if (args.result == ARGON2_OK) {
78165
return Qtrue;
79166
}
80-
if (result == ARGON2_VERIFY_MISMATCH) {
167+
if (args.result == ARGON2_VERIFY_MISMATCH) {
81168
return Qfalse;
82169
}
83-
if (result == ARGON2_DECODING_FAIL || result == ARGON2_DECODING_LENGTH_FAIL) {
84-
rb_raise(rb_eArgError, "%s", argon2_error_message(result));
170+
if (args.result == ARGON2_DECODING_FAIL || args.result == ARGON2_DECODING_LENGTH_FAIL) {
171+
rb_raise(rb_eArgError, "%s", argon2_error_message(args.result));
85172
}
86173

87-
rb_raise(cArgon2idError, "%s", argon2_error_message(result));
174+
rb_raise(cArgon2idError, "%s", argon2_error_message(args.result));
88175
}
89176

90177
void
91178
Init_argon2id(void)
92179
{
180+
rb_ext_ractor_safe(true);
181+
93182
id_encoded = rb_intern("@encoded");
94183

95184
mArgon2id = rb_define_module("Argon2id");

test/argon2id/test_password.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,31 @@ def test_create_password_does_not_equal_incorrect_password
580580
refute password == "differentpassword"
581581
end
582582

583+
def test_create_is_thread_safe
584+
threads = 10.times.map do |i|
585+
Thread.new(i) do |n|
586+
password = Argon2id::Password.create("password-#{n}", t_cost: 2, m_cost: 256, parallelism: 1)
587+
assert password == "password-#{n}"
588+
end
589+
end
590+
591+
threads.each(&:value)
592+
end
593+
594+
def test_verify_is_thread_safe
595+
hash = Argon2id::Password.create("password", t_cost: 2, m_cost: 256, parallelism: 1).to_s
596+
597+
threads = 10.times.map do |i|
598+
Thread.new do
599+
password = Argon2id::Password.new(hash)
600+
assert password == "password"
601+
refute password == "wrong"
602+
end
603+
end
604+
605+
threads.each(&:value)
606+
end
607+
583608
def test_hashing_password_verifies_correct_password
584609
hash = Argon2id::Password.create("password").to_s
585610
password = Argon2id::Password.new(hash)

0 commit comments

Comments
 (0)