5 ms·

> With the right sandboxing techniques, SELinux and mitigations could prevent the attacker from doing anything with root permissions. Please review this commit

> With the right sandboxing techniques, SELinux and mitigations could prevent the attacker from doing anything with root permissions. Please review this commit[0] where the sandbox detection was “improved”. [0] https://git.tukaani.org/?p=xz.git;a=commitdiff;h=328c52da8a2bbb81307644efdb58db2c422d9ba7 https://git.tukaani.org/?p=xz.git;a=commitdiff;h=328c52da8a2...

ronsor 3y ago
Well, the definition of "improve" depends on one's goals.
Denvercoder9 3y ago
That one's a separate attack vector, which is seemingly unused in the sshd attack. It only disables sandboxing of the xzdec(2) utility, which is not used in the sshd attack.
- formerly_proven 3y ago
  Which strongly suggests that they planned and/or executed more backdoors via Jia Tan’s access.
  - pja 3y ago
    I guess xzdec was supposed to sandbox itself where possible so they disabled the sandbox feature check in the build system so that future payload exploits passed to xzdec wouldn’t have to escape the sandbox in order to do anything useful? Sneaky.
glandium 3y ago
Oh, that one is interesting, because it only breaks it in cmake.
- sn 3y ago
  I wonder if there is anything else cmake related that should be looked at. Wasn't cmake support originally added to xz to use with Windows and MSVC?
  - glandium 3y ago
    But that's a check for a Linux feature. So the more interesting question would be, what in the Linux world might be building xz-utils with cmake, I guess using ExternalProject_Add or something similar.
    - sn 3y ago
      Yes this is Linux. At this time we don't know exactly how much is affected and what originally drew the attention of the attacker(s).
dolmen 3y ago
I can't blame anyone who has missed that dot dissimulated at the beginning of the line. https://git.tukaani.org/?p=xz.git;a=commitdiff;h=f9cf4c05edd14dedfe63833f8ccbe41b55823b00 https://git.tukaani.org/?p=xz.git;a=commitdiff;h=f9cf4c05edd...
- Muromec 3y ago
  I specifically opened this diff to search for a sneaky dot, knowing it’s there, and wasn’t able to find it until I checked the revert patch
  - Aquilla_ 3y ago
    Same, I knew a sneaky dot was in the diff but had to ctrl-f the diff to find it.
- gouggoug 3y ago
  For people like me whose C knowledge is poor, can you explain why this dot is significant? What does it do in actuality?
  - ezekg 3y ago
    As far as I can tell, the check is to see if a certain program compiles, and if so, disable something. The dot makes it so that it always fails to compile and thus always disables that something.
    - _nalply 3y ago
      > if a certain program compiles, and if so, disable something. Tiny correction: [...] enable something. The idea is: If that certain program does not compile it is because something is not available on the system and therefore needs to be disabled. That dot undermines that logic. The program fails because of a syntax error caused by the dot and not because something is missing. It is easy to overlook because that dot is tiny and there are many such tests. I had a similar problem with unit testing of a library. Expected failures need to be tested as well. As an example imagine writing a matrix inversion library. Then you need to verify that you get something like a division by zero error if you invert the zero matrix. You write a unit test for that and by mistake you insert a syntax error. Then you run the unit test and it fails as expected but not in the correct way. It's subtle. It fails as expected but it fails because of unexpected wrong causes. The solution: Check the errors carefully!
db48x 3y ago
Yes, but don't forget that there are different kinds of sandboxes. SELinux never needs the cooperation of any program running on the system in order to correctly sandbox things. No change to Xz could ever make SELinux less effective.
- ynik 3y ago
  But don't forget that xz is also used as part of dpkg for unpacking packages. The whole purpose of dpkg is to update critical system packages. Any SELinux policy that protects from a backdoored dpkg/xz installing a rootkit during the next kernel security update; will also prevent installing real kernel security updates. The particular way of attack in this OpenSSH backdoor can maybe be prevented; but we've got to realize that the attacker already had full root permissions and there's no way of protecting from that.
  - db48x 3y ago
    SELinux policies are much more subtle than that. You don’t restrict what xz or liblzma can do, you restrict what the whole process can do. That process is either sshd or dpkg, and you can give them completely different access to the system, so that if dpkg tries to launch an interactive shell it fails, while sshd fails if it tries to overwrite a system file such as /bin/login or whatever. Neither would ordinarily do that, but the payload delivered via the back door might attempt it and wouldn’t succeed. And you would get a report stating what had happened, so if you’re paying attention the back door starts to become obvious. Also I think dpkg switched to Zstd, didn’t it? Or am I misremembering? But you’re not wrong; ultimately both sshd and dpkg are critical infrastructure. SELinux can prevent them from doing completely wrong things, but obviously it wouldn’t be useful for it to prevent them from doing their jobs. And those jobs are security critical already. SELinux is not a panacea, merely defense in depth.

ronsor 3y ago

Well, the definition of "improve" depends on one's goals.

Denvercoder9 3y ago

That one's a separate attack vector, which is seemingly unused in the sshd attack. It only disables sandboxing of the xzdec(2) utility, which is not used in the sshd attack.

formerly_proven 3y ago
Which strongly suggests that they planned and/or executed more backdoors via Jia Tan’s access.
- pja 3y ago
  I guess xzdec was supposed to sandbox itself where possible so they disabled the sandbox feature check in the build system so that future payload exploits passed to xzdec wouldn’t have to escape the sandbox in order to do anything useful? Sneaky.

glandium 3y ago

Oh, that one is interesting, because it only breaks it in cmake.

sn 3y ago
I wonder if there is anything else cmake related that should be looked at. Wasn't cmake support originally added to xz to use with Windows and MSVC?
- glandium 3y ago
  But that's a check for a Linux feature. So the more interesting question would be, what in the Linux world might be building xz-utils with cmake, I guess using ExternalProject_Add or something similar.
  - sn 3y ago
    Yes this is Linux. At this time we don't know exactly how much is affected and what originally drew the attention of the attacker(s).

dolmen 3y ago

I can't blame anyone who has missed that dot dissimulated at the beginning of the line. https://git.tukaani.org/?p=xz.git;a=commitdiff;h=f9cf4c05edd14dedfe63833f8ccbe41b55823b00 https://git.tukaani.org/?p=xz.git;a=commitdiff;h=f9cf4c05edd...

Muromec 3y ago
I specifically opened this diff to search for a sneaky dot, knowing it’s there, and wasn’t able to find it until I checked the revert patch
- Aquilla_ 3y ago
  Same, I knew a sneaky dot was in the diff but had to ctrl-f the diff to find it.
gouggoug 3y ago
For people like me whose C knowledge is poor, can you explain why this dot is significant? What does it do in actuality?
- ezekg 3y ago
  As far as I can tell, the check is to see if a certain program compiles, and if so, disable something. The dot makes it so that it always fails to compile and thus always disables that something.
  - _nalply 3y ago
    > if a certain program compiles, and if so, disable something. Tiny correction: [...] enable something. The idea is: If that certain program does not compile it is because something is not available on the system and therefore needs to be disabled. That dot undermines that logic. The program fails because of a syntax error caused by the dot and not because something is missing. It is easy to overlook because that dot is tiny and there are many such tests. I had a similar problem with unit testing of a library. Expected failures need to be tested as well. As an example imagine writing a matrix inversion library. Then you need to verify that you get something like a division by zero error if you invert the zero matrix. You write a unit test for that and by mistake you insert a syntax error. Then you run the unit test and it fails as expected but not in the correct way. It's subtle. It fails as expected but it fails because of unexpected wrong causes. The solution: Check the errors carefully!

db48x 3y ago

Yes, but don't forget that there are different kinds of sandboxes. SELinux never needs the cooperation of any program running on the system in order to correctly sandbox things. No change to Xz could ever make SELinux less effective.

ynik 3y ago
But don't forget that xz is also used as part of dpkg for unpacking packages. The whole purpose of dpkg is to update critical system packages. Any SELinux policy that protects from a backdoored dpkg/xz installing a rootkit during the next kernel security update; will also prevent installing real kernel security updates. The particular way of attack in this OpenSSH backdoor can maybe be prevented; but we've got to realize that the attacker already had full root permissions and there's no way of protecting from that.
- db48x 3y ago
  SELinux policies are much more subtle than that. You don’t restrict what xz or liblzma can do, you restrict what the whole process can do. That process is either sshd or dpkg, and you can give them completely different access to the system, so that if dpkg tries to launch an interactive shell it fails, while sshd fails if it tries to overwrite a system file such as /bin/login or whatever. Neither would ordinarily do that, but the payload delivered via the back door might attempt it and wouldn’t succeed. And you would get a report stating what had happened, so if you’re paying attention the back door starts to become obvious. Also I think dpkg switched to Zstd, didn’t it? Or am I misremembering? But you’re not wrong; ultimately both sshd and dpkg are critical infrastructure. SELinux can prevent them from doing completely wrong things, but obviously it wouldn’t be useful for it to prevent them from doing their jobs. And those jobs are security critical already. SELinux is not a panacea, merely defense in depth.