[Add]: Add SignClient Plugin

[Wi1l-B0t]

Description

Add SignClient Plugin to implement the Signer interface, so that the signer can be implemented as an independent service.

If want to protect the private keys in the node, these private keys cannot be stored as plain text in the node.
And the private key can only be decrypted in a protected place(for example: enclave).

But these protection mechanisms are varied, different cloud vendors and different CPUs have different mechanisms and usages.
So there are many benefits to implementing the protected part as a separate service.

This is a plugin, so if this plugin is not installed, it will not change any of the current behaviour.

Type of change

• Optimization (the change is only an optimization)
• Style (the change is only a code style for better maintenance or standard purpose)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[superboyiii]

Should be a project under plugins in neo solution. For the UT, it should be included in tests.

Expected:

[Jim8y]

@Wi1l-B0t

[superboyiii]

I suggest enriching the comments to help people better understand the meaning of this plugin. Something like /// <inheritdoc/> is not good to be there if we will finally merge this. And for the new command get account status, it surely can't be used without TEE. So we'd better to make some warning or explaination for this method. Otherwise they will see something like this:

DebugException is not good to be there.

[shargon]

Exe for tests?

[Wi1l-B0t]

Every test project has this.

[shargon]

It's better to move the parse logic to settings, we will get a possible error earlier

[Wi1l-B0t]

It's better to move the parse logic to settings, we will get a possible error earlier

Fixed

[Wi1l-B0t]

I suggest enriching the comments to help people better understand the meaning of this plugin. Something like /// <inheritdoc/> is not good to be there if we will finally merge this. And for the new command get account status, it surely can't be used without TEE. So we'd better to make some warning or explaination for this method. Otherwise they will see something like this: DebugException is not good to be there.

OK, I will fix it.

[Wi1l-B0t]

Should be a project under plugins in neo solution. For the UT, it should be included in tests. Expected:

OK, Fix it soon.

[Wi1l-B0t]

I suggest enriching the comments to help people better understand the meaning of this plugin. Something like /// <inheritdoc/> is not good to be there if we will finally merge this. And for the new command get account status, it surely can't be used without TEE. So we'd better to make some warning or explaination for this method. Otherwise they will see something like this: DebugException is not good to be there.

Fixed

[cschuchardt88]

Why not create a agent? like ssh-agent and ssh-forward-agent? I would like to see a real key-exchange than some made up one.

[cschuchardt88]

meeting talk

[Wi1l-B0t]

Why not create a agent? like ssh-agent and ssh-forward-agent? I would like to see a real key-exchange than some made up one.

What needs to be changed?

This is not about agent or key-exchange.

This feature separates the part of signing with a private key and keeps the private key in a more secure place(sgx enclave, aws nitro, etc.).
Even if a neo node is hacked by an attacker, it will not be able to obtain the private key in the node.

[Jim8y]

meeting talk

This is a feature being requested by NGD to protect privatekey in TEE, specifically for TEE, and has being under working for 2 months, its different from what you want.

[shargon]

Why not create a agent? like ssh-agent and ssh-forward-agent? I would like to see a real key-exchange than some made up one.

What needs to be changed?

This is not about agent or key-exchange.

This feature separates the part of signing with a private key and keeps the private key in a more secure place(sgx enclave, aws nitro, etc.). Even if a neo node is hacked by an attacker, it will not be able to obtain the private key in the node.

But they will be able to sign anything, without access to the private key

[shargon]

The endPoint is not protected by password?

[cschuchardt88]

meeting talk

This is a feature being requested by NGD to protect privatekey in TEE, specifically for TEE, and has being under working for 2 months, its different from what you want.

doesn't mean this is right or correct solution

[Wi1l-B0t]

Why not create a agent? like ssh-agent and ssh-forward-agent? I would like to see a real key-exchange than some made up one.

What needs to be changed?
This is not about agent or key-exchange.
This feature separates the part of signing with a private key and keeps the private key in a more secure place(sgx enclave, aws nitro, etc.). Even if a neo node is hacked by an attacker, it will not be able to obtain the private key in the node.

But they will be able to sign anything, without access to the private key

Yes, this implementation has some issues and needs some improvement

[Jim8y]

The expected behavior is: transactions or messages are constructed in the TEE side, and there will be whitelist/blacklist, only specific messages and trasnactions will be constructed and signed. This pr is only a plugin, it has no signing logic yet. but since author decides to close it. Lets keep discussing.

[shargon]

The expected behavior is: transactions or messages are constructed in the TEE side, and there will be whitelist/blacklist, only specific messages and trasnactions will be constructed and signed. This pr is only a plugin, it has no signing logic yet. but since author decides to close it. Lets keep discussing.

It's good for me, but require https (possible now) and auth

[Wi1l-B0t]

The expected behavior is: transactions or messages are constructed in the TEE side, and there will be whitelist/blacklist, only specific messages and trasnactions will be constructed and signed. This pr is only a plugin, it has no signing logic yet. but since author decides to close it. Lets keep discussing.

It's good for me, but require https (possible now) and auth

The problem with the current implementation is that any data can be signed,
and this data can be the data of transfer tx,
so there is a security problem.

I'll change the current interface and implementation, so that it can only sign consensus messages.

require https (possible now) and auth

I don't think https is necessary, because the signed data/messages are public, and the neo process and the sign process run on the same node.

auth may have no actual protection, because an attacker can get the auth token(or whatever) from the neo process.

[shargon]

If it run in the same node, http is not the best protocol to communicate, use pipes

[Wi1l-B0t]

If it run in the same node, http is not the best protocol to communicate, use pipes

Use http because grpc is based on http(or https), and tcp is more convenient.
And aws nitro uses vsock.

[cschuchardt88]

@Wi1l-B0t lets talk requirements and solutions, having a different program like an agent service is fine. But having a plugin is not best solution. Plugins are optional.

[Jim8y]

@Wi1l-B0t lets talk requirements and solutions, having a different program like an agent service is fine. But having a plugin is not best solution. Plugins are optional.

For the purpose of having TEE itself is optional, need TEE enabled server, not all node support it.

[cschuchardt88]

https://github.com/ms-iot/security/tree/master/Limpet

Building-and-Executing-TEE-based-applications-on-Azure-(April-2020).pdf

using System;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Security.Cryptography.Cng;

public class TpmKeyExample
{
    public static void Main(string[] args)
    {
        // 1.  Create a CngKey using the Microsoft Platform Crypto Provider
        CngKeyCreationParameters keyParams = new() { Provider = new CngProvider("Microsoft Platform Crypto Provider") };
        CngKey key = CngKey.Create(CngAlgorithm.Rsa, "MyTPMKey", keyParams);

        // 2.  Use the key (e.g., sign a message)
        using (RSACng rsa = new RSACng(key))
        {
            // ... your code to sign or encrypt using rsa ...
        }

        // 3.  Export the public key (if needed)
        //  public byte[] publicKey = rsa.ExportSubjectPublicKeyInfo();

        // 4.  Clean up the key
        key.Dispose();
    }
}

[Wi1l-B0t]

https://github.com/ms-iot/security/tree/master/Limpet

Building-and-Executing-TEE-based-applications-on-Azure-(April-2020).pdf

using System;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Security.Cryptography.Cng;

public class TpmKeyExample
{
    public static void Main(string[] args)
    {
        // 1.  Create a CngKey using the Microsoft Platform Crypto Provider
        CngKeyCreationParameters keyParams = new() { Provider = new CngProvider("Microsoft Platform Crypto Provider") };
        CngKey key = CngKey.Create(CngAlgorithm.Rsa, "MyTPMKey", keyParams);

        // 2.  Use the key (e.g., sign a message)
        using (RSACng rsa = new RSACng(key))
        {
            // ... your code to sign or encrypt using rsa ...
        }

        // 3.  Export the public key (if needed)
        //  public byte[] publicKey = rsa.ExportSubjectPublicKeyInfo();

        // 4.  Clean up the key
        key.Dispose();
    }
}

This may be a way to implement it.

The ISigner internface has been added to sign something when the neo node running.
This interface can has different implementations.

[Wi1l-B0t]

@Wi1l-B0t lets talk requirements and solutions, having a different program like an agent service is fine. But having a plugin is not best solution. Plugins are optional.

This is a optional feature, so add this as a plugin