O primeiro CVE a gente não esquece

Hoje quero contar a você uma história que mexeu bastante comigo: o primeiro problema de segurança sério que alguém reportou no Keyrunes, o servidor de autenticação e autorização que eu venho desenvolvendo em Rust.

Confesso que assusta um pouco receber um report desses sobre o próprio código. Mas, com o tempo, eu percebi que é exatamente o tipo de coisa que faz a gente melhorar. E esse foi um caso bem didático, então vou explicar com calma. Além de tudo esse é um ponto que sempre cobrava nos locais que trabalhei tanto pensar em segurança quanto ter um código aberto.

Quem achou

O crédito é todo do Rathnadeepan, que aparece no GitHub como HotWater (@HotWater-015). Ele encontrou duas falhas relacionadas, obrigado demais pelo report, pelo cuidado de descrever o caminho de ataque e pela paciência. Isso fez diferença pra eu conseguir corrigir rápido. Até por que meu projeto ainda é bem pequeno, mas acho importante e por isso prego a tanto tempo em trabalhar com projetos open-source, pois sim pode ter pessoas mal intencionadas mas, também há muitas pessoas que querem ajudar um ecossistema a crescer.

O que estava errado

A raiz dos dois problemas é a mesma: eu confiava em dados que o cliente controla, e confiava em nomes como se fossem identidade.

A organização vinha do corpo da requisição

No cadastro (UserService::register), o campo organization_id era aceito do jeito que chegava. Ou seja, dava pra registrar um usuário no namespace empresa x dizendo que ele pertencia à organização de outro tenant. A partir daí, os endpoints tenant scoped passavam a operar sobre a organização da vítima, inclusive lendo e rotacionando a secret_key dela. Em outras palavras um atacante poderia criar uma conta registrar o seu namespace e a partir dele acessar o namespace de outra empresa.

Tinha ainda um segundo buraco. Em get_org_key e rotate_org_key, eu verificava se o usuário era admin, mas não verificava se a organização realmente pertencia ao namespace dele. Ou seja, o organization_id do próprio usuário era tratado como verdade absoluta.

Acredito que por ser a primeira vez que trabalho com multi tenant no banco, isso me fez ignorar que o banco busca as informações por tenant e quem comanda a regra é o próprio namespace assim com a mesma consulta diferentes empresas conseguem usar o sistema. Isso é comum em sistemas SAAS, no meu caso o Keyrunes não é um SAAS mas, ele pode trabalhar com vários clientes diferentes.

Vale a dica pros amigos que trabalham nesse tipo de arquitetura desse risco.

O nome superadmin valia demais

Essa aqui é mais traiçoeira. Eu verificava privilégio pelo nome do grupo: se o token tinha superadmin na lista de grupos, a pessoa era superadmin. Sim, errei feio, errei rude, acho que é um erro meio básico confiar apenas na permissão, sempre o ideal é verificar a politica e através dela dar acesso, mas ai deixei passar essa por que a principio só o domínio principal ia ter um super admin.

Só que cada schema de tenant tem um grupo chamado superadmin, porque a migration de sincronização cria um em todo lugar, fiz isso por que em algum momento tava mais fácil administrar sempre tendo esse grupo em todos os tenant. E o admin de um tenant conseguia criar grupos. Então, na prática, ele criava um grupo superadmin no próprio tenant, se atribuía a ele e pronto: o código de autorização enxergava um superadmin.

Com isso, ele chegava ao dashboard administrativo, ao check-permission e até ao bypass da chave de organização, coisas que deveriam ser exclusivas do superadmin global.

A lição aqui é simples e eu vou repetir sempre: nome não é permissão, a politica sempre tem que ser o cidadão de primeira classe no seu sistema.

Como corrigi

As correções saíram na versão 0.4.2. Resumo das mudanças, na ordem em que eu fiz.

1. Um único lugar pra decidir quem é superadmin global

Em vez de espalhar groups.contains("superadmin") pelo código, criei um método no AuthenticatedUser:

impl AuthenticatedUser {
    /// Whether this caller is a **global** superadmin.
    pub fn is_global_superadmin(&self) -> bool {
        self.namespace == crate::constants::DEFAULT_NAMESPACE
            && self.groups.iter().any(|g| g == SUPERADMIN_GROUP)
    }
}

Agora, toda decisão que depende de "posso agir fora do meu tenant" passa por esse método. Ele exige os dois: estar no namespace public e ter o grupo. Um admin de tenant nunca passa daqui, mesmo que tenha um grupo com esse nome.

2. Reservar o nome superadmin ao namespace global

Nos pontos de criação e atribuição de grupo, o nome fica bloqueado fora do public:

if namespace != DEFAULT_NAMESPACE
    && req.name.trim().eq_ignore_ascii_case(SUPERADMIN_GROUP)
{
    return Err(anyhow!(
        "group name '{}' is reserved to the global namespace",
        SUPERADMIN_GROUP
    ));
}

O mesmo vale para assign_user_to_group e para resolve_group_ids, que é quem resolve os grupos no cadastro. Repare no trim() e no eq_ignore_ascii_case: sem eles, um nome como " SuperAdmin " passaria batido. Vale ressaltar que no mundo ideal eu já teria criado um sistema de policies e permissions mas, em todo o caso deixando esse grupo só pro public já resolve por hora esse problema.

3. A organização é função do namespace

No register, se o cliente mandar um organization_id, ele precisa bater com a organização que de fato pertence àquele namespace:

if let Some(requested_id) = req.organization_id
    && let Some(org) = self.org_repo.find_by_namespace(namespace).await?
    && org.organization_id != requested_id
{
    return Err(anyhow!(
        "organization {} does not belong to namespace '{}'",
        requested_id,
        namespace
    ));
}

O cliente pode repetir o valor, mas não pode escolher outro.

4. Conferir o dono antes de revelar ou rotacionar a chave

Em get_org_key e rotate_org_key, a organização só é tocada depois de confirmar que ela pertence ao namespace do usuário, coisa básica, nunca mude o dado se não tem certeza que o requisitante pertence ao tenant que vai sofrer o input do dado, pra isso sempre precisa validar os dados e token:

Ok(Some(org)) if org.namespace != user.namespace => {
    return ErrorResponse::forbidden("Organization does not belong to your namespace")
        .into_response();
}

Como isso ta no nível do backend, mesmo que o atacante mude o payload da request estamos nos assegurando pelo próprio banco, mesmo que ele saiba o user admin de outro tenant ele precisa ter um token JWT válido ai o ataque não fica tão simples.

Também movi a checagem de papel para depois de descobrir qual é a organização do usuário, para que a decisão seja tomada sempre com o dado real na mão.

5. Listagem de organizações

A regra ficou explícita: só o superadmin global lista todas as organizações. Quem não é superadmin global recebe apenas as organizações do próprio namespace, filtradas no servidor. Antes, a lógica dependia do nome do grupo e de dois ifs encadeados, o que era fácil de errar.

Como testar que ficou corrigido

Os testes de tests/organization_authz_test.rs cobrem exatamente essa separação entre admin de tenant e superadmin global, e tem casos de cadastro com organization_id de outro namespace. Eu também alinhei o mock do repositório de organização com a convenção de tenant, porque ele estava contradizendo a checagem nova e fazendo teste passar pelo motivo errado.

Aqui é sempre importante ressaltar sei que o projeto precisava ter mais testes, mas uma coisa importante é que se você não projeta bem os testes isso acaba tendo bypass. Eu achava que estava correto mas, o próprio mock estava errado.

Se você for testar o seu próprio sistema, minha sugestão é escrever o teste do ataque primeiro, vê-lo falhar e só depois corrigir. Foi assim que eu fiz aqui, e dá muito mais confiança de que o buraco realmente fechou. E sim, é difícil pensar naturalmente quais as formas de atacar, mas acho que vale o treino e isso se você não tem em mente acredito que nem uma IA generativa salva.

O que eu levo daqui

  • Todos os inputs são evil. Ouvi isso do grande Henrique Bastos e tem que ser um mantra pra todo o desenvolvedor web. Se o namespace é conhecido pelo servidor, a organização deve ser derivada dele. Nunca cofie em nada que entra.
  • Nome de papel não é identidade. Quando uma permissão depende de algo "global", ela precisa de uma verificação explícita, não de uma string.
  • Verifique o dono do recurso antes de devolver o dado, não só o papel de quem pede. Como disse antes NUNCA confie só no payload da request.
  • Revisão de segurança antes do release. Ter alguém de fora olhando o código pega coisas que a gente, de tanto ver, deixa passar. Esse é um ponto que infelizmente eu sofro por ser um projeto solo, muitas vezes uso a IA pra me ajudar nessa etapa como a "visão de fora", mas nesse caso precisa obrigatoriamente já pedir certas diretrizes de segurança chumbadas nas regras dele, ele por si só não vai ver isso.

Foi o primeiro CVE, e é um daqueles que a gente não esquece. Obrigado de novo ao Rathnadeepan.

Até a próxima, beijos de luz!

Comentários