Minion authentication susceptible to a man-in-the-middle attack #2916

Closed
dzderic opened this Issue Dec 16, 2012 · 12 comments

4 participants

@dzderic

Currently, a minion verifies that the master is who they say they are by sending them a random token encrypted with the master's public key, and verifying that the master is able to decrypt the token.

The problem is, the message sent back to the minion has no integrity checks so it's possible to intercept their communications and alter certain information (like the AES key). I've put a proof-of-concept attack up here which does just that.

Two possible fixes off the top of my head are getting the master to digitally sign the messages or encrypting the whole message with the minion's public key. Of course, the best solution would be to get SSL implemented at the ØMQ-level, although that still looks quite far off.

@UtahDave
Salt Stack member

Thanks for the report. We'll get right on this.

@thatch45
Salt Stack member

Sorry I have been quiet on this one, my Sunday was completely packed! I will get this resolved today and a point release out ASAP

@thatch45 thatch45 closed this in 70422c9 Dec 17, 2012
@thatch45 thatch45 reopened this Dec 17, 2012
@thatch45
Salt Stack member

Sorry, bad commit message closed this issue, not quite done yet!

@thatch45 thatch45 added a commit that referenced this issue Dec 17, 2012
@thatch45 thatch45 Minion side additions for #2916 99257f8
@thatch45
Salt Stack member

@dzderic, I think I got it, I am going back to double check all cryptographic strings and logic. Please confirm and we will cut 0.11.1 with this security fix

@dzderic

RE: 99257f8 There's nothing stopping someone from removing the signature from the response and not getting it verified.

@thatch45
Salt Stack member

Yep, I just saw and added that

@thatch45
Salt Stack member

d9cc638
Fixes that one. I am running it again

@thatch45 thatch45 added a commit that referenced this issue Dec 18, 2012
@thatch45 thatch45 Minion side additions for #2916 2805fab
@thatch45
Salt Stack member

Ok, I think this is ready to roll, I am going over it once more and then cutting 0.11.1

@thatch45
Salt Stack member

This has been resolved, I am cutting 0.11.1 right now

@thatch45 thatch45 closed this Dec 20, 2012
@holmboe

Currently there is a package up at http://pypi.python.org/pypi/salt/0.11.1 but there is still no tag in git. It seems some step in the release process were skipped.

@holmboe

Scratch that. Some weirdness with my local clone. Troubleshooted it together with nkuttler on IRC, didn't really find what was the proble, but a new clone from github solved it.

@UtahDave
Salt Stack member

@holmboe often a git fetch --tags will fix that for you.

@holmboe holmboe pushed a commit to holmboe/salt that referenced this issue Jan 3, 2013
@thatch45 thatch45 Master side additions to fix #2916 b20137f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment