Require aesfunc tokens #7356

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
@dlanderson
Contributor

dlanderson commented Sep 19, 2013

See also #7353

The master currently allows any minion with an accepted key to send commands to the master as any other minion. This pull request requires that commands sent from a minion to the master include a token encrypted with the minion's public key.

Unfortunately, requiring this token breaks backwards compatibility with all previous minion versions. I have added "require_aes_tokens: False" to the master config, and the master will allow old minions to send commands without a token unless require_aes_tokens is set to True. I gave it a warn_until version 0.20 to allow for salt admins to upgrade their minions to at least 0.17 (assuming this pull request gets included in 0.17)

I think salt should, by default, not trust the minions as much as possible. This greatly reduces the ability of evil minions to do nasty things.

@thatch45

This comment has been minimized.

Show comment Hide comment
@thatch45

thatch45 Sep 19, 2013

Owner

We should not refer to the token as aes and the verify as rsa, since these are routines that can be updated in the future and they are not cryptographic per-say. This is just a simple id challenge.
Also, this warning will FLOOD the logs and be a very bad thing. This warning should just be posted once when the master starts up

Owner

thatch45 commented Sep 19, 2013

We should not refer to the token as aes and the verify as rsa, since these are routines that can be updated in the future and they are not cryptographic per-say. This is just a simple id challenge.
Also, this warning will FLOOD the logs and be a very bad thing. This warning should just be posted once when the master starts up

@dlanderson

This comment has been minimized.

Show comment Hide comment
@dlanderson

dlanderson Sep 19, 2013

Contributor

@thatch45 you're right. I renamed them to be properly descriptive and moved the log deprecation so it only logs once, but I think I'm going to close this and refactor some things on a new branch.

Contributor

dlanderson commented Sep 19, 2013

@thatch45 you're right. I renamed them to be properly descriptive and moved the log deprecation so it only logs once, but I think I'm going to close this and refactor some things on a new branch.

@dlanderson dlanderson closed this Sep 19, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment