From f344e66e95b2b778bdd993b77d76e6605d29c341 Mon Sep 17 00:00:00 2001 From: renaud dubois Date: Mon, 16 Sep 2024 16:19:27 +0200 Subject: [PATCH] Add weak keys testing in Setkey utility Add weak keys as identified by Crypto Experts and fuzzed by Guido Vranken. --- README.md | 4 +- src/lib/libSCL_eccUtils.sol | 257 ++++++++++++++++-------------------- test/libSCL_eccUtils.t.sol | 49 +++++-- 3 files changed, 150 insertions(+), 160 deletions(-) diff --git a/README.md b/README.md index 220fcd8..4cbb1d2 100644 --- a/README.md +++ b/README.md @@ -46,8 +46,10 @@ The results of the completed audits are in the doc/audit folder. | Team | branch | Target |status |Residual risks| |--------:|---------|:---------|:---------|:--:| | [CryptoExperts](https://github.com/get-smooth/crypto-lib/tree/main/doc/Audits) | CryptoExperts |P256 | Completed | 0| -| [Veridise](https://github.com/get-smooth/crypto-lib/tree/main/doc/Audits) | Veridise |P256+Ed25519 | Completed | 0| +| [Veridise](https://github.com/get-smooth/crypto-lib/tree/main/doc/Audits) | Veridise |P256, Ed25519 | Completed | 0 | +| Formal Land | Veridise | RIP7696 | Planned | - | +We are also grateful to Guido (https://github.com/guidovranken) which notice by its independant (and amazing) Fuzzing work that our weak keys testing was incorrect. # Curves implementation status diff --git a/src/lib/libSCL_eccUtils.sol b/src/lib/libSCL_eccUtils.sol index f38a49d..3348544 100644 --- a/src/lib/libSCL_eccUtils.sol +++ b/src/lib/libSCL_eccUtils.sol @@ -19,7 +19,67 @@ import { ModInv } from "@solidity/modular/SCL_modular.sol"; library SCL_ECCUTILS{ - + + function ecNormalize(uint256 p, uint256 X, uint256 Y, uint256 ZZ, uint256 ZZZ) public + view returns(uint256 x, uint256 y) + { + uint256 ZZZm1= ModInv(ZZZ,p); + y=mulmod(Y,ZZZm1,p); + ZZZm1=mulmod(ZZ,ZZZm1,p);//1/z + ZZZm1=mulmod(ZZZm1, ZZZm1, p);//1/z^2 + x=mulmod(X, ZZZm1, p); + + } + + function ecAddn(uint256 p,uint256 X1,uint256 Y1, uint256 X2, uint256 Y2 ) public view returns (uint256 X, uint256 Y){ + + assembly{ + function ecAddn2(x1, y1, zz1, zzz1, x2, y2, _p) -> _x, _y, _zz, _zzz { + y1 := sub(_p, y1) + y2 := addmod(mulmod(y2, zzz1, _p), y1, _p) + x2 := addmod(mulmod(x2, zz1, _p), sub(_p, x1), _p) + _x := mulmod(x2, x2, _p) //PP = P^2 + _y := mulmod(_x, x2, _p) //PPP = P*PP + _zz := mulmod(zz1, _x, _p) ////ZZ3 = ZZ1*PP + _zzz := mulmod(zzz1, _y, _p) ////ZZZ3 = ZZZ1*PPP + zz1 := mulmod(x1, _x, _p) //Q = X1*PP + _x := addmod(addmod(mulmod(y2, y2, _p), sub(_p, _y), _p), mulmod(sub(_p,2), zz1, _p), _p) //R^2-PPP-2*Q + + x1:=mulmod(addmod(zz1, sub(_p, _x), _p), y2, _p)//necessary split not to explose stack + _y := addmod(x1, mulmod(y1, _y, _p), _p) //R*(Q-X3) + } + X,Y,X1,Y1:=ecAddn2(X1,Y1,1,1,X2, Y2, p) + } + + return ecNormalize(p,X,Y,X1,Y1); + } + + function EcDbl(uint256 p, uint256 a, uint256 X, uint256 Y) public view returns (uint256 dX, uint256 dY) { + assembly{ + function ecDbl(x, y, zz, zzz, _p,_a) -> _x, _y, _zz, _zzz{ + let T1 := mulmod(2, y, _p) //U = 2*Y1, y free + let T2 := mulmod(T1, T1, _p) // V=U^2 + let T3 := mulmod(x, T2, _p) // S = X1*V + T1 := mulmod(T1, T2, _p) // W=UV + _y:= addmod(mulmod(3, mulmod(x,x,_p),_p),mulmod(_a,mulmod(zz,zz,_p),_p),_p)//M=3*X12+aZZ12 + + _zzz := mulmod(T1, zzz, _p) //zzz3=W*zzz1 + _zz := mulmod(T2, zz, _p) //zz3=V*ZZ1 + + _x := addmod(mulmod(_y, _y, _p), mulmod(sub(_p,2), T3, _p), _p) //X3=M^2-2S + T2 := mulmod(_y, addmod(_x, sub(_p, T3), _p), _p) //-M(S-X3)=M(X3-S) + + _y := addmod(mulmod(T1, y, _p), T2, _p) //-Y3= W*Y1-M(S-X3), we replace Y by -Y to avoid a sub in ecAdd + _y:= sub(_p, _y) + } + + dX,dY,X,Y:=ecDbl(X,Y,1,1,p,a) + + } + return ecNormalize(p,dX,dY,X,Y); + } + + function ecPow128(uint256 p, uint256 a, uint256 X, uint256 Y, uint256 ZZ, uint256 ZZZ) public view returns(uint256 x128, uint256 y128){ assembly{ @@ -48,162 +108,69 @@ library SCL_ECCUTILS{ x128=mulmod(X, ZZ, p); y128=mulmod(Y, ZZZ, p); } -/// @notice Verifies the input parameters of RIP7696, second opcode. ie curve equations and weak keys + +/// @notice Verifies the input parameters of RIP7696 ie curve equations and weak keys //test helper to precompute P**128 and Q**128 function SetKey(uint256 p, uint256 a,uint256 b, uint256 gx, uint256 gy, uint256 qx, uint256 qy) public view returns (bool status, uint256[10] memory ExtendedKey ){ uint256[10] memory Qpa=[qx,qy,0,0 ,p, a, gx, gy, 0, 0]; + uint256 x; + uint256 y; + uint256 G2x; + uint256 G2y; status=true; - //test that provided points are on curve + //test that provided points are on curve and not infinity if(ec_isOnCurve(p,a,b,gx,gy)!=true) return (false, Qpa); if(ec_isOnCurve(p,a,b,qx,qy)!=true) return (false, Qpa); - (Qpa[2], Qpa[3])=ecPow128(p, a, qx,qy,1,1); - (Qpa[8], Qpa[9])=ecPow128(p, a, gx,gy,1,1); + //banned weak keys, according to CRX report, section 2.3 + if(qx==gx) status=false;//Q=+-G is always banned + + //I. Weak keys for the windowing RIP7212 method + //reject 2G and -2G as valid public keys + (G2x,G2y)=EcDbl(p,a,gx,gy); + if(G2x==qx) status=false; + //reject 3G and -3G + (Qpa[8], Qpa[9])=ecAddn(p,G2x,G2y,gx,gy);//store 3G + if(Qpa[8]==qx) status=false;//reject 3G==Q and 3G==-Q + + //reject 1/2G and -1/2G as valid public keys + (Qpa[2], Qpa[3])=EcDbl(p,a,qx,qy);//2Q + if(Qpa[2]==gx) status=false;//reject 2Q=G and 2Q=-G + if(Qpa[2]==Qpa[8]) status=false;//2Q=+-3G - status=ecCheckPrecompute(Qpa); - - return (status, Qpa); -} - -//mapping from Q in input to function, contains Qx, Qy, Qx', Qy', p, a, gx, gy, gx', gy' -//where P' is P multiplied by 2 pow 128 for shamir's multidimensional trick -//todo: remove all magic numbers -uint constant _Qx=0x00; -uint constant _Qy=0x20; -uint constant _Qx2pow128=0x40; -uint constant _Qy2pow128=0x60; -uint constant _modp=0x80; -uint constant _a=0xa0; -uint constant _gx=0xc0; -uint constant _gy=0xe0; -uint constant _gpow2p128_x=0x100; -uint constant _gpow2p128_y=0x120; - -//Starting from mload(0x40) this is the mapping in allocated memory -//https://medium.com/@ac1d_eth/technical-exploration-of-inline-assembly-in-solidity-b7d2b0b2bda8 -//mapping from 0x40 in memory -uint256 constant _Prec_T8=0x800; -uint256 constant _Ap=0x820; -uint256 constant _y2=0x840; -uint256 constant _zzz2=0x860; -uint256 constant _free=0x880; - -// a brutal copy paste of Ecmulmuladd precomputations -function ecCheckPrecompute( - uint256 [10] memory Q//store Qx, Qy, Q'x, Q'y p, a, gx, gy, gx2pow128, gy2pow128 - ) public view returns (bool flag) { - /* I. precomputations phase */ - - uint256 X; - uint256 Y; - uint256 ZZZ; - uint256 ZZ; - flag=false; - - // bytes memory Mem = new bytes(16*4*32); - assembly ("memory-safe") { - - mstore(0x40, add(mload(0x40), _Prec_T8)) - mstore(add(mload(0x40), _Ap), mload(add(Q, 0x80))) //load modulus into AP addresse - - //store 4 256 bits values starting from addr+offset - function mstore4(addr, offset, val1, val2, val3, val4){ - mstore(add(offset, addr),val1 ) - offset:=add(32, offset) - mstore(add(offset, addr),val2 ) - offset:=add(32, offset) - mstore(add(offset, addr),val3 ) - offset:=add(32, offset) - mstore(add(offset, addr),val4 ) - offset:=add(32, offset) - } - /* I. precomputations */ - //allocate memory for 15 projective points, first slot is unused - - let _modulusp:=mload(add(mload(0x40), _Ap)) - //normalized addition of two point, must not be neutral input - function ecAddn2(x1, y1, zz1, zzz1, x2, y2, _p) -> _x, _y, _zz, _zzz { - y1 := sub(_p, y1) - y2 := addmod(mulmod(y2, zzz1, _p), y1, _p) - x2 := addmod(mulmod(x2, zz1, _p), sub(_p, x1), _p) - _x := mulmod(x2, x2, _p) //PP = P^2 - _y := mulmod(_x, x2, _p) //PPP = P*PP - _zz := mulmod(zz1, _x, _p) ////ZZ3 = ZZ1*PP - if iszero(_zz) {//either P1=P2 or P1=-P2, not allowed - mstore(0x80, shl(229, 4594637)) - mstore(0x84, 32) - mstore(0xA4, 30) - mstore(0xC4, "Amount to raise smaller than 0") - revert(0x80, 0x64) - } - _zzz := mulmod(zzz1, _y, _p) ////ZZZ3 = ZZZ1*PPP - zz1 := mulmod(x1, _x, _p) //Q = X1*PP - _x := addmod(addmod(mulmod(y2, y2, _p), sub(_p, _y), _p), mulmod(sub(_p,2), zz1, _p), _p) //R^2-PPP-2*Q + (x,y)=ecAddn(p,Qpa[2], Qpa[3],qx,qy);//reject 3Q=G and -3Q=G + if(x==gx) status=false; + if(x==G2x) status=false;//reject 3Q=2G and -3Q=2G + + + (Qpa[2], Qpa[3])=ecPow128(p, a, qx,qy,1,1);//2**128Q + (Qpa[8], Qpa[9])=ecPow128(p, a, gx,gy,1,1);//2**128G + + //II. Weak keys for the 4MSM RIP7696 method + if(Qpa[8]==qx) status=false;//reject Q=+-2**128G + if(Qpa[2]==gx) status=false;//reject G=+-2**128Q + (x,y)=ecAddn(p, gx, gy, Qpa[8], Qpa[9]);//(1+2**128)G + if(x==qx) status=false;//reject Q=+-(1+2**128)G + + (x,y)=ecAddn(p, gx, gy, Qpa[2], Qpa[3]);//(1+2**128)Q + if(x==gx) status=false;//reject Q=+-1/(1+2**128)G + if(x==Qpa[8]) status=false;//reject Q=+(2**128/(1+2**128))G - x1:=mulmod(addmod(zz1, sub(_p, _x), _p), y2, _p)//necessary split not to explose stack - _y := addmod(x1, mulmod(y1, _y, _p), _p) //R*(Q-X3) - - } - - - mstore4(mload(0x40), 128, mload(add(Q,_gx)), mload(add(Q,_gy)), 1, 1) //G the base point - mstore4(mload(0x40), 256, mload(add(Q,_gpow2p128_x)), mload(add(Q,_gpow2p128_y)), 1, 1) //G'=2^128.G - - X:=mload(add(Q,_gpow2p128_x)) - Y:=mload(add(Q,_gpow2p128_y)) - X,Y,ZZ,ZZZ:=ecAddn2( X,Y,1,1, mload(add(Q,_gx)),mload(add(Q,_gy)), _modulusp) //G+G' - mstore4(mload(0x40), 384, X,Y,ZZ,ZZZ) //Q, the public key - mstore4(mload(0x40), 512, mload(Q),mload(add(32,Q)),1,1) - - X,Y,ZZ,ZZZ:=ecAddn2( mload(Q),mload(add(Q,32)),1,1, mload(add(Q,_gx)),mload(add(Q,_gy)),_modulusp )//G+Q - mstore4(mload(0x40), 640, X,Y,ZZ,ZZZ) - - - X:=mload(add(Q,_gpow2p128_x)) - Y:=mload(add(Q,_gpow2p128_y)) - X,Y,ZZ,ZZZ:=ecAddn2(X,Y,1,1,mload(Q),mload(add(Q,32)), _modulusp)//G'+Q - mstore4(mload(0x40), 768, X,Y,ZZ,ZZZ) - - X,Y,ZZ,ZZZ:=ecAddn2( X,Y,ZZ,ZZZ, mload(add(Q,_gx)), mload(add(Q,_gy)), _modulusp)//G'+Q+G - mstore4(mload(0x40), 896, X,Y,ZZ,ZZZ) - - mstore4(mload(0x40), 1024, mload(add(Q, 64)), mload(add(Q, 96)),1,1) //Q'=2^128.Q + G2y= p-Qpa[9]; + (G2x,G2y)=ecAddn(p, gx, gy, Qpa[8],G2y);//(1-2**128)G + if(G2x==qx) status=false;//reject Q=(1-2**128)G + if(G2x==x) status=false;//reject Q=((1-2**128)/(1+2**128)G + + G2y= p-Qpa[3]; + (G2x,G2y)=ecAddn(p, gx, gy, Qpa[2],G2y );//(1-2**128)Q + if(G2x==qx) status=false;//reject Q=(1-2**128)G + if(G2x==x) status=false;//reject Q=((1-2**128)/(1+2**128)G - X,Y,ZZ,ZZZ:=ecAddn2(mload(add(Q, 64)), mload(add(Q, 96)),1,1, mload(add(Q,_gx)),mload(add(Q,_gy)), mload(add(mload(0x40), _Ap)) )//Q'+G - mstore4(mload(0x40), 1152, X,Y,ZZ,ZZZ) - - - X:=mload(add(Q,_gpow2p128_x)) - Y:=mload(add(Q,_gpow2p128_y)) - X,Y,ZZ,ZZZ:=ecAddn2(mload(add(Q, 64)), mload(add(Q, 96)),1,1, X,Y, mload(add(mload(0x40), _Ap)) )//Q'+G' - mstore4(mload(0x40), 1280, X,Y,ZZ,ZZZ) - - X,Y,ZZ,ZZZ:=ecAddn2(X, Y, ZZ, ZZZ, mload(add(Q,_gx)), mload(add(Q,_gy)), mload(add(mload(0x40), _Ap)) )//Q'+G'+G - mstore4(mload(0x40), 1408, X,Y,ZZ,ZZZ) - - X,Y,ZZ,ZZZ:=ecAddn2( mload(Q),mload(add(Q,32)),1,1, mload(add(Q, 64)), mload(add(Q, 96)), mload(add(mload(0x40), _Ap)) )//Q+Q' - mstore4(mload(0x40), 1536, X,Y,ZZ,ZZZ) - - X,Y,ZZ,ZZZ:=ecAddn2( X,Y,ZZ,ZZZ, mload(add(Q,_gx)), mload(add(Q,_gy)), mload(add(mload(0x40), _Ap)) )//Q+Q'+G - mstore4(mload(0x40), 1664, X,Y,ZZ,ZZZ) - - X:= mload(add(768, mload(0x40)) )//G'+Q - Y:= mload(add(800, mload(0x40)) ) - ZZ:= mload(add(832, mload(0x40)) ) - ZZZ:=mload(add(864, mload(0x40)) ) - X,Y,ZZ,ZZZ:=ecAddn2( X,Y,ZZ,ZZZ,mload(add(Q, 64)), mload(add(Q, 96)), mload(add(mload(0x40), _Ap)) )//G'+Q+Q'+ - mstore4(mload(0x40), 1792, X,Y,ZZ,ZZZ) - - X,Y,ZZ,ZZZ:=ecAddn2( X,Y,ZZ,ZZZ,mload(add(Q,0xc0)),mload(add(Q,_gy)), mload(add(mload(0x40), _Ap)) )//G'+Q+Q'+G - // Prec[15] - mstore4(mload(0x40), 1920, X,Y,ZZ,ZZZ) - } - flag=true;// no revert means no pathologic case were reached - } - - } + return (status, Qpa); + } +} diff --git a/test/libSCL_eccUtils.t.sol b/test/libSCL_eccUtils.t.sol index ee358ba..5354fb6 100644 --- a/test/libSCL_eccUtils.t.sol +++ b/test/libSCL_eccUtils.t.sol @@ -64,25 +64,46 @@ function test_goodkeys_wycheproof() public view{ } } - - function test_weakkeys() public view{ + //value of weak keys were also partially independantly found via Guido's work + function test_weakkeys() public view returns (bool){ bool status=false; - //identify weak keys for 4MSM multiplication - uint256 qx=gx; + //identify weak keys as identified by CRX report + + //value 2Q, 3Q, 2G, 3G ... computed using sage + //1. Q=G + uint256 qx=gx;//1.Q=G uint256 qy=gy; - //todo: add cases identified in weak_ecdsa_keys.md - - //the catch SHALL fail, as we are trying to use a weak key - try SCL_ECCUTILS.SetKey(p, a, b, gx, gy, qx, qy) returns (bool val, uint256[10] memory Qpa) { - status=false; - } catch Error(string memory /*reason*/) { - status=true; - } - assertEq(status, true); - + (status,)=SCL_ECCUTILS.SetKey(p, a, b, gx, gy, qx, qy); + assertEq(status, false); + //2. 2Q=G + qx=19439240795854216504166878352080480852025048076250165894565412345120149900726; + qy=64185496424002857229093033340967490116833136543672130696164427119171088715107; + (status,)=SCL_ECCUTILS.SetKey(p, a, b, gx, gy, qx, qy); + assertEq(status, false); + //3. 2Q=-G + qx=19439240795854216504166878352080480852025048076250165894565412345120149900726; + qy=51606592786353391533604413608440083413253006871618183499369204189696009138844; + (status,)=SCL_ECCUTILS.SetKey(p, a, b, gx, gy, qx, qy); + assertEq(status, false); + + //4. 3Q=G + + qx= 36858631515729577427618021587181665754426975973028251330501583611661900650952; + qy= 57614611834006547571837437085124862610314612006698381665563836969722143801436; + (status,)=SCL_ECCUTILS.SetKey(p, a, b, gx, gy, qx, qy); + assertEq(status, false); + + //5. 3Q=-2G + qx= 47820944831959596514351589625154938811624096318765624397616398316301423766546; + qy= 109730970122496639866270775665754787707917542520773780854335396537559893617185; + (status,)=SCL_ECCUTILS.SetKey(p, a, b, gx, gy, qx, qy); + assertEq(status, false); + + + return true; }