Skip to content
This repository has been archived by the owner on Mar 2, 2024. It is now read-only.

KM498 ✅ Use ed25519 key algorithm for ArgoCD deploy key #894

Merged
merged 8 commits into from
Feb 17, 2022

Conversation

yngvark
Copy link
Contributor

@yngvark yngvark commented Jan 27, 2022

Description

Changes deploy key to use ed25519 algorithm for SSH keys.

Unfortunately, the Golang standard library haven't yet implemented support for marshalling a ed25519 private key into the OpenSSH format, which is required by Git, hence ArgoCD. To do this, I have copied the implementation from https://github.com/mikesmitty/edkey and documented its use. Also wrote a simple test to attempt to verify that produced private key is secure. The documentation includes a link to the issue (see link below) that tracks the implemenation of support for the OpenSSH format.

Background: golang/go#37132

Motivation and Context

https://trello.com/c/9GxBElfD/498-argocd-stops-working-2022-03-15-due-to-old-format-of-deploy-key

How to prove the effect of this PR?

Verify old key

  • In AWS console, open SSM -> Parameter store, search for "private" to find the privatekey for your cluster (something like /okctl/x/github/deploykeys/oslokommune/x/privatekey)
  • Show the secret, it should should a 4096 bit long RSA key

Remove and reapply ArgoCD

  • cluster declaration -> argoCD: false
  • okctl apply cluster ...
  • cluster declaration -> argoCD: true
  • okctl apply cluster ...

Verify new key

  • Refresh the secret in SSM. It should now show a much shorter key, as ed25519 keys are much shorter.
  • In https://github.com/oslokommune/your-iac-repo/settings/keys, you can verify that you have a new deploy key
  • In ArgoCD, choose an App and click "synchronize". This should trigger using the new key.

Also, in okctl state -> github, the public key should now start with "ed25519".

Additional info

The upgrade for this is in: oslokommune/okctl-upgrade#14

Types of changes

  • 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)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated the release notes (for the next release).

@yngvark yngvark changed the title Update key generation for ArgoCD KM498🐛Update key generation for ArgoCD Jan 27, 2022
@yngvark yngvark changed the title KM498🐛Update key generation for ArgoCD KM498🐛Use ed25519 key format for ArgoCD deploy key Jan 27, 2022
@yngvark yngvark changed the title KM498🐛Use ed25519 key format for ArgoCD deploy key KM498 🐛 Use ed25519 key format for ArgoCD deploy key Jan 27, 2022
@yngvark yngvark force-pushed the KM498-use_ed25519_ssh_key_format branch 5 times, most recently from 2921691 to eb411b0 Compare February 11, 2022 07:46
@yngvark yngvark changed the title KM498 🐛 Use ed25519 key format for ArgoCD deploy key KM498 🐛 Use ed25519 key algorithm for ArgoCD deploy key Feb 11, 2022
@yngvark yngvark force-pushed the KM498-use_ed25519_ssh_key_format branch from de27875 to 269deda Compare February 11, 2022 09:34
@yngvark yngvark marked this pull request as ready for review February 11, 2022 09:41
@yngvark yngvark requested a review from a team February 11, 2022 09:41
@yngvark yngvark changed the title KM498 🐛 Use ed25519 key algorithm for ArgoCD deploy key KM498 ✅ Use ed25519 key algorithm for ArgoCD deploy key Feb 11, 2022
Copy link
Member

@deifyed deifyed left a comment

Choose a reason for hiding this comment

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

LGTM. Can't say I feel comfortable QA'ing crypto related functions. Maybe ensure theres other eyes on this as well?

@yngvark yngvark force-pushed the KM498-use_ed25519_ssh_key_format branch from 269deda to 694eea3 Compare February 16, 2022 13:32
@sonarcloud
Copy link

sonarcloud bot commented Feb 16, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Feb 16, 2022

Codecov Report

Merging #894 (694eea3) into master (ab281a2) will increase coverage by 2.32%.
The diff coverage is 35.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #894      +/-   ##
==========================================
+ Coverage   44.19%   46.52%   +2.32%     
==========================================
  Files         277      239      -38     
  Lines        7811     7347     -464     
==========================================
- Hits         3452     3418      -34     
+ Misses       4359     3929     -430     
Impacted Files Coverage Δ
pkg/api/core/service_component.go 0.00% <0.00%> (ø)
pkg/api/core/service_kube.go 10.34% <0.00%> (-1.20%) ⬇️
pkg/api/core/service_objectstorage.go 0.00% <0.00%> (ø)
pkg/api/parameter_api.go 0.00% <ø> (ø)
pkg/apis/okctl.io/v1alpha1/cluster_v1alpha1.go 66.66% <ø> (+6.66%) ⬆️
...client/core/service_application_postgres_client.go 0.00% <0.00%> (ø)
pkg/client/core/service_argocd_client.go 0.00% <0.00%> (ø)
pkg/client/core/service_certificate_client.go 0.00% <0.00%> (ø)
pkg/client/core/service_cluster_client.go 0.00% <0.00%> (ø)
pkg/client/core/service_component.go 0.00% <0.00%> (ø)
... and 34 more

@yngvark yngvark merged commit 9b10c71 into master Feb 17, 2022
@yngvark yngvark deleted the KM498-use_ed25519_ssh_key_format branch February 17, 2022 06:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants