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

Critical options String values double-length encoding #207

Merged
merged 5 commits into from
Mar 22, 2024

Conversation

jeromegn
Copy link
Contributor

There was recent clarification on how critical options values should be read: https://bugzilla.mindrot.org/show_bug.cgi?id=2389

The gist of it is that values can be of any kind, but they generally are strings. To support more types, the value is encoded in a more generic way. For a string, this means there are 2 length prefixes (4 bytes for the total length of the value and then 4 bytes for the length of the string).

This PR aims at fixing this issue.

@jeromegn jeromegn requested a review from tarcieri March 21, 2024 17:35
@jeromegn
Copy link
Contributor Author

Looks like I broke some tests, going to look at that.

@jeromegn
Copy link
Contributor Author

I've had to special case 0-len values because we don't want to double-length-prefix those.

@tarcieri tarcieri merged commit 01a9eba into RustCrypto:master Mar 22, 2024
13 checks passed
@jeromegn jeromegn deleted the fix-crit-options-values branch March 22, 2024 13:21
baloo added a commit to baloo/SSH that referenced this pull request Apr 12, 2024
Added:
- impl `decode_as` for `KeypairData` ([RustCrypto#211])

Changed:
- clarify SSH vs OpenSSH formats ([RustCrypto#206])

Fixed:
- fix `certificate::OptionsMap` encoding ([RustCrypto#207])
- fixup `EcdsaPrivateKey` Debug impl ([RustCrypto#210])

[RustCrypto#206]: RustCrypto#206
[RustCrypto#207]: RustCrypto#207
[RustCrypto#210]: RustCrypto#210
[RustCrypto#211]: RustCrypto#211
@baloo baloo mentioned this pull request Apr 12, 2024
baloo added a commit that referenced this pull request Apr 12, 2024
Added:
- impl `decode_as` for `KeypairData` ([#211])

Changed:
- clarify SSH vs OpenSSH formats ([#206])

Fixed:
- fix `certificate::OptionsMap` encoding ([#207])
- fixup `EcdsaPrivateKey` Debug impl ([#210])

[#206]: #206
[#207]: #207
[#210]: #210
[#211]: #211
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.

2 participants