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

Use authorized key parsing instead of reading binary certificates #3113

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

florian-feicht
Copy link

@florian-feicht florian-feicht commented Apr 22, 2024

Checklist:

The current implementation of the public key parsing a binary public key is needed (https://pkg.go.dev/golang.org/x/crypto/ssh#ParsePublicKey).
I would propose to switch to https://pkg.go.dev/golang.org/x/crypto/ssh#ParseAuthorizedKey as it supports a common string format, which is a lot easier to handle in secrets.

Example in go playground:
https://go.dev/play/p/doCqwWh6jpl

Signed-off-by: florian-feicht <florian.feicht@adesso-afida.com>
Signed-off-by: florian-feicht <florian.feicht@adesso-afida.com>
Signed-off-by: florian-feicht <florian.feicht@adesso-afida.com>
@florian-feicht florian-feicht marked this pull request as ready for review April 22, 2024 17:11
@@ -303,7 +303,7 @@ type SFTPEventSource struct {
Username *corev1.SecretKeySelector `json:"username,omitempty" protobuf:"bytes,3,opt,name=username"`
// Password required for authentication if any.
Password *corev1.SecretKeySelector `json:"password,omitempty" protobuf:"bytes,4,opt,name=password"`
// SSHKeySecret refers to the secret that contains SSH key
// SSHKeySecret refers to the secret that contains SSH key. Key needs to contain private key and public key.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It changes to require both private and public keys?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the implementation required private and public key already. Only adjusted the comment to make it clear.

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

Successfully merging this pull request may close these issues.

None yet

2 participants