Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Isolated fix for CVE-2017-7893? Which releases apart 2016.3.6 fix the issue? #48939

Open
carnil opened this issue Aug 4, 2018 · 9 comments

Comments

Projects
None yet
5 participants
@carnil
Copy link

commented Aug 4, 2018

Hi

While going within Debian trough a couple of CVEs for salt noticed the CVE-2017-7893 mentioned in the https://docs.saltstack.com/en/2017.7/topics/releases/2016.3.6.html which credits Frank Spierings as the discoverer).

In further research I only further found that in later releases the Credit mentioning was added, 24dea24 and 33c0644 .

Unfortunately neither https://bugzilla.redhat.com/show_bug.cgi?id=1572139 or https://bugzilla.novell.com/show_bug.cgi?id=1090665 is helping identifying the fix.

Can you share the fix which would neet to be cherry-picked (and/or which releases contain the fix)?

@gtmanfred

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2018

The fix should be just the second part of this

0a0f46f

We ended up turning this behavior back off by default though, in #40206

@anarcat

This comment has been minimized.

Copy link

commented Oct 24, 2018

We ended up turning this behavior back off by default though, in #40206

... why?

@gtmanfred

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2018

i don't remember @cro do you?

@anarcat

This comment has been minimized.

Copy link

commented Oct 24, 2018

and why should we consider CVE-2017-7893 fixed if the protection is disabled?

@anarcat

This comment has been minimized.

Copy link

commented Oct 30, 2018

ping? I'm trying to figure out how to backport those fixes into Debian for security updates, and it's not clear at all what the proper course of action is, or if this is even fixed in current stable releases.

Thanks for any update.

@cro

This comment has been minimized.

Copy link
Member

commented Oct 30, 2018

@anarcat, sorry, I was on vacation for a couple of days.

We ended up turning this behavior back off by default though, in #40206

... why?

We turned sign_pub_messages back off by default because it broke compatibility with older masters (#40203). We don't guarantee compatibility between newer minions and older masters, but we strive for it since sometimes folks upgrade their minions by accident and it's painful to recover when they won't talk to the current masters.

CVE-2017-7893 is fixed by the code in this PR. The best mitigation scenario is when sign_pub_messages is turned on. The behavior is documented in the master configuration file here: https://github.com/saltstack/salt/blob/develop/conf/master

@anarcat

This comment has been minimized.

Copy link

commented Oct 30, 2018

So the upgrade path for users is to apply the patch, then simultaneously turn on sign_pub_messages at once, is that right?

@cro

This comment has been minimized.

Copy link
Member

commented Oct 30, 2018

Yes, provided they don't have older masters that would be bitten by #40203.

@anarcat

This comment has been minimized.

Copy link

commented Oct 30, 2018

thanks. i have determined this cannot be fixed in the jessie version anyways (2014.1.13) as it doesn't have master signing in the first place, so I've skipped that issue, for what that's worth. hopefully a fix can be deployed in stretch (2016.11.2) nevertheless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.