ECDH support

[olegbespalov]

What?

This PR brings base support for the ECDH into the xk6-webcrypto.

It contains:

• Key pair generation
• Importing/exporting public/private keys (raw and PKCS8 formats)
• derive bits operation
• WebPlatform API test case for the importing/exporting keys

Part of #49

Note: the WebPlatform API test case for the derive bits is out of the scope of the PR (but will be done straight after the merging of this PR) to reduce the scope of the PR. It's also good to mention that following #69 is dependent on this change and continued implementation at some aspects (e.g. spki support)

The derived key operation is also out of the scope of reducing the PR size.

Why?

ECDH is part of the WebCrypto API

[mstoykov]

rt.NewArrayBuffer uses the runtime , but this is done in the event loop.

Given your code I would recommend moving away from promises.New to goja.NewPromise and calling vu.RegisterCallback just before the goroutine creation. Then calling from the if below in hte callback as in

Suggested change

	if err != nil {
	reject(err)
	return
	}
	resolve(rt.NewArrayBuffer(result))
	callback(func() error {
	if err != nil {
	reject(err)
	return
	}
	resolve(rt.NewArrayBuffer(result))
	})

This way this whole thing will be executed on the event loop and you can both use the runtime but also the not wrapped resolve/reject.

[olegbespalov]

Let me clarify 🤔

Is the idea to go out of the vanilla go routine because we still use runtime? (rt.NewArrayBuffer)

[olegbespalov]

The clarification question still present, but the change is here a15ebeb

[oleiade]

I'd love clarifications on this too as it's often a space of using Goja/EventLoop where I feel less confident. I'm not sure I understood the rationale behind moving away from promises.New either 🙂

[mstoykov]

the problem is that rt.NewArrayBuffer is not happening on the event loop in the original code. It happens on a goroutine that isn't running on the event loop. At which point you have a race as this is accessing the runtime, again off the event loop.

promises.New is a wrapper around goja.NewPromises and the event loop, so that the resolve and reject calls can be called on the event loop without the users specifically working with RegisterCallback and co.

In this case though as the argument to resolve needs to be an array buffer - you already need to run the rt.NewArryaBuffer part on the event loop - so you need to use RegisterCallback either way.

Also in this case as before you make the goroutine you are on the event loop, the usefulness of promises.New's reject being able to be called off the event loop isn't useful and neither is the calling of resolve if you need to again go back to the event loop to make the ArrayBuffer.

As such my proposal was to not use it at all and instead use RegisterCallback.

// pseudo-go
func something() promise {
  // here we are on the event loop
  p, res, rej := promises.New()
  if err != nil {
    rej(err); // this now actually just queues a job on the eventloop, and then when the callstack is empty will actuall reject the promise. 
   // not a deal breaker by any problem, but 
    return p;
  }

  p, res, rej := goja.NewPromise();

  if err != nil {
    rej(err); // this actually just rejects it outright, so the event loop doesn't need to go through this
    return p 
  }
  callback := vu.RegisterCallback()
  go func() {
    // do async work no actual usage of `rt`
   callback(func() error {
     if err !=nil {
       rej(err); // this already is on the event loop as it is the callback - so no need to queue a new job
       return
    }
    res(rt.newArrayBuffer()); // here we are on the event loop so we can use rt no problem.
  }) 
  }
  return p
}

In the above case if you drop promises.New - you will also never need to do RegisterCallback and directly interact with the event loop in the case you have an error.

tl;dr: You can still use promises.New - you just will make additional eventloop jobs that will not be useful and you in practice still need to RegisterCallback . If we didn't need to do work with the runtime before we can resolve/reject the promise - we could've used promises.New

[olegbespalov]

Okay, if I'm reading this right, the a15ebeb changes are fine, but we also need to refactor a bit other places where rt accesses inside the plain go routines, e.g.:

• https://github.com/grafana/xk6-webcrypto/blob/main/webcrypto/subtle_crypto.go#L193
• https://github.com/grafana/xk6-webcrypto/blob/main/webcrypto/subtle_crypto.go#L293
• etc

[mstoykov]

Looking at the changes - you now do not have a gorotuine - you directly use the callback, so nohting happens of the event loop .

Otherwise - yes the those are other cases of the same problem that we haven't caught.

[olegbespalov]

How about this 411de40 ? reject is still out of the callback, but my understanding this should be fine 🤔

[olegbespalov]

Okay, as a summary of the internal conversation with @mstoykov

We should access the runtime (rt) only through the event loop (in this example, it's inside the callback), but if we directly use rt.NewPromise the both resolve and reject should be executed on top of the event loop, so the result version https://github.com/grafana/xk6-webcrypto/pull/67/files#diff-9e8a31e9776cf152256880a32e1c5751bf67322c23c0cb096d71cf0633d8fe14R665-R673

That way, we'll be protected from the possible races.

[olegbespalov]

And I'm going to adjust all usages inside the module accordingly #71

[mstoykov]

LGTM in general, I have left some questions and there is one place where there is usage of the runtiem off the event loop that is the requested change

[oleiade]

Looking good, great job 👏🏻 I'm really happy to see ECDH land in the extension 🚀