Xz: un solo carácter que anuló Landlock en Linux

 (git.tukaani.org)
1 puntos por GN⁺ 2024-03-31 | 1 comentarios | Compartir por WhatsApp
  • El cambio en CMakeLists.txt de xz.git, al pasar la detección del sandbox Landlock de comprobar la existencia del header a una prueba de compilación real, incluyó un caso donde apareció un carácter . aislado dentro del código de prueba
  • La nueva verificación se agregó para filtrar sistemas que, aunque tengan linux/landlock.h, podrían no contar con las definiciones de syscall necesarias
  • La prueba de compilación incluye <linux/landlock.h>, <sys/syscall.h>, <sys/prctl.h> y hace referencia a prctl, SYS_landlock_create_ruleset, SYS_landlock_restrict_self, LANDLOCK_CREATE_RULESET_VERSION
  • El criterio anterior, HAVE_LINUX_LANDLOCK_H, cambia a HAVE_LINUX_LANDLOCK, y la configuración de SANDBOX_COMPILE_DEFINITION y SANDBOX_FOUND también sigue ese resultado
  • El commit corrige la prueba de funcionalidad de Linux Landlock en los builds con Autotools y CMake, y el diff proporcionado muestra los cambios del lado de CMake

Cambio en el método de detección de Landlock en CMake

  • El archivo objetivo es CMakeLists.txt de xz.git, y el punto modificado es el bloque # Sandboxing: Landlock
  • La lógica anterior de CMake, cuando ENABLE_SANDBOX era ON o landlock y SANDBOX_FOUND era falso, solo comprobaba la existencia del header con check_include_file(linux/landlock.h HAVE_LINUX_LANDLOCK_H)
  • Después del cambio, se usa check_c_source_compiles para compilar realmente un pequeño código en C y verificar si Landlock puede utilizarse
    • Algunos sistemas pueden tener linux/landlock.h pero no las definiciones de syscall necesarias para Linux Landlock
    • El código de prueba incluye <linux/landlock.h>, <sys/syscall.h>, <sys/prctl.h>
    • Dentro de my_sandbox() se hace referencia a prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0), SYS_landlock_create_ruleset, SYS_landlock_restrict_self, LANDLOCK_CREATE_RULESET_VERSION

Carácter aislado y cambio de nombre de variable

  • Después del bloque de includes del código de prueba de compilación insertado, hay una línea con un . aislado
  • La variable de resultado cambia de HAVE_LINUX_LANDLOCK_H a HAVE_LINUX_LANDLOCK
  • La condición también cambia de if(HAVE_LINUX_LANDLOCK_H) a if(HAVE_LINUX_LANDLOCK)
  • El valor de SANDBOX_COMPILE_DEFINITION usa "HAVE_LINUX_LANDLOCK" en lugar de "HAVE_LINUX_LANDLOCK_H"
  • Después de eso, se mantiene la configuración de SANDBOX_FOUND ON

Lecturas relacionadas

1 comentarios

 
GN⁺ 2024-03-31
Comentarios en Hacker News

  • Respuesta: https://git.tukaani.org/?p=xz.git;a=commitdiff;h=f9cf4c05edd...
    Si no conoces bien el sistema de control de acceso Landlock de Linux, aquí está la explicación: https://docs.kernel.org/userspace-api/landlock.html
    Página oficial (supongo...) de respuesta al incidente de xz: https://tukaani.org/xz-backdoor/

    • O sea, ¿esa función activaba Landlock solo si comprobaba que el código C que seguía compilaba?
      ¿Y entonces, por un solo punto minúsculo pegado cerca del borde izquierdo del diff, difícil de notar, el código C siempre quedaba inválido y por eso Landlock siempre terminaba desactivado?
      Impresionante de lo mezquino, y mezquino de lo impresionante. Yo tampoco lo vi la primera vez que lo leí
    • Esto tiene plausible negación
      Si quisieras ocultarlo, también hay formas mejores, como cambiar caracteres Unicode parecidos. Algunos hasta se ven idénticos píxel por píxel según la fuente
      Puede que me haya perdido algo del contexto, pero ¿ya quedó confirmada la intencionalidad aquí?
    • Esto es tan retorcido que, incluso si te atraparan en pleno PR, podrías zafarte diciendo “ah, mi IDE lo autoformateó así”
    • Lasse Collin volvió a aparecer y parece que está enojado
    • Siento que mi sistema de control de versiones resalta mejor los caracteres modificados que estas enormes franjas verde/rojo

  • Lo estaba revisando con cuidado y vi un punto justo donde claramente no debía haber uno, así que pensé “no está tan difícil como parecía”
    Toqué un poco la pantalla y el punto se movió
    Maldita polvo en el monitor

  • No puedo creer que la seguridad del sistema dependa de una cadena de exactitud tan frágil. Había muchísimos puntos donde esto se podía haber detenido
    + # A compile check is done here because some systems have
    + # linux/landlock.h, but do not have the syscalls defined
    + # in order to actually use Linux Landlock.
    Habría que corregir los headers de esos sistemas para excluirlos explícitamente. Si un header no declara sus propias capacidades, ¿entonces para qué sirve?
    Y tampoco entiendo por qué no hay ni una sola prueba que verifique que la seguridad siga intacta después de que se genera el blob binario (aunque se haya compilado desde open source)
    Ni siquiera desplegarías un sistema de pagos en un sitio web sin pruebas end-to-end de las funciones críticas. Parece haber un desajuste de prioridades donde agregar funciones pesa más que la robustez
    Espero que la corrección no sea simplemente quitar el .. Acabo de ver otra publicación en HN que mostraba justamente eliminar el .

    • Introdujo un framework de pruebas completamente nuevo y luego fue sembrando lentamente el backdoor durante 2 años
    • Lo citado son palabras de Jia Tan [1]. Ese comentario es el que escribió el actor malicioso mientras rompía deliberadamente la verificación desde el principio
      Corregir headers o agregar más pruebas no lo habría detenido. Para empezar, no había ninguna señal de que el header estuviera roto, y además las pruebas adicionales también podrían haberse manipulado por otros medios o ignorado en el tarball de lanzamiento
      [1] https://git.tukaani.org/?p=xz.git;a=commit;h=328c52da8a2bbb8...
    • Un poco al margen, pero GCC 9.4.0 llegó a distribuir un header arm_acle.h roto [https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100985] — cualquier código que lo incluyera siempre fallaba al compilar
      Como un usuario podía intentar compilar con GCC 9.4.0, y la funcionalidad que dependía de ese header no era crítica para la aplicación, el build simplemente desactivaba esa función y mostraba una advertencia si detectaba que el header estaba roto
      Volviendo a xz, si la seguridad no fuera la máxima prioridad, parece razonable seguir adelante ignorando el fallo de una función opcional. Claro, después de los hechos es fácil señalar con el dedo, pero fuera de ese contexto no es una decisión completamente absurda
    • Como usuario de open source, no sé lo suficiente como para hacer este tipo de propuestas; agradecería que quien pueda lo desarrolle y lo aporte directamente
    • ¿Tenemos claro que el comentario de código citado es correcto?

  • Vaya, su último commit también está empeorando los reportes de seguridad: https://git.tukaani.org/?p=xz.git;a=commitdiff;h=af071ef7702...

    • ¿Por qué aceptaron esto? De verdad me da curiosidad

  • ¿Landlock dónde/cómo se suponía que iba a usarse?
    Parece que en una biblioteca de propósito general para compresión/descompresión sería difícil usarlo en la práctica. La biblioteca no puede saber qué debe hacer el programa ni qué debería restringir.
    En un programa eso podría ser más claro.
    El ataque a sshd usaba liblzma como biblioteca. Entonces, apagar Landlock parece no tener relación, ¿no? ¿Es una señal de que había más código malo aún no descubierto, o de que planeaban meterlo después?

    • Probablemente el propio sshd lo habría usado para bloquear sus propios privilegios después de cierto punto de la ejecución.
      Si se elimina Landlock, el daemon no se autoimpone restricciones, así que cuando se llegue a la etapa de explotación será más fácil ejecutar el payload.
      No creo que ambas cosas sean independientes. Parece que ya tenían en mente el payload y se dieron cuenta de que esto sería un obstáculo.
    • Qué raro. Hacer dos maniobras tipo backdoor no relacionadas entre sí dentro del mismo proyecto me parece realmente torpe.
      Se gana poco y solo se aumenta mucho el riesgo de que te descubran.
      Este incidente es muy confuso. Algunas partes son increíblemente sofisticadas, mientras que otras se ven bastante descuidadas.
    • Se puede usar y es una buena idea, pero sirve más para evitar la explotación de vulnerabilidades que para un backdoor intencional.
      La idea es que la biblioteca inicie una tarea compleja en un hilo separado, y que el código del usuario espere ese hilo dentro de la API que llamó. El hilo se aplica Landlock a sí mismo y luego empieza el trabajo. Si algo sale mal, el código queda aislado.
      Claro, en este caso no funcionaría porque el sandbox estaría bajo control del atacante. El atacante simplemente podría apagarlo o hacerlo más débil de lo normal. Aun así, podría hacerse de otras maneras.

  • ¿Qué buscaban exactamente aquí? ¿Meter más backdoors después, o backdoors con una negación más plausible? Según yo, ni oss-fuzz ni este cambio habrían encontrado realmente el backdoor descubierto.
    Entonces, ¿por qué poner todos los huevos del backdoor en una sola canasta, o sea, en una sola biblioteca?

    • Esa biblioteca estaba lo bastante metida en el ecosistema, gozaba de suficiente confianza y su desarrollador principal había pasado largos periodos fuera de internet por problemas de salud mental.
      Además, no se trataba de backdoorear la biblioteca en sí, sino de apuntar a las herramientas que la usan. Un estilo “Reflections on trusting trust”.
      Hasta que falló, sonaba como un plan perfecto.
    • Pero ¿quién puede decir que era solo una biblioteca?

  • Me intriga por qué era útil impedir que se activara Landlock en xz. ¿Pensaban inyectar contenido malicioso en un archivo xz en una etapa posterior? Si era así, ¿por qué no meter simplemente el comportamiento malicioso directamente en xz?

    • Una vulnerabilidad intencional es mucho más fácil de ocultar que el contenido malicioso real.
      Tal vez podrías meter a escondidas un desbordamiento de búfer o un use-after-free en un proyecto en C que mantienes sin que te descubran. Distribuir un verdadero troyano es mucho más difícil, y eso quedó claro con el backdoor xz-to-sshd.
    • Además de apuntar a ssh, podría haber habido un backdoor más sutil enfocado en xz mismo. Claramente el objetivo era la discreción.

  • Esto llama demasiado la atención de una manera inesperada. La técnica para desactivar la funcionalidad es ingeniosa y el commit se ve bastante plausible. Pero ¿por qué elegir un error de sintaxis evidente en vez de simplemente equivocarse al escribirlo? Por ejemplo, si el error hubiera sido PR_SET_NO_NEW_PRIV, ¿alguien lo habría notado?
    Eso también habría dado una negación más plausible.

  • Por otro lado, este equipo de malware le dejó un gran dataset a quienes entrenan IA para identificar problemas de seguridad. Todos los commits tienen problemas de seguridad, y la comunidad open source los va revisando y encontrando uno por uno.
    Gracias a los mantenedores que están limpiando todo esto. Seguro no es un trabajo divertido.

    • No parece algo que vaya a generalizar bien. A lo mucho sería una carrera armamentista.
    • De los usos de IA que he visto, este está bastante bueno.
    • Nunca había oído hablar de eso; ¿podrías compartir algo de información al respecto?

  • Por eso realmente no me gustan los sistemas de build estilo configure que activan y desactivan cosas automáticamente. Si quiero algo, quiero activarlo explícitamente. Si hay una razón suficiente para que una función venga por defecto, entonces al menos debería poder desactivarse explícitamente.

    • Al empaquetar, esto es un dolor enorme. Configuras el paquete, agregas dependencias hasta que por fin compila y crees que ya quedó. Pero entonces falta la función X. ¿Qué pasó? Ah, no estaba libY y por eso se desactivó silenciosamente. Vuelves atrás y la agregas. Luego un usuario reporta que falta la función Z.
      Simplemente deberían tener un conjunto base de funciones y permitir --enable-a --disable-b según se necesite.
      Que además se traguen silenciosamente los bugs del proceso de detección es otro problema.