From 8cd5a66517e42298c38af12689a566973084f877 Mon Sep 17 00:00:00 2001 From: wiredfool Date: Fri, 30 Sep 2022 12:16:49 +0100 Subject: [PATCH 1/3] Use pgcrypto's random bytes for cryptographically good random numbers --- packages/totp/sql/launchql-totp--0.0.3.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/totp/sql/launchql-totp--0.0.3.sql b/packages/totp/sql/launchql-totp--0.0.3.sql index 88ad00e..c301f4d 100644 --- a/packages/totp/sql/launchql-totp--0.0.3.sql +++ b/packages/totp/sql/launchql-totp--0.0.3.sql @@ -118,7 +118,7 @@ $EOFCODE$ LANGUAGE sql STRICT IMMUTABLE; CREATE FUNCTION totp.random_base32 ( _length int DEFAULT 20 ) RETURNS text LANGUAGE sql AS $EOFCODE$ SELECT - string_agg(('{a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z,2,3,4,5,6,7}'::text[])[ceil(random() * 32)], '') + string_agg(('{a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z,2,3,4,5,6,7}'::text[])[ceil(get_byte(gen_random_bytes(1),0)/8 +1)], '') FROM generate_series(1, _length); $EOFCODE$; From dd5ca59e6324e366a293c5318251ddd5317c43b0 Mon Sep 17 00:00:00 2001 From: wiredfool Date: Fri, 30 Sep 2022 12:18:32 +0100 Subject: [PATCH 2/3] Constant time equality for the TOTP verify comparison --- packages/totp/sql/launchql-totp--0.0.3.sql | 42 +++++++++++++++++----- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/packages/totp/sql/launchql-totp--0.0.3.sql b/packages/totp/sql/launchql-totp--0.0.3.sql index c301f4d..ee725f9 100644 --- a/packages/totp/sql/launchql-totp--0.0.3.sql +++ b/packages/totp/sql/launchql-totp--0.0.3.sql @@ -100,15 +100,41 @@ BEGIN END; $EOFCODE$ LANGUAGE plpgsql STABLE; +CREATE FUNCTION totp.constant_time_equal(a text, b text, minlength int DEFAULT 6) RETURNS boolean AS $EOFCODE$ +-- Compare all of the individual characters of each string +-- minlength is optional, prevents timing attacks to discover the length of the string +-- Create a table of true/false for each individual character comparison +-- Count the true/false +-- Compare the count of true to the number of comparisons. + WITH maxlen AS ( + SELECT max(s) AS l + FROM (VALUES (octet_length(a)), + (octet_length(b)), + (minlength)) AS val(s) + ), + matches AS ( + SELECT substring(a FROM ix for 1) = substring(b FROM ix for 1) AS eq + FROM (SELECT generate_series(1, (SELECT l FROM maxlen)) AS ix) AS series + ), + counts AS ( + SELECT count(*) AS ct, eq + FROM matches GROUP BY eq + ) + SELECT (SELECT l FROM maxlen) = coalesce((SELECT ct FROM counts WHERE eq='t'), 0) + +$EOFCODE$ language sql VOLATILE; + CREATE FUNCTION totp.verify ( secret text, check_totp text, period int DEFAULT 30, digits int DEFAULT 6, time_from timestamptz DEFAULT now(), hash text DEFAULT 'sha1', encoding text DEFAULT 'base32', clock_offset int DEFAULT 0 ) RETURNS boolean AS $EOFCODE$ - SELECT totp.generate ( - secret, - period, - digits, - time_from, - hash, - encoding, - clock_offset) = check_totp; + SELECT totp.constant_time_equal( + totp.generate ( + secret, + period, + digits, + time_from, + hash, + encoding, + clock_offset), + check_totp); $EOFCODE$ LANGUAGE sql; CREATE FUNCTION totp.url ( email text, totp_secret text, totp_interval int, totp_issuer text ) RETURNS text AS $EOFCODE$ From 1cc1d16b9f3a124aa36beb51fa09bba83b13cd9f Mon Sep 17 00:00:00 2001 From: wiredfool Date: Fri, 30 Sep 2022 17:58:01 +0100 Subject: [PATCH 3/3] [SECURITY] Fix entropy * Breaking changes -- function signatures all take a bytea for secrets now. * [SECURITY] random_base32 is removed, this was giving n characters of 5 bytes of entropy, NOT n characters * 8 bits of entropy * Generate_secret now returns a raw bytea of n bits of randomness * Verify/generate now take a bytea of raw key text, not a base32 version. * The URL function returns the base32 version. * Pad_secret was unused, and was removed * base32_to_hex was unused after passing bytea around, so removed. * Anything with a variable default is volatile (now(), we'd expect different results every time it's called, in a different transaction anyway) --- packages/totp/sql/launchql-totp--0.0.3.sql | 87 +++++----------------- 1 file changed, 20 insertions(+), 67 deletions(-) diff --git a/packages/totp/sql/launchql-totp--0.0.3.sql b/packages/totp/sql/launchql-totp--0.0.3.sql index ee725f9..bc03b0c 100644 --- a/packages/totp/sql/launchql-totp--0.0.3.sql +++ b/packages/totp/sql/launchql-totp--0.0.3.sql @@ -32,46 +32,6 @@ BEGIN END; $EOFCODE$ LANGUAGE plpgsql STRICT IMMUTABLE; -CREATE FUNCTION totp.pad_secret ( input bytea, len int ) RETURNS bytea AS $EOFCODE$ -DECLARE - output bytea; - orig_length int = octet_length(input); -BEGIN - IF (orig_length = len) THEN - RETURN input; - END IF; - - -- create blank bytea size of new length - output = lpad('', len, 'x')::bytea; - - FOR i IN 0 .. len-1 LOOP - output = set_byte(output, i, get_byte(input, i % orig_length)); - END LOOP; - - RETURN output; -END; -$EOFCODE$ LANGUAGE plpgsql IMMUTABLE; - -CREATE FUNCTION totp.base32_to_hex ( input text ) RETURNS text AS $EOFCODE$ -DECLARE - output text[]; - decoded text = base32.decode(input); - len int = character_length(decoded); - hx text; -BEGIN - - FOR i IN 1 .. len LOOP - hx = to_hex(ascii(substring(decoded from i for 1)))::text; - IF (character_length(hx) = 1) THEN - -- if it is odd number of digits, pad a 0 so it can later - hx = '0' || hx; - END IF; - output = array_append(output, hx); - END LOOP; - - RETURN array_to_string(output, ''); -END; -$EOFCODE$ LANGUAGE plpgsql IMMUTABLE; CREATE FUNCTION totp.hotp ( key bytea, c int, digits int DEFAULT 6, hash text DEFAULT 'sha1' ) RETURNS text AS $EOFCODE$ DECLARE @@ -84,21 +44,15 @@ BEGIN END; $EOFCODE$ LANGUAGE plpgsql IMMUTABLE; -CREATE FUNCTION totp.generate ( secret text, period int DEFAULT 30, digits int DEFAULT 6, time_from timestamptz DEFAULT now(), hash text DEFAULT 'sha1', encoding text DEFAULT 'base32', clock_offset int DEFAULT 0 ) RETURNS text AS $EOFCODE$ + +CREATE FUNCTION totp.generate ( secret bytea, period int DEFAULT 30, digits int DEFAULT 6, time_from timestamptz DEFAULT now(), hash text DEFAULT 'sha1', clock_offset int DEFAULT 0 ) RETURNS text AS $EOFCODE$ DECLARE c int := FLOOR(EXTRACT(EPOCH FROM time_from) / period)::int + clock_offset; - key bytea; BEGIN - - IF (encoding = 'base32') THEN - key = ( '\x' || totp.base32_to_hex(secret) )::bytea; - ELSE - key = secret::bytea; - END IF; - - RETURN totp.hotp(key, c, digits, hash); + RETURN totp.hotp(secret, c, digits, hash); END; -$EOFCODE$ LANGUAGE plpgsql STABLE; +$EOFCODE$ LANGUAGE plpgsql VOLATILE; + CREATE FUNCTION totp.constant_time_equal(a text, b text, minlength int DEFAULT 6) RETURNS boolean AS $EOFCODE$ -- Compare all of the individual characters of each string @@ -124,7 +78,7 @@ CREATE FUNCTION totp.constant_time_equal(a text, b text, minlength int DEFAULT 6 $EOFCODE$ language sql VOLATILE; -CREATE FUNCTION totp.verify ( secret text, check_totp text, period int DEFAULT 30, digits int DEFAULT 6, time_from timestamptz DEFAULT now(), hash text DEFAULT 'sha1', encoding text DEFAULT 'base32', clock_offset int DEFAULT 0 ) RETURNS boolean AS $EOFCODE$ +CREATE FUNCTION totp.verify ( secret bytea, check_totp text, period int DEFAULT 30, digits int DEFAULT 6, time_from timestamptz DEFAULT now(), hash text DEFAULT 'sha1', clock_offset int DEFAULT 0 ) RETURNS boolean AS $EOFCODE$ SELECT totp.constant_time_equal( totp.generate ( secret, @@ -132,34 +86,33 @@ CREATE FUNCTION totp.verify ( secret text, check_totp text, period int DEFAULT 3 digits, time_from, hash, - encoding, clock_offset), check_totp); -$EOFCODE$ LANGUAGE sql; +$EOFCODE$ LANGUAGE sql VOLATILE; -CREATE FUNCTION totp.url ( email text, totp_secret text, totp_interval int, totp_issuer text ) RETURNS text AS $EOFCODE$ +CREATE FUNCTION totp.url ( email text, totp_secret bytea, totp_interval int, totp_issuer text ) RETURNS text AS $EOFCODE$ SELECT - concat('otpauth://totp/', totp.urlencode (email), '?secret=', totp.urlencode (totp_secret), '&period=', totp.urlencode (totp_interval::text), '&issuer=', totp.urlencode (totp_issuer)); + concat('otpauth://totp/', + totp.urlencode (email), + '?secret=', + totp.urlencode (base32.encode(totp_secret::text)), + '&period=', + totp.urlencode (totp_interval::text), + '&issuer=', + totp.urlencode (totp_issuer)); $EOFCODE$ LANGUAGE sql STRICT IMMUTABLE; -CREATE FUNCTION totp.random_base32 ( _length int DEFAULT 20 ) RETURNS text LANGUAGE sql AS $EOFCODE$ - SELECT - string_agg(('{a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z,2,3,4,5,6,7}'::text[])[ceil(get_byte(gen_random_bytes(1),0)/8 +1)], '') - FROM - generate_series(1, _length); -$EOFCODE$; - CREATE FUNCTION totp.generate_secret ( hash text DEFAULT 'sha1' ) RETURNS bytea AS $EOFCODE$ BEGIN -- See https://tools.ietf.org/html/rfc4868#section-2.1.2 -- The optimal key length for HMAC is the block size of the algorithm CASE - WHEN hash = 'sha1' THEN RETURN totp.random_base32(20); -- = 160 bits - WHEN hash = 'sha256' THEN RETURN totp.random_base32(32); -- = 256 bits - WHEN hash = 'sha512' THEN RETURN totp.random_base32(64); -- = 512 bits + WHEN hash = 'sha1' THEN RETURN gen_random_bytes(20); -- = 160 bits + WHEN hash = 'sha256' THEN RETURN gen_random_bytes(32); -- = 256 bits + WHEN hash = 'sha512' THEN RETURN gen_random_bytes(64); -- = 512 bits ELSE RAISE EXCEPTION 'Unsupported hash algorithm for OTP (see RFC6238/4226).'; RETURN NULL; END CASE; END; -$EOFCODE$ LANGUAGE plpgsql VOLATILE; \ No newline at end of file +$EOFCODE$ LANGUAGE plpgsql VOLATILE;