search

containers / common

Location for shared common files in github.com/containers repos.
Apache License 2.0
193 stars 201 forks source link

Use skeema/knownhosts, not x/crypto/ssh/knownhosts #2212

Closed l0rd closed 3 weeks ago

l0rd commented 1 month ago

TL;DR This PR addresses this issue. In this containers/podman PR, I have added some specific tests.

image

The package golang.org/x/crypto/ssh/knownhosts has an issue when an SSH server has many public keys (i.e., supports multiple crypto algorithms).

For instance, if the local known_hosts file entries don't match the first SSH server key but match other SSH server keys, the handshake fails with a key mismatch error.

See https://github.com/golang/go/issues/29286 and https://github.com/containers/podman/issues/23575.

Package github.com/skeema/knownhosts is a wrapper of x/crypto/ssh/knownhosts that addresses this issue.

This commit replaces the usage of x/crypto/ssh/knownhosts in containers/common with github.com/skeema/knownhosts.

rhatdan commented 3 weeks ago

LGTM

l0rd commented 3 weeks ago

@containers/podman-maintainers PTAL, I have added some new podman tests that are passing with the changes included in this PR.

openshift-ci[bot] commented 3 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: l0rd, Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/containers/common/blob/main/OWNERS)~~ [Luap99] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment