Xz: o único caractere que desativou o Landlock no Linux

 (git.tukaani.org)
1 pontos por GN⁺ 2024-03-31 | 1 comentários | Compartilhar no WhatsApp
  • A mudança em CMakeLists.txt no xz.git, ao trocar a detecção do sandbox Landlock de uma verificação da presença do cabeçalho para um teste de compilação real, introduziu um caso em que um caractere . isolado apareceu dentro do código de teste
  • A nova verificação foi adicionada para filtrar sistemas que têm linux/landlock.h, mas podem não ter as definições de syscall necessárias
  • O teste de compilação inclui <linux/landlock.h>, <sys/syscall.h> e <sys/prctl.h>, e referencia prctl, SYS_landlock_create_ruleset, SYS_landlock_restrict_self e LANDLOCK_CREATE_RULESET_VERSION
  • O critério anterior, HAVE_LINUX_LANDLOCK_H, foi alterado para HAVE_LINUX_LANDLOCK, e as configurações de SANDBOX_COMPILE_DEFINITION e SANDBOX_FOUND também passam a seguir esse resultado
  • O commit corrige o teste de recurso do Linux Landlock nas builds com Autotools e CMake, e o diff fornecido mostra a mudança no lado do CMake

Mudança no método de detecção do Landlock no CMake

  • O arquivo afetado é o CMakeLists.txt do xz.git, e o ponto alterado é o bloco # Sandboxing: Landlock
  • A lógica anterior do CMake, quando ENABLE_SANDBOX era ON ou landlock e SANDBOX_FOUND não estava definido, verificava apenas a existência do cabeçalho com check_include_file(linux/landlock.h HAVE_LINUX_LANDLOCK_H)
  • Depois da mudança, check_c_source_compiles passou a compilar de fato um pequeno código C para verificar a disponibilidade do Landlock
    • Alguns sistemas podem ter linux/landlock.h e ainda assim não possuir as definições de syscall necessárias para o Linux Landlock
    • O código de teste inclui <linux/landlock.h>, <sys/syscall.h> e <sys/prctl.h>
    • Dentro de my_sandbox(), ele referencia prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0), SYS_landlock_create_ruleset, SYS_landlock_restrict_self e LANDLOCK_CREATE_RULESET_VERSION

Caractere isolado e mudança de nome de variável

  • Após o bloco de includes do código de teste de compilação inserido, há uma linha com um . isolado
  • A variável de resultado foi alterada de HAVE_LINUX_LANDLOCK_H para HAVE_LINUX_LANDLOCK
  • A condição também mudou de if(HAVE_LINUX_LANDLOCK_H) para if(HAVE_LINUX_LANDLOCK)
  • O valor de SANDBOX_COMPILE_DEFINITION usa "HAVE_LINUX_LANDLOCK" em vez de "HAVE_LINUX_LANDLOCK_H"
  • A configuração posterior de SANDBOX_FOUND ON foi mantida

Leituras relacionadas

1 comentários

 
GN⁺ 2024-03-31
Opiniões no Hacker News

  • Resposta: https://git.tukaani.org/?p=xz.git;a=commitdiff;h=f9cf4c05edd...
    Se você não conhece bem o sistema de controle de acesso Landlock do Linux, há uma explicação aqui: https://docs.kernel.org/userspace-api/landlock.html
    Página oficial (provavelmente...) de resposta ao incidente do xz: https://tukaani.org/xz-backdoor/

    • Então aquela função verificava se o código C a seguir compilava e, só nesse caso, ativava o Landlock?
      Mas por causa de um pequeno ponto final difícil de notar, colado perto da margem esquerda do diff, o código C ficava sempre inválido, e por isso o Landlock ficava sempre desativado?
      Impressionante de tão canalha, e canalha de tão impressionante. Eu também não vi na primeira leitura
    • Isso tem plausible deniability
      Se fosse para esconder, há maneiras melhores de trocar por caracteres Unicode parecidos. Alguns parecem idênticos até no nível de pixels, dependendo da fonte
      Posso ter perdido o fio da meada, mas a intenção deliberada foi comprovada aqui?
    • Isso é tão malicioso que, mesmo se fosse pego na hora durante o PR, daria para passar batido dizendo “ah, meu IDE autoformatou assim”
    • Lasse Collin reapareceu e provavelmente está irritado
    • Tenho a impressão de que meu sistema de controle de versão destaca melhor os caracteres alterados do que essas strings inteiras em verde/vermelho

  • Eu estava passando os olhos com cuidado e vi um ponto final em um lugar onde claramente não deveria estar, então pensei: “não é tão difícil quanto parecia”
    Toquei de leve na tela e o ponto se mexeu
    Maldita poeira no monitor

  • É inacreditável que a segurança do sistema dependa de uma cadeia de correção tão frouxa. Havia inúmeros pontos em que isso poderia ter sido barrado
    + # 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.
    O header desses sistemas deveria ser corrigido para excluí-los explicitamente. Qual é o sentido de um header se ele não declara sua própria funcionalidade?
    E também não entendo por que, depois que um blob binário é criado (mesmo que compilado a partir de código aberto), não há nenhum teste para verificar se a segurança está intacta
    Você provavelmente não colocaria no ar a função de pagamento de um site sem testes de ponta a ponta para a funcionalidade principal. Parece haver um desalinhamento de prioridades em que adicionar recursos fica acima da confiabilidade
    Espero que a correção não seja simplesmente remover o .. Acabei de ver outro post no HN mostrando a remoção do .

    • Ele introduziu um framework de testes completamente novo e, depois, ao longo de 2 anos, foi implantando a backdoor lentamente
    • O que você citou são palavras de Jia Tan [1]. É um comentário escrito pelo agente malicioso enquanto ele quebrava a verificação de propósito desde o início
      Corrigir o header ou adicionar testes não teria impedido isso. Para começo de conversa, não havia sinal de que o header estivesse quebrado, e testes adicionais também poderiam ter sido comprometidos de outra forma ou ignorados no tarball de release
      [1] https://git.tukaani.org/?p=xz.git;a=commit;h=328c52da8a2bbb8...
    • Meio tangencial, mas o GCC 9.4.0 chegou a distribuir um header arm_acle.h quebrado [https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100985] — código que incluísse esse header sempre falhava ao compilar
      Como o usuário poderia tentar compilar com o GCC 9.4.0, e a funcionalidade que dependia daquele header não era essencial para a aplicação, quando o build detectava que o header estava quebrado, simplesmente desativava aquele recurso e emitia um aviso
      Voltando ao xz, se segurança não for a prioridade máxima, ignorar a falha de um recurso opcional e seguir em frente também parece razoável. Claro que, em retrospecto, é fácil apontar o dedo, mas sem esse contexto não é uma decisão totalmente absurda
    • Como usuário de open source, não sei o suficiente para fazer uma sugestão dessas; então, se você desenvolver e contribuir diretamente, acho que seria bem-vindo
    • Você sabe se o código do comentário que citou está correto?

  • Nossa, o último commit dele está piorando o relato de segurança: https://git.tukaani.org/?p=xz.git;a=commitdiff;h=af071ef7702...

    • Por que isso foi aceito? Pergunta séria

  • Landlock deveria ser usado onde/como?
    Acho que, na prática, seria difícil usar em bibliotecas genéricas, como as de compressão/descompressão. A biblioteca não tem como saber o que o programa deve fazer nem o que deve restringir
    Em um programa, isso pode ser mais claro
    O ataque ao sshd usou a liblzma como biblioteca. Então desativar o Landlock parece irrelevante, não? Será que é sinal de que há mais código malicioso ainda não descoberto, ou de que planejavam inseri-lo depois?

    • Provavelmente o próprio sshd o usaria para bloquear suas próprias permissões depois de um certo ponto da execução
      Ao remover o Landlock, o daemon não se trava, então fica mais fácil executar o payload quando a exploração chega a essa etapa
      Não vejo as duas coisas como não relacionadas. Acho que eles já tinham o payload em mente e perceberam que isso seria um obstáculo
    • Estranho. Acho muito desleixado fazer dois trabalhos meio backdoor sem relação entre si no mesmo projeto. Parece que se ganha pouco e se aumenta muito o risco de ser descoberto
      Este caso é muito confuso. Algumas partes são sofisticadas a ponto de serem difíceis de acreditar, enquanto outras parecem bastante descuidadas
    • Pode ser usado e é uma boa ideia, mas o objetivo é impedir a exploração de vulnerabilidades, não um backdoor intencional
      A ideia é que a biblioteca inicie uma tarefa complexa em uma thread separada, e que o código do usuário aguarde essa thread dentro da API chamada. A thread aplica Landlock a si mesma e então começa o trabalho. Se algo der errado, o código fica isolado
      Claro que, neste caso, a sandbox é controlada pelo invasor, então isso não funciona. O invasor pode simplesmente desativá-la ou torná-la mais fraca do que o original. Ainda assim, dá para fazer de outras formas

  • O que exatamente eles estavam tentando fazer aqui? Inserir mais backdoors depois, backdoors com negação mais plausível? Na minha visão, nem o oss-fuzz nem essa alteração teriam realmente encontrado o backdoor descoberto
    Mas por que colocar todos os ovos de backdoor em uma cesta só, isto é, em uma única biblioteca?

    • Essa biblioteca estava enraizada o suficiente, era confiável o suficiente, e o desenvolvedor principal às vezes ficava muito tempo longe da internet por problemas de saúde mental
      Além disso, não se trata de colocar um backdoor na própria biblioteca, mas de mirar as ferramentas que a usam. É no estilo “Reflections on trusting trust”
      Até falhar, soa como um plano perfeito
    • Mas quem pode dizer que era apenas uma biblioteca?

  • Fico curioso sobre por que seria útil impedir que o Landlock fosse ativado no xz. Será que pretendiam injetar conteúdo malicioso em arquivos xz numa etapa posterior? Se fosse isso, por que não simplesmente colocar comportamento malicioso no próprio xz?

    • Porque uma vulnerabilidade intencional é muito mais fácil de esconder do que conteúdo malicioso real
      Talvez seja possível inserir às escondidas um buffer overflow ou uso após liberação em um projeto C mantido por você sem ser pego. Distribuir um cavalo de Troia de verdade é muito mais difícil, e isso ficou claro no backdoor xz-para-sshd
    • Além do alvo em ssh, pode haver um backdoor mais sutil mirando o próprio xz. Claramente o objetivo era a discrição

  • Isso é surpreendentemente chamativo. A técnica de desativar o recurso é inteligente e o commit é bem plausível. Mas por que escolher um erro de sintaxe óbvio em vez de simplesmente errar a grafia? Por exemplo, será que teríamos percebido se o erro fosse PR_SET_NO_NEW_PRIV?
    Isso também teria mais plausibilidade de negação

  • Separadamente, essa equipe de malware criou um ótimo dataset para treinar IA a identificar problemas de segurança. Como todo commit tem um problema de segurança, e a comunidade open source vai examiná-los um a um até encontrá-los
    Agradeço aos mantenedores que estão fazendo a limpeza. Certamente não é uma tarefa divertida

    • Não acho que isso vá generalizar bem. No máximo, é só uma corrida armamentista
    • É um uso de IA que achei bem interessante
    • Nunca ouvi falar disso; você poderia postar algumas informações relacionadas?

  • Por isso eu realmente não gosto de sistemas de build no estilo configure que ativam e desativam recursos automaticamente. Se quero algo, prefiro ativá-lo explicitamente. Se houver um bom motivo para deixar um recurso como padrão, então deveria ser possível desativá-lo explicitamente em vez disso

    • Ao empacotar, isso é uma dor enorme. Você configura o pacote, adiciona dependências até ele compilar e acha que terminou. Mas o recurso X está faltando. O quê? Ah, faltou a libY, então ele foi desativado silenciosamente. Você volta e adiciona. Aí um usuário relata que o recurso Z não existe
      Bastaria ter um conjunto de recursos padrão e permitir --enable-a --disable-b conforme necessário
      Engolir silenciosamente bugs no processo de verificação é outro problema