Remove unsafe cell from SecretKey

[davxy]

Instead of applying the resplit after the operation. Resplit before (by cloning where necessary).

This also allows the secret key to be Sync

[davxy]

maybe this resplit here is redundant as the secret has just been constructed

[davxy]

A - maybe better - alternative is to define SecretScalar as a newtype around one single generic PrimeField (instead of two) and then internally having some SecretScalarSplit defined with two elements.

The SecretScalarSplit is then constructed on the fly internally where required. The ephemeral SecretScalarSplit instance is then destroyed immediately.

This second approach could be better as it performs one single split instead of two (i.e. one on construction externally and one before the op internally - as we can't assume that the user externally performed the re-split, e.g. for long living SecretScalar instances)

[burdges]

Yeah, this should the best compromize. I never did this before because we still interact with the same key material many times this way, but..

The big timing vulnerability is the conditional adds in the scalar multiplication, which yes this avoids just fine. Also, we incure no costs by making the field constant time, unlike the elliptic curve operations.

It'd be nice if arkworks provided constant-time arithmetic somehow of course, but whatever..

Also..

I'm unsure how much this matters for sassafras. It matters for identity of course, but I implemented this hack mostly nugget_bls in bridges, but everyone picked syed's development off my old crappy bls crate, which is probably just vunlerable, like every other bls12-381. lol

As an aside, dfinity should be more vulnerable than other blockchains: few validators on identical hardware in few data centers. Also, if you break the threshold number of keys in one reshare period, then you break their chain key forever.

[davxy]

Superseeded by #88

[burdges]

I kinda prefer #87 over #88 really, primarily because #87 admits more implementations. #88 exposes the scalar directly via pub, not sure if that's ever useful.

The big difference is: #88 creates one scalar directly, not two seperate ones. If arkworks ever gets constant time arithemtic then this simplifies the spec, but SW curves are generally known for not being constant time, so if this improves the spec is debatable.

We've started running schnorrkel & others in the runtime, which while dubious looks harmess. Is there a need for that here too?

[davxy]

@burdges The public scalar thing is easily fixable: 68b4510. Now is private. Don't know why I made it pub in the first place 😃

Yeah #88 maintains one scalar but it splits it just before each operation. IMO splitting it during construction (as in #87) doesn't offer any additional value as:

• keeping it splitted in memory doesn't guarantee any additional "secrecy" as one can just add the two components.
• a proper usage requires to perform a resplit before the operation anyway (so the split on construction thing is quite redundant)

#88 is even more secure as if you are using SecretScalar it ALWAYS resplits before doing the operation. #87 instead leaves the resplit in the hand of the user, which may not call it and reuse the same secret scalar incarnation multiple times.

If the important thing is to use the splitted scalar to perform the operation, then both of them are providing this property (with the difference that #88 has less footprint and is harder to misuse)

[davxy]

We've started running schnorrkel & others in the runtime, which while dubious looks harmess. Is there a need for that here too?

If you refer to paritytech/polkadot-sdk#2044 then this only allows to create the secret in the runtime. Signing is still not exposed by default (requires the full_crypto feature). This is mostly used to construct the pks for tests during genesis.