host matching: handle wildcards with non-standard port #10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In OpenSSH, wildcard host pattern entries in a known_hosts file can match hosts regardless of their port number. However, x/crypto/ssh/knownhosts does not follow this behavior, instead requiring strict port equality; see bug golang/go#52056 for background. This commit implements a workaround in skeema/knownhosts, which is enabled when using the NewDB constructor. Conceptually, the workaround works like this: * At constructor time, when re-reading the known_hosts file (originally to look for @cert-authority lines), also look for lines that have wildcards in the host pattern and no port number specified. Track these lines in a new field of the HostKeyDB struct for later use. * When a host key callback returns no matches (KeyError with empty Want slice) and the host had a nonstandard (non-22) port number, try the callback again, this time manipulating the host arg to be on port 22. * If this second call returned nil error, that means the host key now matched a known_hosts entry on port 22, so consider the host as known. * If this second call returned a KeyError with non-empty Want slice, filter down the resulting keys to only correspond to lines with known wildcards, using the preprocessed information from the first step. This ensures we aren't incorrectly returning non-wildcard entries among the Want slice. The implementation for the latter 3 bullets gets embedded directly in the host key callback returned by HostKeyDB.HostKeyCallback, by way of some nested callback wrapping. This only happens if the first bullet actually found at least one wildcard in the file.
|
Merging momentarily based on positive feedback in #9 (comment) providing real-world confirmation it's working correctly, in addition to the unit tests |
svc-squareup-copybara
pushed a commit
to cashapp/misk
that referenced
this pull request
Dec 3, 2024
| Package | Package file | Manager | Update | Change | |---|---|---|---|---| | [skeema](https://github.com/skeema/skeema) | misk/bin/hermit | hermit | patch | `1.12.0` -> `1.12.1` | --- ### Release Notes <details> <summary>skeema/skeema (skeema)</summary> ### [`v1.12.1`](https://github.com/skeema/skeema/releases/tag/v1.12.1) - **MySQL 9.0 and 9.1** support ([`7c94fcb`](skeema/skeema@7c94fcb), [`47bfc0a`](skeema/skeema@47bfc0a), [`a0a060a`](skeema/skeema@a0a060a)) - The new `VECTOR` column type is supported. In [`skeema diff`](https://www.skeema.io/docs/commands/diff/) and [`skeema push`](https://www.skeema.io/docs/commands/push/), altering a column type between `VECTOR` and any other sufficiently-large binary type is permitted as a [safe operation](https://www.skeema.io/docs/features/safety/#unsafe-change-detection) since the conversion is non-lossy. - MySQL 9 finally processes "inline" foreign key definitions (that is, `REFERENCES` clause in a column definition). These are supported as-is in Skeema. - **MariaDB 11.5 and 11.6** support ([`6165c90`](skeema/skeema@6165c90), [`f24ad30`](skeema/skeema@f24ad30)) - MariaDB 11.5 changes the default collation for Unicode charsets to use uca1400\_ai_ci collations, which are fully supported in Skeema. - MariaDB 11.5 solves the `TIMESTAMP` col type's previous Y2K38 limitation, and Skeema's [lint-has-time](https://www.skeema.io/docs/options/#lint-has-time) annotation message has been adjusted accordingly. - **`CHECK` constraint improvement**: When a diff only affects the *name* of a `CHECK` constraint without modifying its check expression, [`skeema diff`](https://www.skeema.io/docs/commands/diff/) and [`skeema push`](https://www.skeema.io/docs/commands/push/) now ignore this cosmetic change by default. This improves compatibility with external [OSC tools](https://www.skeema.io/docs/features/osc/), which inherently need to rename `CHECK` constraints as part of their operation. This new behavior can be overridden by enabling the [--exact-match](https://www.skeema.io/docs/options/#exact-match) option. ([`f000616`](skeema/skeema@f000616)) - **[Event](https://www.skeema.io/docs/features/events/) handling** improvements and fixes ([Skeema Premium](https://www.skeema.io/download/)) - When an event diff only included a change to the `DEFINER` clause, and no other differences, the `ALTER EVENT` emitted by Skeema was not valid SQL (despite conforming to syntax in the MySQL and MariaDB manuals). To fix this situation, the SQL will now also include an additional no-op clause, such as `ENABLE` for an event that is already enabled. - Several dump normalizations for `CREATE EVENT` statements were inadvertently omitted the first time an event was dumped by [`skeema init`](https://www.skeema.io/docs/commands/init/) or [`skeema pull`](https://www.skeema.io/docs/commands/pull/). - If any workspace query failed (e.g. query timeout), and any events were present in the \*.sql definitions, a panic would result instead of the intended workspace query failure error message. ([#​229](skeema/skeema#229)) - **[SSH tunnel](https://www.skeema.io/docs/features/ssh/) enhancements** ([Skeema Premium](https://www.skeema.io/download/)) - CAs, which are indicated in the known_hosts file using `@cert-authority` lines, are now fully supported. ([skeema/knownhosts#8](skeema/knownhosts#8), [skeema/knownhosts#9](skeema/knownhosts#9)) - known_hosts lines using non-default ports are now matched properly. ([skeema/knownhosts#10](skeema/knownhosts#10)) - If any hand-written \*.sql files use the optional **`CREATE OR REPLACE` SQL syntax**, Skeema now parses and ignores the `OR REPLACE` clause. Previously, use of this syntax would prevent Skeema from parsing the statement. ([`6805737`](skeema/skeema@6805737)) - Enhancements for [Docker workspaces](https://www.skeema.io/docs/features/workspaces/#docker-workspaces) - Significant performance improvements for several common situations ([`d348249`](skeema/skeema@d348249), [`ca85df7`](skeema/skeema@ca85df7), [`7a40155`](skeema/skeema@7a40155)) - When using Percona Server 8.x, the Docker image / point release selection logic has been improved ([`e09350c`](skeema/skeema@e09350c), [`fe55d62`](skeema/skeema@fe55d62), [`af1b3b5`](skeema/skeema@af1b3b5)) - When a redundant non-unique index is flagged by [lint-dupe-index](https://www.skeema.io/docs/options/#lint-dupe-index), the annotation message is now clearer (since false positives may be possible) and suggests making the index be `INVISIBLE` / `IGNORED` before dropping ([#​238](skeema/skeema#238), [#​237](skeema/skeema#237)) - MariaDB's August 2024 point releases have changed the formatting of compressed columns in `SHOW CREATE TABLE`, which affected Skeema's [diff logic safeguards](https://www.skeema.io/docs/features/safety/#table-introspection-validation). This change is now handled and compressed columns are fully supported again. ([`49aed41`](skeema/skeema@49aed41)) - Minor wording changes in log messages and help text, for consistency. ([`4f8fa44`](skeema/skeema@4f8fa44), [`5f7598e`](skeema/skeema@5f7598e)) **Thank you** to all code contributors and issue reporters! An [installation guide](https://www.skeema.io/docs/install/) and [full documentation](https://www.skeema.io/docs/) are available on our website [skeema.io](https://www.skeema.io/). </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 6pm every weekday,before 2am every weekday" in timezone Australia/Melbourne, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). GitOrigin-RevId: 90a37c8a0c3c86b3fa245502cfeefd429129ebed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In OpenSSH, wildcard host pattern entries in a known_hosts file can match hosts regardless of their port number. However, x/crypto/ssh/knownhosts does not follow this behavior, instead requiring strict port equality; see bug golang/go#52056 for background, as well as discussion in previous PR #9.
This PR implements a workaround in skeema/knownhosts, which is enabled when using the NewDB constructor. Conceptually, the workaround works like this:
At constructor time, when re-reading the known_hosts file (originally to look for @cert-authority lines), also look for lines that have wildcards in the host pattern and no port number specified. Track these lines in a new field of the HostKeyDB struct for later use.
When a host key callback returns no matches (KeyError with empty Want slice) and the host had a nonstandard (non-22) port number, try the callback again, this time manipulating the host arg to be on port 22.
If this second call returned nil error, that means the host key now matched a known_hosts entry on port 22, so consider the host as known.
If this second call returned a KeyError with non-empty Want slice, filter down the resulting keys to only correspond to lines with known wildcards, using the preprocessed information from the first step. This ensures we aren't incorrectly returning non-wildcard entries among the Want slice.
The implementation for the latter 3 bullets gets embedded directly in the host key callback returned by HostKeyDB.HostKeyCallback, by way of some nested callback wrapping. This only happens if the first bullet actually found at least one wildcard in the file.