Код-ревью Middle Логирование, реагирование и ПДн обновлено

Ревью: логирование входа на Java

Вопрос

Найдите проблемы в логировании.

public boolean login(String username, String password, HttpServletRequest req) {
    log.info("Login attempt: user=" + username + " password=" + password);
    User u = users.find(username);
    if (u == null || !encoder.matches(password, u.getHash())) {
        log.debug("failed");
        return false;
    }
    log.info("Session " + req.getSession().getId() + " created for " + username
        + " card=" + u.getCardNumber() + " ua=" + req.getHeader("User-Agent"));
    return true;
}

Коротко

Логировать нужно факт и контекст события (кто, откуда, результат), но никогда секреты. Структурированный логгер с полями вместо конкатенации решает и инъекцию, и маскирование.

Что должен найти ревьюер

  1. Безопасностьстрока 2
    В лог пишется пароль, а ввод пользователя без экранирования: возможна инъекция в лог через перевод строки.
  2. Безопасностьстрока 5
    Неудачный вход пишется на уровне debug без пользователя, IP и времени: перебор паролей не будет виден на проде.
  3. Безопасностьстрока 8
    Идентификатор сессии в логе позволяет угнать сессию любому с доступом к логам.
  4. Безопасностьстрока 9
    Полный номер карты в логе нарушает PCI DSS и требования к ПДн.
  5. Безопасностьстрока 9
    User-Agent от клиента без экранирования: инъекция в лог.
  6. Стильстрока 8
    Нет IP-адреса и id запроса в событии успешного входа.

Разбор

Логировать нужно факт и контекст события (кто, откуда, результат), но никогда секреты. Структурированный логгер с полями вместо конкатенации решает и инъекцию, и маскирование.

Ответьте сами, а не прочитайте

В тренажёре вопрос попадёт в повторения и вернётся, когда начнёт забываться. Плюс билеты с зачётом: 20 вопросов, не больше 2 ошибок.