Conversation
Unwrap *keys.PrivateKey to fetch inner signer that satisfies `x509.MarshalPKCS8PrivateKey` expectations. This change allows passing keys.PrivateKey to createJKSWallet. The updated function will correctly marshal the private key for Oracle's JKS wallet creation.
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
|
This is a result of |
|
No need for backports, v16 doesn't seem affected. Pretty sure this is due to recent reshuffling of various crypto bits. |
Tener
added
the
no-changelog
| // unwrap *keys.PrivateKey if necessary. | ||
| if pk, ok := signer.(*keys.PrivateKey); ok { | ||
| signer = pk.Signer | ||
| } |
There was a problem hiding this comment.
LGTM
super nit. since api/utils/key is imported, could we change signer crypto.Signer, to signer *keys.PrivateKey in this file? maybe move x509.MarshalPKCS8PrivateKey to keys.PrivateKey too as a helper
There was a problem hiding this comment.
Hmm, there are quite a few options here if we want to refactor this further.
@nklaassen any ideas here? Looks like these changes are due to:
There was a problem hiding this comment.
Yeah my bad, sorry about this. I hate how all these crypto libs accept a key as any type and throw type safety out the window.
I think I would just do this instead of the type assertion, but that's harder to test against :)
diff --git a/tool/tsh/common/db.go b/tool/tsh/common/db.go
index cf107f20dc..f8c125b378 100644
--- a/tool/tsh/common/db.go
+++ b/tool/tsh/common/db.go
@@ -335,7 +335,7 @@ func databaseLogin(cf *CLIConf, tc *client.TeleportClient, dbInfo *databaseInfo)
if err := generateDBLocalProxyCert(keyRing.TLSPrivateKey, profile); err != nil {
return trace.Wrap(err)
}
- err = oracle.GenerateClientConfiguration(keyRing.TLSPrivateKey, dbInfo.RouteToDatabase, profile)
+ err = oracle.GenerateClientConfiguration(keyRing.TLSPrivateKey.Signer, dbInfo.RouteToDatabase, profile)
if err != nil {
return trace.Wrap(err)
}I think the right solution would be to never embed crypto.Signer and un-embed it from keys.PrivateKey, just because of how many places accept a crypto.Signer and then try to assert it to one of the standard library types, but I'm not sure I have time for that right now, it breaks a lot
For now maybe we could add a function api/utils/keys.MarshalSoftwarePrivateKeyPKCS8DER
func MarshalSoftwarePrivateKeyPKCS8DER(signer crypto.Signer) ([]byte, error) {
switch k := signer.(type) {
case *PrivateKey:
return MarshalSoftwarePrivateKeyPKCS8Der(k.Signer)
case *rsa.PrivateKey, *ecdsa.PrivateKey, ed25519.PrivateKey:
return x509.MarshalPKCS8PrivateKey(k)
default:
return nil, trace.BadParameter("unsupported key type: %T", signer)
}
}There was a problem hiding this comment.
Sounds good, I've added MarshalSoftwarePrivateKeyPKCS8DER now. PTAL.
public-teleport-github-review-bot
Bot
removed request for
atburke,
nklaassen and
tigrato
| // unwrap *keys.PrivateKey if necessary. | ||
| if pk, ok := signer.(*keys.PrivateKey); ok { | ||
| signer = pk.Signer | ||
| } |
There was a problem hiding this comment.
Yeah my bad, sorry about this. I hate how all these crypto libs accept a key as any type and throw type safety out the window.
I think I would just do this instead of the type assertion, but that's harder to test against :)
diff --git a/tool/tsh/common/db.go b/tool/tsh/common/db.go
index cf107f20dc..f8c125b378 100644
--- a/tool/tsh/common/db.go
+++ b/tool/tsh/common/db.go
@@ -335,7 +335,7 @@ func databaseLogin(cf *CLIConf, tc *client.TeleportClient, dbInfo *databaseInfo)
if err := generateDBLocalProxyCert(keyRing.TLSPrivateKey, profile); err != nil {
return trace.Wrap(err)
}
- err = oracle.GenerateClientConfiguration(keyRing.TLSPrivateKey, dbInfo.RouteToDatabase, profile)
+ err = oracle.GenerateClientConfiguration(keyRing.TLSPrivateKey.Signer, dbInfo.RouteToDatabase, profile)
if err != nil {
return trace.Wrap(err)
}I think the right solution would be to never embed crypto.Signer and un-embed it from keys.PrivateKey, just because of how many places accept a crypto.Signer and then try to assert it to one of the standard library types, but I'm not sure I have time for that right now, it breaks a lot
For now maybe we could add a function api/utils/keys.MarshalSoftwarePrivateKeyPKCS8DER
func MarshalSoftwarePrivateKeyPKCS8DER(signer crypto.Signer) ([]byte, error) {
switch k := signer.(type) {
case *PrivateKey:
return MarshalSoftwarePrivateKeyPKCS8Der(k.Signer)
case *rsa.PrivateKey, *ecdsa.PrivateKey, ed25519.PrivateKey:
return x509.MarshalPKCS8PrivateKey(k)
default:
return nil, trace.BadParameter("unsupported key type: %T", signer)
}
}
Co-authored-by: Nic Klaassen <nic@goteleport.com>
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
4 participants
Unwrap
*keys.PrivateKeyto fetch inner signer that satisfiesx509.MarshalPKCS8PrivateKeyexpectations. This change allows passingkeys.PrivateKeytocreateJKSWallet. The updated function will correctly marshal the private key for Oracle's JKS wallet creation.