Repository navigation
Conversation
Adds a `sha2` feature, on by default, and makes the `sha2` dependency
optional. It gates the APIs that hash with `sha2`: key fingerprints,
`Certificate::validate{,_at}`, the `DotSsh` fingerprint lookups,
`HashAlg::digest`, `SshSig::{sign, signed_data}`, `PrivateKey::sign`,
`PublicKey::verify`, the `AssociatedHashAlg` impls for `Sha256` and
`Sha512`, and the `sha2` re-export. The `ed25519`, `p256`, `ppk` and
`rsa` features enable it, because their implementations use SHA-2
directly.
`Digest` now comes from `encoding::digest` (the same trait), so
`AssociatedHashAlg` and the `*_digest`/`*_prehash` signing and
verification methods remain available without `sha2`. In debug builds
without `sha2`, `certificate::Builder::sign` checks the new certificate
with `verify_signature` instead of `validate_at`.
With this, `ssh-key` with `default-features = false`, and therefore
`ssh-protocol`, no longer depends on `sha2`.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1e56d65e-86a1-43d2-89bd-6c4801475f3c
This branch has not been deployed
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.
Motivation
ssh-keyalways depends onsha2, even withdefault-features = false, although only a few of its APIs hash with it: key fingerprints, certificate validation,HashAlg::digest, and thesshsigmethods that hash the message themselves. Builds that use none of these, or that get SHA-2 from another implementation, still compile and shipsha2.I ran into this with a russh fork that has a SymCrypt backend (cataggar/russh#27). Its SymCrypt-only build must do all of its cryptography, randomness included, with SymCrypt, and CI fails that build if crates that implement cryptography, such as
sha2, are in its normal dependency graph. The build usesssh-keyfor its key, certificate and signature types and their encodings, and does its hashing and signing with SymCrypt. Even so,ssh-keystill brings insha2, the only such crate left, so the check needs an exception for it. I've made this change in a fork for that build (cataggar#1), and would like to get it from anssh-keyrelease instead.ssh-protocolis in the same position: it depends onssh-keywithdefault-features = falseand calls none of these APIs, but still getssha2.Compatibility
sha2becomes a default feature, so:ed25519,p256,ppk,rsaorcrypto, which enablesha2.default-features = falsethat enable none of those features and use a gated API (listed below) need to addfeatures = ["sha2"]. Since 0.7 is still in release candidates, now seems the time for that.ssh-protocolre-exportsPublicKey,PrivateKey,Fingerprint,HashAlgandCertificate, so its users lose the gated methods too, unless something else in their build enablesssh-key/sha2. If you'd ratherssh-protocolkeep them by default, I can add a defaultsha2feature to it that enablesssh-key/sha2.Proposal
This makes the
sha2dependency optional, behind asha2feature (sha2 = ["dep:sha2"], likesha1) that's on by default, and gates on it the APIs that hash withsha2:Fingerprint::new, andfingerprint()onKeyData,PublicKeyandPrivateKeyCertificate::validate(which still also needsstd) andCertificate::validate_atDotSsh::private_key_with_fingerprintandDotSsh::public_key_with_fingerprintHashAlg::digestSshSig::sign,SshSig::signed_data,PrivateKey::signandPublicKey::verifyAssociatedHashAlgimpls forSha256andSha512, andpub use sha2These remain without it, so code with another SHA-2 implementation can still create and verify signatures, and make fingerprints from digests it computes:
Fingerprinttype, with parsing,Display,serdeand randomartAssociatedHashAlg, the*_digestand*_prehashsshsigmethods, andSshSig::signed_data_for_prehash. They now takeDigestfromencoding::digest, the samedigest0.11 trait thatsha2re-exports.Certificate::verify_signatureed25519,p256,ppkandrsaenablesha2, becausessh-key's own code for them uses SHA-2:sk-*signatures cover SHA-256 hashes of the application and the message,rsasigns and verifies withrsa-sha2-256andrsa-sha2-512, andppkuses HMAC-SHA-256. (dsaandppkenabledep:sha1the same way.)p384,p521,dsaandencryptiondon't, although their dependencies pull insha2anyway. If you'd rather every algorithm feature enabledsha2, so that fingerprints come with any of them, that's one word per feature.Also:
certificate::Builder::signchecks the new certificate withvalidate_atand the signing key's fingerprint. Withoutsha2, it callsverify_signatureinstead.sshsigare gated onsha2.CHANGELOG.mdalone, since its entries are written at release time.ssh-key.ymlrunscargo test --no-default-features --features ecdsa,rand_core,std. The feature powerset never enablesstdand runs in release mode, so without this step, CI would never buildstd, or the debug-build certificate check, withoutsha2. The powerset itself grows from 24 to 44 combinations.Prior art
#27 removed the
fingerprintfeature in 2022, to cut down on features and feature-related complexity, "especially as fingerprints are important baseline functionality". This does add a feature back, but keeps fingerprints on by default. It's named after the dependency, likesha1, rather thanfingerprint, because it gates more than fingerprints and isn't meant to undo #27.Validation
The same commit passed CI, with this repo's workflows, in my fork (cataggar#2). Locally, I also ran the
ssh-key.ymltest and no_std jobs on stable and 1.85.0, plus clippy, rustdoc, rustfmt and typos.--all-features, the same tests run as onmaster: 100 and 180.cargo tree -e normallists nosha2forssh-key --no-default-features --features alloc,ecdsa,rand_core,std, nor forssh-protocol.sha2exception.sha2have unresolved intra-doc links to the gated methods, as partial-feature builds already have for other features. With--all-features, as in CI and on docs.rs, they're clean.