Fix MasterKeys.gen_signature signing raw PEM bytes (#66259)#69650
Open
dwoz wants to merge 1 commit into
Open
Conversation
``MasterKeys.gen_signature`` on 3007.x+ signed ``pub.public_bytes(PEM)`` directly, which carries the trailing newline emitted per RFC 7468. The auth-reply path transmits ``get_pub_str()`` = ``clean_key(pub)`` to the minion (newline stripped). A minion verifying ``payload["pub_sig"]`` against ``payload["pub_key"]`` therefore fails every time ``master_use_pubkey_signature: True`` is set: the master signed 451 bytes, the minion verifies 450. This is the same root cause as saltstack#68930 (fixed on 3006.x by saltstack#68934), but the 3007.x refactor moved ``gen_signature`` into ``MasterKeys`` and the whitespace-normalization patch didn't propagate. Apply ``clean_key()`` to the PEM before signing so the signed content matches what ``get_pub_str()`` sends. Fixes saltstack#66259
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
1 participant
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.
What does this PR do?
Applies
clean_key()to the master public-key PEM before signing it inMasterKeys.gen_signature, so the signed content matches the pub_key themaster transmits to minions in the auth reply. Without this, minions cannot
verify the signature whenever
master_use_pubkey_signature: Trueis set.What issues does this PR fix or reference?
Fixes #66259
Previous Behavior
MasterKeys.gen_signatureon 3007.x/3008.x/master signedpub.public_bytes(PEM)directly, including the trailing newline emittedper RFC 7468 (451 bytes for a 4096-bit RSA pub key). But
MasterKeys.get_pub_str()transmitsclean_key(pub)(450 bytes,newline stripped). A minion computing
verify_signature(sign_pub, payload["pub_key"], payload["pub_sig"])therefore verifies the wrongbyte range and rejects the master with:
...on every auth attempt.
Same root cause as #68930 (fixed on 3006.x by #68934). The 3007.x
refactor moved
gen_signatureintoMasterKeys.gen_signatureand thewhitespace-normalization patch did not carry forward because the new
code path derives the pub bytes from
public_bytes(PEM)instead ofreading them from disk.
New Behavior
gen_signaturenormalizes the pub PEM throughclean_key()beforepriv.sign(...), matching whatget_pub_str()sends. Signatureverification succeeds against the transmitted
pub_keyfor minions ofany version.
Merge requirements satisfied?
changelog/66259.fixed.md)tests/pytests/unit/crypt/test_crypt.py::test_master_keys_gen_signature_signs_clean_key)Commits signed with GPG?
No