Skip to content

Commit 92cf99a

Browse files
authored
Add ARelaxCostRestriction parameter to Scrypt (#41)
Add ARelaxCostRestriction parameter to Scrypt to allow N >= 65536 when r = 1 The RFC 7914 constraint N < 2^(128*r/8) was confirmed by Colin Percival (Scrypt creator and RFC co-author) to be an accidental error in the RFC. The intended bound was N < 2^(128*r*8), which is trivially satisfied. RFC errata 5971, 5972, 5973 have been filed. The Ethereum Web3 Secret Storage standard uses N=262144, r=1, p=8 as its default Scrypt parameters. These are widely used in practice (geth, web3.py, web3.js, etc.) and accepted by the Scrypt reference implementation (Tarsnap) and Go's x/crypto/scrypt, but were rejected by the existing validation. A new ARelaxCostRestriction default parameter (False) is added to ValidatePBKDF_ScryptInputs, the constructor, and CreatePBKDF_Scrypt. When True, the erroneous cost/blocksize check is skipped. Existing callers are unaffected. See: golang/go#33703 (comment)
1 parent b9c3806 commit 92cf99a

2 files changed

Lines changed: 50 additions & 13 deletions

File tree

HashLib/src/Base/HlpHashFactory.pas

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,11 @@ TPBKDF_Scrypt = class sealed(TObject)
611611
/// <param name="ABlockSize">the block size, must be >= 1.</param>
612612
/// <param name="AParallelism">Parallelization parameter. Must be a positive integer less than or equal to
613613
/// <code>(System.High(Int32) div (128 * ABlockSize * 8))</code>.</param>
614+
/// <param name="ARelaxCostRestriction">
615+
/// When <c>True</c>, skips the <c>N &lt; 2^(128*r/8)</c> constraint
616+
/// from RFC 7914 that rejects <c>r=1</c> when <c>N &gt;= 65536</c>.
617+
/// See <c>ValidatePBKDF_ScryptInputs</c> for details and references.
618+
/// </param>
614619
/// <returns>
615620
/// The PBKDF_Scrypt KDF Interface Instance <br />
616621
/// </returns>
@@ -621,8 +626,8 @@ TPBKDF_Scrypt = class sealed(TObject)
621626
/// The cost, blocksize or parallelism is Invalid.
622627
/// </exception>
623628
class function CreatePBKDF_Scrypt(const APasswordBytes,
624-
ASaltBytes: THashLibByteArray; ACost, ABlockSize, AParallelism: Int32)
625-
: IPBKDF_Scrypt; static;
629+
ASaltBytes: THashLibByteArray; ACost, ABlockSize, AParallelism: Int32;
630+
ARelaxCostRestriction: Boolean = False): IPBKDF_Scrypt; static;
626631

627632
end;
628633

@@ -1569,11 +1574,11 @@ class function TKDF.TPBKDF_Argon2.CreatePBKDF_Argon2(const APassword
15691574
{ TKDF.TPBKDF_Scrypt }
15701575

15711576
class function TKDF.TPBKDF_Scrypt.CreatePBKDF_Scrypt(const APasswordBytes,
1572-
ASaltBytes: THashLibByteArray; ACost, ABlockSize, AParallelism: Int32)
1573-
: IPBKDF_Scrypt;
1577+
ASaltBytes: THashLibByteArray; ACost, ABlockSize, AParallelism: Int32;
1578+
ARelaxCostRestriction: Boolean): IPBKDF_Scrypt;
15741579
begin
15751580
Result := TPBKDF_ScryptNotBuildInAdapter.Create(APasswordBytes, ASaltBytes,
1576-
ACost, ABlockSize, AParallelism);
1581+
ACost, ABlockSize, AParallelism, ARelaxCostRestriction);
15771582
end;
15781583

15791584
end.

HashLib/src/KDF/HlpPBKDF_ScryptNotBuildInAdapter.pas

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,40 @@ TPBKDF_ScryptNotBuildInAdapter = class sealed(TKDF, IPBKDF_Scrypt,
9797

9898
public
9999

100+
/// <summary>
101+
/// Validates Scrypt input parameters.
102+
/// </summary>
103+
/// <param name="ACost">CPU/Memory cost parameter N.</param>
104+
/// <param name="ABlockSize">Block size parameter r.</param>
105+
/// <param name="AParallelism">Parallelization parameter p.</param>
106+
/// <param name="ARelaxCostRestriction">
107+
/// When <c>True</c>, skips the <c>N &lt; 2^(128*r/8)</c> constraint
108+
/// from RFC 7914 (which rejects <c>r=1</c> when <c>N &gt;= 65536</c>).
109+
/// Colin Percival (Scrypt creator and RFC co-author) confirmed this
110+
/// constraint was an accidental error; the intended bound was
111+
/// <c>N &lt; 2^(128*r*8)</c>, which is trivially satisfied.
112+
/// The Scrypt reference implementation (Tarsnap) does not enforce it,
113+
/// and the Ethereum Web3 Secret Storage standard depends on
114+
/// <c>N=262144, r=1, p=8</c> which violates this erroneous constraint.
115+
/// <list type="bullet">
116+
/// <item>RFC errata: https://www.rfc-editor.org/errata/rfc7914 (5971, 5972, 5973)</item>
117+
/// <item>Author confirmation: https://github.com/golang/go/issues/33703#issuecomment-568198927</item>
118+
/// <item>OpenSSL: https://github.com/openssl/openssl/issues/24650</item>
119+
/// <item>Go: https://github.com/golang/go/issues/33703</item>
120+
/// <item>geth: https://github.com/ethereum/go-ethereum/issues/19977</item>
121+
/// <item>eth-account: https://github.com/ethereum/eth-account/issues/181</item>
122+
/// <item>noble-hashes: https://github.com/paulmillr/noble-hashes/issues/61</item>
123+
/// <item>RustCrypto: https://github.com/RustCrypto/password-hashes/issues/546</item>
124+
/// <item>Rustic: https://github.com/rustic-rs/rustic/issues/1394</item>
125+
/// <item>Node.js: https://github.com/nodejs/node/pull/28799</item>
126+
/// </list>
127+
/// </param>
100128
class procedure ValidatePBKDF_ScryptInputs(ACost, ABlockSize,
101-
AParallelism: Int32); static;
129+
AParallelism: Int32; ARelaxCostRestriction: Boolean = False); static;
102130

103131
constructor Create(const APasswordBytes, ASaltBytes: THashLibByteArray;
104-
ACost, ABlockSize, AParallelism: Int32);
132+
ACost, ABlockSize, AParallelism: Int32;
133+
ARelaxCostRestriction: Boolean = False);
105134

106135
destructor Destroy; override;
107136

@@ -409,7 +438,7 @@ class function TPBKDF_ScryptNotBuildInAdapter.MFCrypt(const APasswordBytes,
409438
end;
410439

411440
class procedure TPBKDF_ScryptNotBuildInAdapter.ValidatePBKDF_ScryptInputs(ACost,
412-
ABlockSize, AParallelism: Int32);
441+
ABlockSize, AParallelism: Int32; ARelaxCostRestriction: Boolean);
413442
var
414443
LMaxParallel: Int32;
415444
begin
@@ -419,10 +448,12 @@ class procedure TPBKDF_ScryptNotBuildInAdapter.ValidatePBKDF_ScryptInputs(ACost,
419448
raise EArgumentHashLibException.CreateRes(@SInvalidCost);
420449
end;
421450

422-
// Only value of ABlockSize that cost (as an int) could be exceeded for is 1
423-
if ((ABlockSize = 1) and (ACost >= 65536)) then
451+
if (not ARelaxCostRestriction) then
424452
begin
425-
raise EArgumentHashLibException.CreateRes(@SBlockSizeAndCostIncompatible);
453+
if ((ABlockSize = 1) and (ACost >= 65536)) then
454+
begin
455+
raise EArgumentHashLibException.CreateRes(@SBlockSizeAndCostIncompatible);
456+
end;
426457
end;
427458

428459
if (ABlockSize < 1) then
@@ -446,10 +477,11 @@ procedure TPBKDF_ScryptNotBuildInAdapter.Clear();
446477
end;
447478

448479
constructor TPBKDF_ScryptNotBuildInAdapter.Create(const APasswordBytes,
449-
ASaltBytes: THashLibByteArray; ACost, ABlockSize, AParallelism: Int32);
480+
ASaltBytes: THashLibByteArray; ACost, ABlockSize, AParallelism: Int32;
481+
ARelaxCostRestriction: Boolean);
450482
begin
451483
Inherited Create();
452-
ValidatePBKDF_ScryptInputs(ACost, ABlockSize, AParallelism);
484+
ValidatePBKDF_ScryptInputs(ACost, ABlockSize, AParallelism, ARelaxCostRestriction);
453485
FPasswordBytes := System.Copy(APasswordBytes);
454486
FSaltBytes := System.Copy(ASaltBytes);
455487
FCost := ACost;

0 commit comments

Comments
 (0)