-
Notifications
You must be signed in to change notification settings - Fork 0
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
Draft upgrade commands #1
base: main
Are you sure you want to change the base?
Conversation
eduardOrthopy
commented
Jun 7, 2024
- Code for discussions about upgrading/onboarding without creating new keys
- Code for experimenting with cert upgrade paths without original sig key change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you replace signing keys, existing sessions/entity statements will break.
Either we split the mTLS and JWT signing keys or we do key-rollover. I.e. keep the old key but no longer issue new keys.
Gematik already supports multiple signing keys, which is their idea on how to rotate keys.
@@ -35,7 +35,7 @@ public class KeyGeneratorCommand implements Callable<Integer> { | |||
@CommandLine.Option( | |||
names = {"-i", "--iss", "--issuer-uri"}, | |||
description = "the issuer uri of the 'Fachdienst' identiy provider", | |||
required = true) | |||
required = false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this optional? Pretty sure that's required for the self-signed certificate. No?
Otherwise we'd at least need to skip cert generation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I experimented with extracting it. but that did not yet pan out
|
||
public Integer call() throws Exception { | ||
|
||
var sigName = "sig"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I though about before: Maybe we should split the signing keys for JWTs and the mTLS client key. That would also reduce the risk of breaking existing tokens, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thoughts exactly. Personally, I think we will need to do that anyway for the exchange of mTLS keys.
I do not want to run through the whole Process every 5-6 months.
ehealthid-cli/src/main/java/com/oviva/ehealthid/cli/MTlsRefreshCommand.java
Outdated
Show resolved
Hide resolved
|
||
private JWK generateCertificate(JWK key) | ||
throws JOSEException, IOException, OperatorCreationException, CertificateEncodingException { | ||
// TODO: This is one huge playground method |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the starting point I used: https://connect2id.com/products/nimbus-oauth-openid-connect-sdk/examples/utils/self-signed-client-cert
// The idea here is to give users that currently have keys, that do not have | ||
// the relevant x5c field, the ability to generate a certificate for mTLS | ||
// without going through the fuzz of generating a new key and talking to | ||
// Gematik/BfArM about it. Which just generates work for everyone. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
…hCommand.java Co-authored-by: Thomas Richner <[email protected]>
So since 0.10 I completely removed the need to configure the mTLS keys as well as the So I think that PR is no longer necessary, or is it? |