Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support using P-256 keys for block and transaction signing #491

Merged
merged 20 commits into from
Oct 24, 2023
Merged

Conversation

swatanabe
Copy link
Collaborator

@swatanabe swatanabe commented Sep 22, 2023

Motivation

The NIST curves are much more widely supported than secp256k1. Adding them will give us better compatibility with third party software and hardware.

Data formats

  • The signature is encoded in compact form (64 bytes, r+s, big endian)
  • The public key is encoded as an X.509 Subject Public Key Info
  • The private key is encoded using PKCS #8

Verify Service

The VerifySys service (verify-sys) uses Botan. Botan supports picking out only the required components much better than openssl does and compiles to WASM with only minimal tweaks. The main patch I needed to make was to add a stub exception library that simply aborts when an exception is thrown. VerifySys supports both named curves and arbitrary curves with explicit domain parameters. However, only 256-bit curves are useful right now, because SHA256 is applied by native.

Block Signing

Native block signing is supported using OpenSSL. Any private key supported by OpenSSL can be used to sign, however key generation is not configurable and always generates an r1 key. As with VerifySys, the use of SHA256 is hard-coded.

psibase command line support

All key command line arguments can be either base58 encoded keys as before or the name of a file containing a PEM or DER encoded key. A base58 key uses verifyec-sys/auth-ec-sys. ASN.1 encodings use verify-sys/auth-sys. Only k1 keys are supported right now. The public key must be in compressed form and must use the named curve representation of the parameters.

js frontend support

Keys can be either PEM or Base58 encoded. When creating an account, account-sys always uses auth-sys. When signing or importing keys, the auth service will be deduced by querying the chain.

@James-Mart
Copy link
Member

James-Mart commented Sep 25, 2023

VerifySys supports both named curves and arbitrary curves with explicit domain parameters. However, only 256-bit curves are useful right now, because SHA256 is applied by native.

Am I understanding correctly that verify-sys can be used to verify nist and k1 curves?

@swatanabe
Copy link
Collaborator Author

Am I understanding correctly that verify-sys can be used to verify nist and k1 curves?

Yes. Also brainpool and other more obscure curves.

@James-Mart
Copy link
Member

Yes. Also brainpool and other more obscure curves

Any reason not to switch everything to this and delete verify-ec-sys?

@swatanabe
Copy link
Collaborator Author

Any reason not to switch everything to this and delete verify-ec-sys?

I haven't checked performance. secp256k1 is more optimized.

}
else if (key.service.str() == "verify-sys")
{
result = std::make_shared<OpenSSLProver>(key.service, key.rawData);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the key data be fracpacked?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? It's already in a binary format.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed that the k1 key is expected to be packed, why is it different?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The k1 key is actually a variant<k1_key, r1_key>, so it needs additional encoding. The BER encoded key already contains a discriminator to identify the type of key. I don't see any reason to add another layer of wrapping.

@@ -52,6 +52,7 @@ const ACCOUNTS: [AccountNumber; 27] = [
account!("token-sys"),
transaction_sys::SERVICE,
account!("verifyec-sys"),
account!("verify-sys"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The names are a bit misleading now, since nist curves are ec, but can only be verified with verify-sys. Maybe rename verifyec-sys to verifyk1-sys?

@James-Mart James-Mart added the Node infra Related to the necessary infrastructure provided by all psibase infrastructure providers label Sep 26, 2023
- Botan requires a larger stack to initialize the oid tables
- raw_hash needs to be available
- Change account-sys and common-sys to use auth-sys instead of
  auth-ec-sys and adjust key and signature formats
@James-Mart James-Mart changed the title Support P-256 keys Support using P-256 curves for block and transaction signing Oct 2, 2023
@James-Mart James-Mart changed the title Support using P-256 curves for block and transaction signing Support using P-256 keys for block and transaction signing Oct 2, 2023
@swatanabe
Copy link
Collaborator Author

The current performance numbers don't look good. I'm going to check how much of that is global initialization.

ns verify-sys verifyec-sys
43013901 733819
42598396 2903760
45292236 778158
66910510 2885490
42495448 1679233

@swatanabe
Copy link
Collaborator Author

The current performance numbers don't look good. I'm going to check how much of that is global initialization.

Most of the cost appears to be in initializing tables for EC group operations.

@swatanabe
Copy link
Collaborator Author

Tests are failing because binaryen 105 (from ubuntu 22.04) doesn't fully support v128.

@James-Mart
Copy link
Member

Can't we just change the 22.04 image to use the version of binaryen currently installed in 20.04?

@swatanabe
Copy link
Collaborator Author

Can't we just change the 22.04 image to use the version of binaryen currently installed in 20.04?

We could, but I'd rather minimize our dependence on non-system packages. I didn't see any significant performance difference for this service. We can reconsider minimum versions in six months when 24.04 ships.

Limitations:
- psibase chooses the auth service based on whether the key is base58
  (auth-ec-sys) or PEM (auth-sys). Since it does not query the chain,
  there is no other way to deduce the auth service.
- The PEM encoded keys must be put directly on the command line. Reading
  a file is not supported.
- The private key always signs using a compressed public key, but the public
  keys are not normalized. This can lead to signatures not being
  recognized unless the user is careful about managing encodings.
@swatanabe swatanabe merged commit da03175 into main Oct 24, 2023
3 checks passed
@swatanabe swatanabe deleted the r1-support branch October 24, 2023 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Node infra Related to the necessary infrastructure provided by all psibase infrastructure providers
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants