Repository navigation
ssh-encoding: add initial Mpint 16384-bit size limit - #615
Merged
Merged
Conversation
OpenSSH enforces the following limit on the size of `mpint`: https://github.com/openssh/openssh-portable/blob/e416edf/sshbuf.h#L41 This commit enforces the same limit for the standard `Mpint` constructors. Unfortunately we support infallible `From<Uint>` impls that can't enforce the size limit, which are currently TODOs. Perhaps before the next `ssh-key` release we can consider making another breaking release of `ssh-encoding` to address those TODOs properly. See also: RustCrypto/RSA#793
tarcieri
added a commit
that referenced
this pull request
Nov 10, 2026
Move nested decoding, length bounds, completion checks, and failure state into a shared `Decoder<R>` type which owns the underlying reader. This removes considerable complexity from the reader types, including their individual `Reader::read_prefixed` implementations and the supporting macro, leaving them responsible only for supplying bytes and reporting the remaining length. Nested decoding now uses a common `read_nested` implementation which temporarily narrows the decoder's bounds. Each nesting level uses the same `Decoder<R>` type, preserving `no_alloc` support without the recursively nested generic reader types that previously caused monomorphization bloat. This is a breaking change: `Decode` implementations now accept `&mut Decoder<impl Reader>`, which is slightly uglier from an API perspective but enables a much better underlying implementation. A blanket `From<R>` implementation keeps call sites concise by allowing `Reader`s to be converted into a `Decoder` with `into()`.. We are also motivated by #615 to make another breaking `ssh-encoding` release before the next ssh-key release: making the remaining infallible `Mpint` conversions fallible would allow its size limits to be enforced consistently. Because this is a breaking change, the versions of `ssh-cipher`, `ssh-derive`, and `ssh-encoding` have all been bumped to v0.4.0-pre.
tarcieri
added a commit
that referenced
this pull request
Nov 10, 2026
Move nested decoding, length bounds, completion checks, and failure state into a shared `Decoder<R>` type which owns the underlying reader. This removes considerable complexity from the reader types, including their individual `Reader::read_prefixed` implementations and the supporting macro, leaving them responsible only for supplying bytes and reporting the remaining length. Nested decoding now uses a common `read_nested` implementation which temporarily narrows the decoder's bounds. Each nesting level uses the same `Decoder<R>` type, preserving `no_alloc` support without the recursively nested generic reader types that previously caused monomorphization bloat. This is a breaking change: `Decode` implementations now accept `&mut Decoder<impl Reader>`, which is slightly uglier from an API perspective but enables a much better underlying implementation. A blanket `From<R>` implementation keeps call sites concise by allowing `Reader`s to be converted into a `Decoder` with `into()`.. We are also motivated by #615 to make another breaking `ssh-encoding` release before the next ssh-key release: making the remaining infallible `Mpint` conversions fallible would allow its size limits to be enforced consistently. Because this is a breaking change, the versions of `ssh-cipher`, `ssh-derive`, and `ssh-encoding` have all been bumped to v0.4.0-pre.
tarcieri
added a commit
that referenced
this pull request
Nov 10, 2026
Move nested decoding, length bounds, completion checks, and failure state into a shared `Decoder<R>` type which owns the underlying reader. This removes considerable complexity from the reader types, including their individual `Reader::read_prefixed` implementations and the supporting macro, leaving them responsible only for supplying bytes and reporting the remaining length. Nested decoding now uses a common `read_nested` implementation which temporarily narrows the decoder's bounds. Each nesting level uses the same `Decoder<R>` type, preserving `no_alloc` support without the recursively nested generic reader types that previously caused monomorphization bloat. This is a breaking change: `Decode` implementations now accept `&mut Decoder<impl Reader>`, which is slightly uglier from an API perspective but enables a much better underlying implementation. A blanket `From<R>` implementation keeps call sites concise by allowing `Reader`s to be converted into a `Decoder` with `into()`.. We are also motivated by #615 to make another breaking `ssh-encoding` release before the next ssh-key release: making the remaining infallible `Mpint` conversions fallible would allow its size limits to be enforced consistently. Because this is a breaking change, the versions of `ssh-cipher`, `ssh-derive`, and `ssh-encoding` have all been bumped to v0.4.0-pre.
This file contains hidden or 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
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.
OpenSSH enforces the following limit on the size of
mpint:https://github.com/openssh/openssh-portable/blob/e416edf/sshbuf.h#L41
This commit enforces the same limit for the standard
Mpintconstructors. Unfortunately we support infallibleFrom<Uint>impls that can't enforce the size limit, which are currently TODOs.Perhaps before the next
ssh-keyrelease we can consider making another breaking release ofssh-encodingto address those TODOs properly.See also: RustCrypto/RSA#793