A validação de email+senha deve tomar o mesmo tempo quando o usuário existe ou não

[tcarreira]

De forma a deixar a rota /api/v1/sessions uma black box (ou seja, não ser possível saber se um usuário existe ou não através do tempo de resposta do servidor), precisamos fazer algumas alterações.

Sugestão de fluxo de validação de login, em pseudo-código

startTime = getMonotonicTime()
targetElapsed = random(2.5, 3.5) // uma duração de tempo aleatória entre 2.5s e 3.5s

userFromDB = getUserByUsername(requestedUser)
if (userFromDB) {
  isMatch = checkPasswordMatch(userFromDB, requestedPassword)
}

await waitForElapsed(startTime, targetElapsed) // espera até terem passado pelo menos entre 2.5s e 3.5s desde o startTime

if isMatch {
  return true
}
return false

Vantagens:

• a rota é consistentemente lenta, previne timing attacks
• o hash da senha só é calculado quando o usuário existe (fazer o hash é caro)
• aquele waitForElapsed() pode ser implementado com um sleep, o que reduz o CPU usado (fica mais barato quando o usuário não existe)

[filipedeschamps]

@coffee-is-power por que essa issue foi fechada?

[coffeeispower]

A feature ja foi implementada

[filipedeschamps]

Onde exatamente?

[coffeeispower]

#171
E tambem essa issue ja esta parada a 2 meses, se o criador da issue quiser voltar e so pedir para abrir

[filipedeschamps]

@coffee-is-power não entendi. Onde no código (ou até naquela issue), demonstram que "A validação de usuário+senha deve tomar o mesmo tempo quando o usuário existe ou não" foi implementado?

[aprendendofelipe]

Edit: Como o login agora utiliza o email, e não mais o username, o que eu argumentei abaixo não se aplica. E a implementação sugerida se justifica sim, na verdade, ainda mais do que antes.

Meus 10 centavos para esse assunto é que não existe justificativa para essa implementação enquanto houverem outras formas simples de obter os usuários válidos, como nesse endpoint:

https://www.tabnews.com.br/api/v1/users

E mesmo se eliminasse a rota acima, bastaria o atacante ir testando esse outro endpoint antes de tentar descobrir a senha:

https://www.tabnews.com.br/[username]

[filipedeschamps]

@aprendendofelipe alguns pontos importantes nessa questão:

1. A lista de usuários não faz diferença como vetor de ataque nessa situação, pois a nossa autenticação não é mais feita usando o username como antigamente, ela é feita apenas usando o email. Como você falou, o atacante pode fazer o scrape do username de outras formas, por isso foi importante alterar a autenticação para apenas email, uma vez que isso é um dado privado e não é exposto em nenhum lugar do TabNews. Isso foi uma sugestão do @tcarreira feita na mesma bateria de sugestões dessa issue daqui.
2. Então esta implementação não tenta evitar a listagem dos usuários, mas tenta evitar que o atacante saiba da existência de um email na base, pois se o email não existe, a resposta vai ser muito mais rápida. Isso porque do jeito que foi implementado, o cálculo do hash da senha, que é uma etapa devagar, só acontece se o email foi encontrado.
3. Então ele sabendo da existência ou não de um email, o atacante pode escolher se focar em atacar a senha. Por exemplo, o meu email público aqui no GitHub é filiped@gmail.com e o atacante pode hoje tentar descobrir se eu usei esse email ou não no TabNews. Se responder muito rápido na tentativa de criar a sessão, ele sabe que eu não usei e nem adianta tentar atacar a senha, mas se responder devagar, bingo.
4. Isso também pode ser usado quando há vazamento de email+senha de outros lugares. Isso acontecia muito no Pagar.me, onde um e-commerce qualquer tinha um vazamento da base de usuários, o pessoal quebrava o hash e começava a testar essa combinação contra o nosso endpoint de autenticação para ver se por sorte algum usuário tinha usado o mesmo email e senha. Na época que eu trabalhava lá implementamos um rate limiting que identificava esse comportamento e continuava retornando uma resposta de erro, mesmo que o atacante tinha acertado a combinação. Enfim, na situação atual do TabNews, dá para identificar que pelo menos o mesmo email foi utilizado num caso de vazamento de lista de emails.
5. De qualquer forma, o endpoint /user talvez deveria ser removido por enquanto até que a gente tenha um caso concreto que necessite dele para algum client, e principalmente quando ele tiver paginação. Então @aprendendofelipe sugiro não aplicar o PR fix(api-users): users endpoint expect read:user:list feature #494 e fazer um outro que remova (desliste) esse endpoint. E quando o endpoint voltar, ele poderia ser listado de forma pública sem problemas, o GitHub faz isso também e no caso deles as pessoas já construíram coisas legais em cima 👍
6. Mas no final das contas, precisamos implementar rate limiting, e isso já está listado como tarefa nessa Milestone 🤝

[aprendendofelipe]

Corretíssimo @filipedeschamps! Eu vi o getUserByUsername no pseudo código (que o @tcarreira deve ter escrito antes da alteração para email) e nem lembrei que não usamos o username no login 😅

Sobre retirar o endpoint users, você que manda! 👍

Mas pensa só um pouco antes, pois pode ser útil manter o acesso somente para você (desde que adicione a feature read:user:list ao seu usuário).

Da maneira que ficou no #494, ninguém mais terá acesso, já que nenhum usuário tem essa feature.

E também já está com os testes que vão impedir que a rota seja exposta novamente.

[filipedeschamps]

Da maneira que ficou no #494, ninguém mais terá acesso, já que nenhum usuário tem essa feature.

Puts, é verdade! Sensacional!!!! Acabei de editar o draft da Milestone 5 com esse PR 🤝 🤝 🤝

[ThallesP]

O e-mail também é dito que existe na hora do cadastro:

Para resolver isso, logo após enviar o formulário de cadastro, redirecionar o usuário para a confirmação de e-mail independente se já existir um e-mail cadastrado, mas com a seguinte mensagem "Iremos enviar um e-mail de validação caso ainda não esteja cadastrado".
Assim, no back-end, iremos enviar a confirmação apenas se o e-mail não existir no banco de dados.

[filipedeschamps]

@ThallesP show! Uma dúvida que tenho sobre esse ponto de evitar a conferência do email é que muita gente fala para fazer isso na hora do cadastro, mas ainda fica um furo que é na hora da atualização por email, pois esse é outro vetor. Qual a melhor UX nesse caso?

Por exemplo:

• Usuário A possui o email a@a.com
• Usuário B possui o email b@b.com
• Usuário B faz requisição para atualizar seu email para a@a.com.
• O que deveria acontecer?

[ThallesP]

@filipedeschamps
Pensei em duas opções:

• Usar quase mesmo esquema que eu disse para o cadastro, e ao atualizar o e-mail, pedir a verificação para esse novo e-mail e também dar a opção do usuário fornecer outro e-mail dentro dessa nova verificação (caso ele tenha errado na hora de atualizar). (minha opção favorita)
• Dizer que o e-mail existe, mas colocar um rate-limit para alterar (1 dia ou 3 dias?) raramente mudamos de e-mail então acredito que não teria problema.

Problema que eu vejo com a primeira opção é caso um usuário mal intencionado tente fazer spam atualizando para e-mails aleatórios, talvez precise de um rate-limit para prevenir isso.

[rodrigoKulb]

Tentando organizar as Issues, acredito que avisar que o e-mail já está cadastrado é uma funcionalidade muito importante. o github faz assim hoje.

Sobre o tempo de login com usuário válido e não valido, já foi implementado?

[aprendendofelipe]

Tentando organizar as Issues, acredito que avisar que o e-mail já está cadastrado é uma funcionalidade muito importante. o github faz assim hoje.

Sobre o tempo de login com usuário válido e não valido, já foi implementado?

Ainda não foi implementado. E são duas medidas contraditórias, pois o login é com email, então se for mostrar que o email já existe, não é necessária a implementação dessa issue.

Mas veja que, por estas mensagens do @filipedeschamps, a intenção da issue é justamente esconder a informação se o email já estiver cadastrado:

2. Então esta implementação não tenta evitar a listagem dos usuários, mas tenta evitar que o atacante saiba da existência de um email na base...

3. Então ele sabendo da existência ou não de um email, o atacante pode escolher se focar em atacar a senha...

4. Isso também pode ser usado quando há vazamento de email+senha...

Mas tem que ver se é possível fechar todos os furos, pois, se não for possível, talvez seja melhor agir no sentido de impedir a descoberta da senha por força bruta. Já existe o rate-limit, mas como é por IP, não pode ser muito restrito, então deveria haver um limite por usuário.

[rodrigoKulb]

@aprendendofelipe obrigado pelas explicações:

• Realmente a funcionalidade de informar que o e-mail já está em uso é essencial.

Desta forma, realmente não faz sentido quebrar a cabeça com o time de login para verificar se o e-mail existe.
Então não vejo necessidade de manter essa Issue aberta, tendo que o título dela é para alterar o tempo quando o usuário existe ou não.

Estou nessa Issue porque é a mais antiga aberta do projeto, e vou fazer uma "limpeza" da mais antiga para frente.

👍️ - Finalizar essa Issue.
👎️ - Seguir com ela aberta.

[luanmz]

Fala pessoal, tudo beleza?

Confesso que não entendi o porque dessa issue ainda não ter sido fechada, testei para mail enumeration na rota /api/v1/sessions e não notei diferença clara ou substancial nos tempos de resposta que justifique essa rota específica estar vulnerável a enumeração mencionada.

Porém, a historia é diferente para a rota /api/v1/users chamada pela funcionalidade de cadastrar um usuário na base de dados, isso se dá pelo fato de o serviço de email ser chamado apenas quando o email não esta presente na base, permitindo a enumeração via tempo de resposta onde emails existentes respondem a requisição de forma muito mais rápida.

Testei esse comportamento localmente e o validei no próprio site oficial do tabnews (onde a diferença chega a casa 2000ms).

Nos meus testes de possíveis soluções ainda encontrei uma outra enumeração (um pouco mais especifica) na mesma rota, relacionada a com o tratamento de erro quando um usuário tenta se cadastrar com um usuário já existente e um email também já existente (resultado de uma possivel enumeração). O erro de "usuário já existe" é disparado somente se o primeiro match encontrado na base de dados para userData for um usuário igual, se for um email, ele não dispara o erro e mostra a mensagem que o email foi enviado. Permitindo enumerar todos os emails de usuários que estão acima dele na base de dados. (já possuo uma correção satisfatória para esse comportamento)

No intuito de igualar esses tempos de resposta devo atacar essa issue e tentar corrigir essas enumerações. Não sei se deveria abrir uma outra issue específica dessa rota mas por enquanto vou utilizar esta.

[aprendendofelipe]

@luanmz, você foi citado na publicação sobre as novidades do TabNews 🎉💪

https://www.tabnews.com.br/FelipeBarso/tabnews-melhorias-de-seguranca-nos-testes-e-mais