unlock unlimited code runs for $79, limited time →
NOSSLETTER.techLIVE
back to OSSpedia
error-handlingtesting-strategyrefactoring

fix: empty SHA256 fingerprint for SSH known hosts entries on the UI

Project: argoproj/argo-cd | Language: Go | Concepts: error-handling, testing-strategy, refactoring | Difficulty: mid
Link: https://github.com/argoproj/argo-cd/pull/29401

The problem

After upgrading the golang.org/x/crypto dependency, Argo CD's UI and CLI began showing empty or incomplete SHA256 fingerprints for SSH known_hosts entries. Instead of displaying a valid fingerprint, the UI and CLI would show just the "SHA256:" prefix or nothing at all, making it impossible for users to verify SSH keys.

Why it's non-trivial

A naive fix might try to manually hash the key data or ignore error cases, but:

  • The upstream crypto library now requires the full key type in the input, not just the base64 data.
  • Known_hosts entries may be malformed or missing key types, and showing a bare "SHA256:" prefix is misleading.
  • The fix must ensure fingerprints are computed consistently across both listing and creation, and must not break on malformed keys.

The approach

1. Update the UI to show the full fingerprint on hover:
This makes it easier for users to see the full value even if it's clipped in the table.

-<div className='columns small-5'>
+<div className='columns small-5' title={cert.certInfo}>
    {cert.certInfo}

Adds the fingerprint as a tooltip so users can see the full value.


2. Remove unused fields and fix fingerprint computation:
Previously, the code tried to compute the fingerprint from incomplete data. Now, it reconstructs the full key line and parses it correctly.

-type SSHKnownHostsEntry struct {
-	Host string
-	// The type of the key
-	SubType string
-	// The data of the key, including the type
-	Data string
-	// The SHA256 fingerprint of the key
-	Fingerprint string
-}
+type SSHKnownHostsEntry struct {
+	Host string
+	// The type of the key
+	SubType string
+	// The base64 encoded key data, without the key type
+	Data string
+}

Removes the unused Fingerprint field and clarifies that Data is just the base64 portion.


3. Centralize and harden fingerprint calculation:
A new helper function builds the full known_hosts line and parses it, handling errors gracefully.

// sshKnownHostsFingerprint returns the fingerprint of the key held by an SSH
// known hosts entry. Entries are only validated syntactically when they are
// read from the ConfigMap, so the key data may still be unparseable - in that
// case we return an empty string rather than a bare "SHA256:" prefix.
func sshKnownHostsFingerprint(entry *SSHKnownHostsEntry) string {
	_, pubKey, err := certutil.TokenizedDataToPublicKey(entry.Host, entry.SubType, entry.Data)
	if err != nil {
		log.Warnf("Could not parse SSH known hosts key for host %q of type %q: %v", entry.Host, entry.SubType, err)
		return ""
	}
	return sshFingerprintSHA256(pubKey)
}

Ensures that only valid keys get a fingerprint, and logs a warning otherwise.


4. Use the new helper everywhere fingerprints are shown:
Both listing and creation now use the same logic and format.

-for _, entry := range sshKnownHosts {
-	if certutil.MatchHostName(entry.Host, selector.HostNamePattern) && (selector.CertSubType == "" || selector.CertSubType == "*" || selector.CertSubType == entry.SubType) {
-		certificates = append(certificates, appsv1.RepositoryCertificate{
-			ServerName:  entry.Host,
-			CertType:    "ssh",
-			CertSubType: entry.SubType,
-			CertInfo:    "SHA256:" + certutil.SSHFingerprintSHA256FromString(fmt.Sprintf("%s %s", entry.Host, entry.Data)),
-		})
-	}
+for _, entry := range sshKnownHosts {
+	if matchSSHKnownHostsEntry(entry, selector) {
+		certificates = append(certificates, appsv1.RepositoryCertificate{
+			ServerName:  entry.Host,
+			CertType:    "ssh",
+			CertSubType: entry.SubType,
+			CertInfo:    sshKnownHostsFingerprint(entry),
+		})
+	}

Ensures consistent fingerprint formatting and error handling.


5. Add robust tests for edge cases and format consistency:
Tests verify that:

  • Each known_hosts entry gets the correct fingerprint.
  • Malformed keys do not get a misleading fingerprint.
  • The fingerprint format is consistent between creation and listing.
func TestListCertificateSSHUnparseableKey(t *testing.T) {
	t.Parallel()
	clientset := certClientsetWithKnownHosts("foo.example.com ssh-rsa bm90LWEta2V5\n")
	db := NewDB(testNamespace, settings.NewSettingsManager(t.Context(), clientset, testNamespace), clientset)

	certList, err := db.ListRepoCertificates(t.Context(), &CertificateListSelector{CertType: "ssh"})
	require.NoError(t, err)
	require.Len(t, certList.Items, 1)
	assert.Equal(t, "foo.example.com", certList.Items[0].ServerName)
	assert.Empty(t, certList.Items[0].CertInfo)
}

Checks that an unparseable key does not get a fake fingerprint.

func TestCreateSSHKnownHostEntriesFingerprintFormat(t *testing.T) {
	// ... setup omitted ...
	assert.True(t, strings.HasPrefix(created.Items[0].CertInfo, "SHA256:"),
		"created entry reports %q, which is not in ssh-keygen fingerprint format", created.Items[0].CertInfo)

	assert.Equal(t, listed.Items[0].CertInfo, created.Items[0].CertInfo,
		"CreateRepoCertificate and ListRepoCertificates disagree on the fingerprint")
}

Ensures that both create and list APIs agree on fingerprint format.

From the review thread

No notable review discussion on this PR.

Takeaways

  • Always parse and validate input before computing or displaying derived values.
    Don't assume input is well-formed; handle parse failures explicitly to avoid misleading users.

  • Centralize logic for formatting and error handling.
    Use helper functions so all code paths (listing, creation, etc.) produce consistent outputs and error handling.

  • Test both normal and edge cases, including malformed input.
    Good tests should cover not just the happy path, but also broken or unexpected input, and verify that the system fails gracefully.