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

Introduce the capability to keep the private key confined to its host environment. #4345

Closed
andresrosenthal opened this issue Jul 9, 2024 · 18 comments

Comments

@andresrosenthal
Copy link

andresrosenthal commented Jul 9, 2024

Feature Request

Currently private keys are exposed into EDC & directly accessed for signing operations.
This approach becomes problematic when aiming for a "cryptography as a service" model and trying to prevent private keys from leaving their host environment. One example of this would be the use of Vault's Transit secrets engine.

Solution Proposal

I recommend transitioning from the current approach of:

interface PrivateKeyResolver {
    Result<PrivateKey> resolvePrivateKey(String id);
}

To a new one, something like this:

interface SignatureService {
    Result<byte[]> sign(String privateKeyId, String hashAlgorithm, byte[] dataToSign);
}

With this, both options (with and without exposing private keys) would become achievable. The (current) Vault secret approach would be provided by EDC itself, and any custom solution (e.g. Transit secrets engine, direct use of HSM through PKCS#11, etc..) can be easily implemented using this interface.

@github-actions github-actions bot added the triage all new issues awaiting classification label Jul 9, 2024
@paullatzelsperger
Copy link
Member

paullatzelsperger commented Jul 9, 2024

@andresrosenthal I generally like the idea of confining secret data to as secure an environment as possible, but we have to consider the following aspects:

  • signing procedures: the approach you suggested allows for simply signing binary data, but in almost all cases we sign JWTs, where this won't work anymore, because we use the Nimbus library which has its own API for signing. Then, we're relegated to fetch the private key from the vault again, so we've gained nothing.

  • consider other vault implementations: EDC uses Hashicorp Vault, but there are other vault implementations, which might not support this. In those cases, again we have to fetch the private key from the vault, negating the benefit.

  • we are not aiming for "cryptography as a service". I'm not sure where you are getting this notion from. I also disagree with the implication that the pattern you propose would achieve that, because users can still get private keys from the vault.

So yes, leaking private keys must be avoided, but the solution you propose does not achieve that on its own IMO. From what I understand it would effectively be a major refactoring, without any considerable benefit.

@ndr-brt
Copy link
Member

ndr-brt commented Jul 9, 2024

I think you put the feature request in the wrong perspective, that should have been sort of "add possibility to use Transit secret engine to sign" (that in my opinion would bring value) instead of proposing new interface signature speaking about "exposing private keys" that, in fact, it's not a real issue.

The technical details would then eventually be discussed by the committers.

@andresrosenthal
Copy link
Author

andresrosenthal commented Jul 10, 2024

We are currently PoC-ing the use of EDC in our X-Road data exchange ecosystem. Some participating organizations have policies requiring key pairs to be kept inside a Hardware Security Module (HSM). Additionally, we already have a separate "signer" component in production that accesses an HSM internally through PKCS#11 and provides externally an HTTP-based API to other components.

The proposed new interface was not a definitive solution but an illustration of the problem. The main idea is to make EDC more flexible by allowing the injection of a custom signature provider, enabling us to reuse our "signer" in EDC. Currently, this is challenging as it requires forking many classes that use PrivateKeyResolver, Supplier<PrivateKey>, or PrivateKey.

For the foreseeable future as EDC keeps on growing & organizations are gradually going to production with it, I can imagine we'd not be the only ones hindered by the current implementation.

The issue with the Nimbus Jose library could potentially be addressed by implementing a custom JWSSigner, similar to how the AWS KMS extension library solves the problem.

Would it be feasible to change the existing implementation by making the token generation logic to depend on JWSSigner instead of Supplier<PrivateKey>?
The default JWSSigner would then be initialized with a private key from Vault in SecurityDefaultServicesExtension (and not again and again during each JWT generation). This approach would also make it substitutable through a custom @Extension by providing a custom JWSSigner implementation, such as the AWS KMS extension or any other custom solution.

Hope this provides deeper insight.

@paullatzelsperger
Copy link
Member

paullatzelsperger commented Jul 10, 2024

as to @ndr-brt's point, I would recommend formulating clear business requirements instead of jumping to technical conclusions. I would also recommend opening a discussion first.

My personal opinion is that this would be a major refactoring, which would:

  • break downstream projects
  • tie up significant dev resources (yes, we'd wanna to do this ourselves because its a very central and sensitive area of the code)
  • be a niche solution, and we'd have to provide different code paths for vaults other than Hashicorp
  • not even solve the problem, because like i said in many cases we'd still have to get the key from the vault
  • only cater to the needs of one single project, i.e. you. We have not yet received a similar request from anyone else.
  • not offer much value to us

So TL;DR, unless we see clear and reasonable business requirements, and there is a critical mass of people wanting this, I honestly don't see a big chance for this to get realized.

That said, due to the modular nature of EDC you are of course free to implement such extensions in you own environment. Once that has reached a certain level of maturity, we are happy to check it out and accept a contribution.

@paullatzelsperger
Copy link
Member

paullatzelsperger commented Jul 10, 2024

as to @ndr-brt's point, I would recommend formulating clear business requirements instead of jumping to technical conclusions. I would also recommend opening a discussion first.

My personal opinion is that this would be a major refactoring, which would:

  • break downstream projects
  • tie up significant dev resources (yes, we'd wanna to do this ourselves because its a very central and sensitive area of the code)
  • be a niche solution, and we'd have to provide different code paths for vaults other than Hashicorp
  • not even solve the problem, because like i said in many cases we'd still have to get the key from the vault
  • only cater to the needs of one single project, i.e. you. We have not yet received a similar request from anyone else.
  • not offer much value to us

So TL;DR, unless we see clear and reasonable business requirements, and there is a critical mass of people wanting this, I honestly don't see a big chance for this to get realized.

That said, due to the modular nature of EDC you are of course free to implement such extensions in you own environment. Once that has reached a certain level of maturity, we are happy to check it out and consider accepting a contribution.

[edit] typos

@andresrosenthal
Copy link
Author

andresrosenthal commented Jul 18, 2024

Sorry for the late reply, been busy vacationing :)

Here's a more in-depth technical perspective to give you a clearer understanding of my viewpoint. (Note that necessary adjustments to tests are not included):

Connector: https://gist.github.com/andresrosenthal/8bb853f8828c592e7ce17702f7bbb51a
Identity Hub: https://gist.github.com/andresrosenthal/0f833f71b6d33805c20dca03c9e45796

The main idea is to replace the PrivateKeyResolver provider with a JWSSigner resolver function, while embedding the VaultPrivateKeyResolver within the default JWSSigner resolver as-is.
This exposure of JWSSigner provider would introduce the flexibility to provide a custom implementation of it, including the possibility where the private keys are never exposed to EDC's runtime (Again, I'm referencing nimbus-jose-jwt_aws-kms-extension’s JWSSigner implementation as an example).

  • The changes aren't really that comprehensive & complex, plus they won't break anything as long as there isn't any EDC deployment out there where the default VaultPrivateKeyResolver is overridden.
  • You aren't obligated to support or provide a code path for any other implementation, so you won't be taking on any new commitments. You can rock with just the current Hashicorp's Vault implementation as long as you want. If someone decides to use a custom JWSSigner implementation - it will be their own responsibility.

@petkivim
Copy link

petkivim commented Jul 31, 2024

Here's a little bit more background information from business perspective. Hopefully, it helps you to understand the business need for the proposed change better.

A data space Participant has a private key and a certificate issued by an eIDAS compliant qualified trust service provider. The key and certificate are used to create qualified electronic signatures and therefore, they must be stored in a qualified electronic signature creation device, e.g., Hardware Security Module (HSM). The Participant wants to use the same key and certificate in a data space where they are planning to join using an EDC-based data space connector. Exporting the private key from the HSM is not technically possible and even if it was, it's against the Participants security policy. Therefore, the only option is to be able to access the key managed by the HSM over the PKCS#11 interface that the HSM provides. The key can be used to sign data over the interface, but the signing is always done by the HSM device.

Many organisations have existing keys with certificates issued by eIDAS compliant qualified trust service providers. Usually, those keys are stored in HSMs, the private keys cannot be exported and they are accessed over the PKCS#11 interface. However, exporting the public keys and certificates is possible. Being able to connect the EDC to HSMs would allow reusing the existing keys and certificates instead of having to create and manage new, separate keys and certificates.

@petkivim
Copy link

petkivim commented Aug 1, 2024

We're currently working on making the X-Road® data exchange layer technically compatible with the data space protocol stack and the Gaia-X Trust Framework. We have decided to use the EDC in the implementation. More information about the project is available here.

X-Road is an existing data exchange technology and there are over 20 existing X-Road ecosystems around the world. For example, the Estonian X-Road ecosystem has around 1500 member organisations who already have an existing key pair with a certificate issued by an eIDAS compliant qualified trust service provider. The Estonian X-Road member organisations use HSMs to manage the private keys and exporting them is not possible. Currently, the keys are used by X-Road over the standard PKCS#11 interface which allows using them for signing without exposing the private keys to the outside world.

Our plan is to support using the same private keys and certificates with the new X-Road version based on the EDC. Since the certificates are issued by an eIDAS compliant qualified trust service provider, they meet the requirements of the Gaia-X Trust Framework and therefore, can be used to sign claims by Gaia-X Participants. Currently, our main problem in implementing this approach is that the EDC requires having direct access to the private key. To resolve the issue, we'd like to propose updating the signing and key handling so that direct access to the private key is not required.

@jimmarino
Copy link
Contributor

Thanks for explaining this further, as it provides some important context.

We should first enumerate what operations need to be included in this signing process. For example, creation of VPs do, maybe data plane operations don't. Can you provide a bulleted list?

@andresrosenthal
Copy link
Author

Hi!

Virtually every signing operation carried out by EDC during end-to-end data exchange between participants:

  • Signing of self-issued identity tokens (during communication of control planes)
  • Signing Verifiable Presentation
  • Signing DataPlane’s access token

@petkivim
Copy link

petkivim commented Aug 1, 2024

The reason why even the data plane operations should be included is that X-Road signs every message sent on the data plane level so that their integrity can be guaranteed.

@jimmarino
Copy link
Contributor

OK, next question: leaving aside the question of whether the private key resolver gets modified (or we go with a different solution), what are the requirements for a distinct singing service? I assume the service could be invoked as a remote service or as some library. Also, we need the ability for decorator extensions to contribute claims to be included. For specifics, see JwtGenerationService.

It is possible that only the above service needs to be modified to include an option to use a different JWSSigner in certain circumstances. Specifically, a JwtGenerationService instance is used in multiple places where such capabilities are not relevant, e.g. the OAuth2 extensions. One possible solution is to allow the JwtGenerationService to be configured with a signer extension that may or may not be used by the extensions that instantiate the former.

If such a solution is workable, it would not impact any other code paths.

@andresrosenthal
Copy link
Author

andresrosenthal commented Aug 1, 2024

what are the requirements for a distinct singing service?

We use a client library which under the hood communicates with the remote signing service. We'd like to invoke calls to this client library's API whenever signatures are needed.

Introducing the option to use a different JWSSigner in the JwtGenerationService sounds great, as it would allow us to implement a custom JWSSigner to embed our library's API calls.
Although JwtGenerationService seems to be reused in most cases, it does not appear to be utilized for signing Verifiable Presentations, where the iron-verifiable-credentials library is used instead.

@paullatzelsperger
Copy link
Member

Although JwtGenerationService seems to be reused in most cases, it does not appear to be utilized for signing Verifiable Presentations

this is only true for LDP-VCs, we use the JwtGenerationService for generating JWT-VPs.

That said, likely the best solution to do this is to introduce a JwsSignerProvider, which takes the private key ID and returns a JWSSigner instance. In the default case, that simply resolves the private key from the vault and returns an appropriate Nimbus implementation, just as it is done now.
However in your case, you would provide a special implementation, that takes a private key ID and returns your custom JWSSigner implementation.

These changes will have to be implemented in EDC:

  • introduce JwsSignerProvider interface,the resulting SPI package will expose Nimbus classes, specifically JWSSigner
  • TokenGenerationService#generate takes a String (=private key ID) instead of a Supplier<PrivateKey>
  • JwtGenerationService gets a new constructor taking a JwsSignerProvider
  • upon generating the token, the JWSSigner is resolved for a private key ID
  • a default JwsSignerProvider is provided by the token-core module.

When implementing a custom JWSSigner, a few things must be kept in mind:

  • the custom JWSSigner must likely be re-instantiated on every sign request, because the private key ID might be different.
  • the custom JWSSigner should not use our Vault interface
  • the module that contains the custom JWSSigner + extension must be available on the classpath at runtime

@petkivim
Copy link

petkivim commented Aug 6, 2024

Thanks, the described approach sounds good to us.

Are we going to split the work so that you will implement the changes to the EDC and for us it's enough to implement a custom JWSSigner? Alternatively, we could contribute to the EDC-related changes too. Also, do you have any initial estimate for implementing the changes to the EDC?

@paullatzelsperger
Copy link
Member

@petkivim First the Technical Committee must discuss and approve the Decision-Record (#4396). Then there will be an implementation PR for the EDC side (draft: #4395).

If you then want to contribute your specific custom JWSSigner to the EDC codebase, there is a specific process.

@petkivim
Copy link

petkivim commented Aug 6, 2024

@paullatzelsperger Thanks for the clarification! For now, we'll keep monitoring the progress of the process. Just let me know if any additional input is needed from our side.

Once we have implemented our custom JWSSigner, we consider contributing it to the EDC codebase.

@ndr-brt ndr-brt removed the triage all new issues awaiting classification label Aug 14, 2024
@ndr-brt
Copy link
Member

ndr-brt commented Aug 14, 2024

closed by #4403

@ndr-brt ndr-brt closed this as completed Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants