Adding P256 Signature Support in TVM

[Nilay27]

Description:

This pull request introduces support for P256 signature verification in the TON Virtual Machine (TVM). The changes include the addition of two new TVM instructions, P256_CHKSIGNU and P256_CHKSIGNS, which are used to verify P256 signatures.

Changes:

1. Code: Added the implementation for the check_p256_signature function, which is used to check P256 signatures. This function is used by the execute_p256_chksignu and execute_p256_chksigns functions, which serve as the entry points for the P256_CHKSIGNU and P256_CHKSIGNS operations respectively.

2. Handler Updates: Updated the handler.rs file to include the new instructions in the TVM instruction set.

3. Documentation: Updated the documentation to include descriptions of the new P256_CHKSIGNU and P256_CHKSIGNS instructions. These instructions check the P256-signature of a hash or data slice using a public key, represented as a Slice. The documentation provides a detailed explanation of the parameters and the expected output of these instructions.

The changes introduced in this PR will allow users to verify P256 signatures within the TVM, enhancing its capabilities and compatibility with different signature schemes.

Solidity integration:

An example of how these new instructions can be called from a Solidity smart contract using a free function and assembly feature. Here's a sample function:

function checkP256Signature( TvmSlice messageHash, TvmSlice signature, uint pubkey) assembly pure returns(bool){
    "P256_CHKSIGNS", 
}

Please review the changes and let me know if any modifications are required.

[cryshado]

@Nilay27 I think we might consider using the OpenSSL implementation as it is more reliable and well-tested, instead of p256. The Rust implementation that you have used seems to be relatively young and not very popular in the community. While this doesn't automatically mean it contains vulnerabilities, it does come with certain risks.

Additionally, I believe alternative implementations may be suggested, apart from OpenSSL, that are natively implemented in the Rust programming language. I invite the community to join in the discussions.

[cryshado]

@Nilay27 In your example, it is not entirely clear why you used the ISZERO and NOT instructions.

[cryshado]

@Nilay27 Also please create test cases with Rust and new vm version that would test the behavior of your new crypto instructions. This can be a separate git repository, as an example.

[Nilay27]

@Nilay27 In your example, it is not entirely clear why you used the ISZERO and NOT instructions.

Aah, the P256_CHKSIGNS will return either true or false so I am just checking it and returning the negation, which would essentially mean returning the original value of P256_CHKSIGNS.

While the result will be the same even without ISZERO and NOT, I saw that any boolean function compiled by TVM automatically added ISZERO and NOT to the return value.

[Nilay27]

@Nilay27 I think we might consider using the OpenSSL implementation as it is more reliable and well-tested, instead of p256. The Rust implementation that you have used seems to be relatively young and not very popular in the community. While this doesn't automatically mean it contains vulnerabilities, it does come with certain risks.

Additionally, I believe alternative implementations may be suggested, apart from OpenSSL, that are natively implemented in the Rust programming language. I invite the community to join in the discussions.

I did try using OpenSSL, however, I was facing issues with installing it's rust version on Mac M1. OpenSSL with rust seems to cause troubles with non-intel-based devices, hence I thought using a dependency which will work on any platform.

[cryshado]

@Nilay27 In your example, it is not entirely clear why you used the ISZERO and NOT instructions.

Aah, the P256_CHKSIGNS will return either true or false so I am just checking it and returning the negation, which would essentially mean returning the original value of P256_CHKSIGNS.

While the result will be the same even without ISZERO and NOT, I saw that any boolean function compiled by TVM automatically added ISZERO and NOT to the return value.

Interesting, can you show a short example of such a function? Are you talking about compiling in Solidity?

[Nilay27]

@Nilay27 In your example, it is not entirely clear why you used the ISZERO and NOT instructions.

Aah, the P256_CHKSIGNS will return either true or false so I am just checking it and returning the negation, which would essentially mean returning the original value of P256_CHKSIGNS.
While the result will be the same even without ISZERO and NOT, I saw that any boolean function compiled by TVM automatically added ISZERO and NOT to the return value.

Interesting, can you show a short example of such a function? Are you talking about compiling in Solidity?

Yes, compiling solidity only using Ton-Solidity-Compiler, however, right now it is working fine and is not adding ISZero and NOT and everything works the same as before.

I have removed those two from the example.

[ilyar]

You also need to add changes to this repository https://github.com/tonlabs/ever-assembler
add tests, I will try to show an example later on how to add tests for this, although I hope you figure it out yourself

[Nilay27]

@Nilay27 I think we might consider using the OpenSSL implementation as it is more reliable and well-tested, instead of p256. The Rust implementation that you have used seems to be relatively young and not very popular in the community. While this doesn't automatically mean it contains vulnerabilities, it does come with certain risks.

Additionally, I believe alternative implementations may be suggested, apart from OpenSSL, that are natively implemented in the Rust programming language. I invite the community to join in the discussions.

Regarding this comment should I look into implementation with OpenSSL or another alternative or should I continue to add tests with the current implementation using p256?

There weren’t any additional comments from the community regarding this after your invitation to discuss on this.

[Nilay27]

@cryshado @ilyar

I have implemented the openssl version of P256_CHKSIGNU and P256_CHKSIGNS.

Moreover, I also wrote tests for it. You can find the tests at:
Nilay27#1

This includes the changes to https://github.com/tonlabs/ever-assembler that I made in the repo:

https://github.com/Nilay27/ever-assembler.git --branch p256-instructions.

[ilyar]

@Nilay27 Hi, thank you for your persistence and fortitude.

I'm an ordinary member of the community and not part of the node team. I tried to pay attention to your PR, here are some comments that I managed to get from different people developing the node:

No, it's not important...

The decision on this PR has low priority because the solution can be made without changes to the VM. We recommend that it is better to make an SDK that is convenient for biometrics and will work with off-chain keys, the essence is the same, but it is most likely much more useful and necessary.

We recommend checking keychain signatures in contracts; this is a crazy idea due to the impossibility of backup.

I’ll also add on my own behalf: you must understand that even if this PR is accepted, let’s consider the best scenario, let’s assume it was accepted today, this will not give you either a guarantee or an understanding of when this opportunity will appear even on the test network, not to mention the mainnet. The process of accepting new changes VM`s is very long and painstaking; at the moment the VM has a number of changes, and they are still not activated, while the features themselves were added last year. Therefore, consider alternative options for solving your goal.

Thank you, keep us updated.

[Nilay27]

Hi @ilyar ,

Thanks for getting the feedback from the team, while I agree with some. It seems there are some misconceptions regarding the need for the OPCode and the product:

1. I understand that currently this OPCode is only needed for my product but it is in the roadmap of TVM to include this feature too: TVM Upgrade 2023-07
   It has already been implemented in the C++ implementation of TVM as can be seen here: C++ implementation

2. The signatures are supposed to be checked in the smart contract itself that is, right now it consumes a lot of gas, 2.4 million to be exact which will not be ideal for any real-world use-case.

3. The backup is not impossible, we plan to add recovery methods (either using social recovery or mpc recovery, so that people can get back their access to the contract).

4. The SDK currently does use off-chain keys and biometrics, the device itself is the signer! It is just that all the devices will only sign using the p256 curve. So that is why I wanted to get this opcode included.

While I understand this process will be long, I am in no way in a hurry to get this PR included now. What I want is at least this to be approved so that I know it will be included in the future.

Moreover, this opcode is supposed to be a part of the TVM upgrade, and the rust-based implementation will need to include one day or another. I am just hoping to selfishly speed up the process as it will benefit my product. :)