tabnews.com.br
tabnews.com.br copied to clipboard
A validação de email+senha deve tomar o mesmo tempo quando o usuário existe ou não
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)
@coffee-is-power por que essa issue foi fechada?
A feature ja foi implementada
Onde exatamente?
#171 E tambem essa issue ja esta parada a 2 meses, se o criador da issue quiser voltar e so pedir para abrir
@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?
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]
@aprendendofelipe alguns pontos importantes nessa questão:
- 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 oemail
. Como você falou, o atacante pode fazer o scrape dousername
de outras formas, por isso foi importante alterar a autenticação para apenasemail
, 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. - 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 oemail
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 oemail
foi encontrado. - 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 é[email protected]
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. - 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. - 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 #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 👍 - Mas no final das contas, precisamos implementar rate limiting, e isso já está listado como tarefa nessa Milestone 🤝
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.
Da maneira que ficou no https://github.com/filipedeschamps/tabnews.com.br/pull/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 🤝 🤝 🤝
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.
@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[email protected]
- Usuário
B
possui o email[email protected]
- Usuário
B
faz requisição para atualizar seu email para[email protected]
. - O que deveria acontecer?
@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.
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?
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
3. Então ele sabendo da existência ou não de um
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.
@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.