Как мы нашли критичную уязвимость AspNetCore.Mvc и перешли на собственную сериализацию
Привет, Хабр!
В этой статье мы хотим поделиться нашим опытом в оптимизации производительности и исследовании особенностей AspNetCore.Mvc.
Предыстория
Несколько лет назад на одном из наших нагруженных сервисов мы заметили существенное потребление ресурсов CPU. Это выглядело странно, так как задачей сервиса было фактически взять сообщение и переложить его в очередь, предварительно произведя над ним некоторые операции, такие как валидация, дополнение данными, и т.п.
В результате профилирования мы обнаружили, что большую часть процессорного времени «съедает» десериализация. Мы выкинули стандартный сериализатор и написали свой на Jil, в результате чего потребление ресурсов снизилось в разы. Все работало как нужно и мы успели об этом позабыть.
Проблема
Мы постоянно совершенствуем наш сервис во всех направлениях, включая безопасность, производительность и отказоустойчивость, поэтому «безопасники» проводят разного рода тесты наших сервисов. И некоторое время назад к нам «прилетает» алерт об ошибке в логе — каким-то образом мы пропустили невалидное сообщение дальше по очереди.
При детальном анализе все выглядело достаточно странно. Есть request-model (тут я приведу упрощенный пример кода):
public class RequestModel
{
public string Key { get; set; }
FromBody]
Required]
public PostRequestModelBody Body { get; set; }
}
public class PostRequestModelBody
{
[Required]
[MinLength(1)]
public IEnumerable ItemIds { get; set; }
}
Есть контроллер с action Post, например:
[Route("api/[controller]")]
public class HomeController : Controller
{
[HttpPost]
public async Task Post(RequestModel request)
{
if (this.ModelState.IsValid)
{
return this.Ok();
}
return this.BadRequest();
}
}
Все кажется логичным. Если придет запрос с Body вида
{"itemIds":["","","" … ] }
API вернет BadRequest, и на это есть тесты.
Тем не менее, в логе видим обратное. Мы взяли сообщение из логов, отправили в API и получили статус OK… и… новую ошибку в логе. Не веря своим глазам мы продебажились и убедились, что да, действительно ModelState.IsValid == true. При этом обратили внимание на необычно долгое время выполнения запроса — порядка 500ms, в то время как обычное время ответа редко превышает 50ms и это в продакшене, который обслуживает тысячи запросов в секунду!
Отличие этого запроса от наших тестов было лишь в том, что запрос содержал 600+ пустых строк…
Дальше будет много кода-букаф.
Причина
Стали разбираться, что же тут не так. Чтобы исключить ошибку, написали чистое приложение (откуда я и привел пример), с помощью которого воспроизвели описанную ситуацию. В общей сложности мы потратили пару человеко-дней на исследование, тесты, ментальный дебаг кода AspNetCore.Mvc и оказалось, что проблема в JsonInputFormatter.
JsonInputFormatter использует JsonSerializer, который, получая стрим для десериализации и тип, пытается сериализовать каждое свойство, если это массив — каждый элемент этого массива. При этом, если происходит ошибка при сериализации, JsonInputFormatter сохраняет каждую ошибку и путь к ней, помечает ее как обработанную, чтобы можно было продолжить попытку десериализации дальше.
Приведу ниже код обработчика ошибок JsonInputFormatter (он доступен на Github по ссылке выше):
void ErrorHandler(object sender, Newtonsoft.Json.Serialization.ErrorEventArgs eventArgs)
{
successful = false;
// When ErrorContext.Path does not include ErrorContext.Member, add Member to form full path.
var path = eventArgs.ErrorContext.Path;
var member = eventArgs.ErrorContext.Member?.ToString();
var addMember = !string.IsNullOrEmpty(member);
if (addMember)
{
// Path.Member case (path.Length < member.Length) needs no further checks.
if (path.Length == member.Length)
{
// Add Member in Path.Memb case but not for Path.Path.
addMember = !string.Equals(path, member, StringComparison.Ordinal);
}
else if (path.Length > member.Length)
{
// Finally, check whether Path already ends with Member.
if (member[0] == '[')
{
addMember = !path.EndsWith(member, StringComparison.Ordinal);
}
else
{
addMember = !path.EndsWith("." + member, StringComparison.Ordinal);
}
}
}
if (addMember)
{
path = ModelNames.CreatePropertyModelName(path, member);
}
// Handle path combinations such as ""+"Property", "Parent"+"Property", or "Parent"+"[12]".
var key = ModelNames.CreatePropertyModelName(context.ModelName, path);
exception = eventArgs.ErrorContext.Error;
var metadata = GetPathMetadata(context.Metadata, path);
var modelStateException = WrapExceptionForModelState(exception);
context.ModelState.TryAddModelError(key, modelStateException, metadata);
_logger.JsonInputException(exception);
// Error must always be marked as handled
// Failure to do so can cause the exception to be rethrown at every recursive level and
// overflow the stack for x64 CLR processes
eventArgs.ErrorContext.Handled = true;
}
Пометка ошибки, как обработанной производится в самом конце обработчика
eventArgs.ErrorContext.Handled = true;
Таким образом реализована фича вывода всех ошибок десериализации и путей к ним, на каких полях/элементах они были, ну… почти всех…
Дело в том, что у JsonSerializer есть ограничение на 200 ошибок, после которого он падает, при этом падает весь код и ModelState становится… валидной! … теряются и ошибки.
Решение
Не долго думая, мы реализовали свой форматтер Json для Asp.Net Core с использованием Jil Deserializer. Поскольку для нас абсолютно не важно количество ошибок в body, а важен лишь факт их наличия (да и в целом сложно представить себе ситуацию, когда это было бы действительно полезно), то код сериализатора получился достаточно простым.
Приведу основной код кастомного JilJsonInputFormatter:
using (var reader = context.ReaderFactory(request.Body, encoding))
{
try
{
var result = JSON.Deserialize(
reader: reader,
type: context.ModelType,
options: this.jilOptions);
if (result == null && !context.TreatEmptyInputAsDefaultValue)
{
return await InputFormatterResult.NoValueAsync();
}
else
{
return await InputFormatterResult.SuccessAsync(result);
}
}
catch
{
// какая-то обработка ошибок
}
return await InputFormatterResult.FailureAsync();
}
Внимание! Jil чувствителен к регистру, это значит, что содержимое Body
{"ItemIds":["","","" … ] }
и
{"itemIds":["","","" … ] }
не одно и тоже. В первом случае, если используется camelCase в свойство ItemIds после десериализации будет null.
Но так как это наш API, мы используем и контролируем его, для нас это не критично. Проблема может возникнуть, если это публичный API и кто-то уже вызывает его, передавая имя параметров не в camelCase.
Результат
Результат даже превзошел наши ожидания, API ожидаемо стал возвращать на искомый запрос BadRequest и делать это очень быстро. Ниже приведу скрины некоторых наших графиков, на которых хорошо видны изменения времени ответа и CPU, до и после деплоя.
Время выполнения запроса:
Примерно в 16:00 был деплой. До деплоя время выполнения p99 составляло 30–57ms, после деплоя стало 9–15ms. (На повторные пики 18:00 можно не обращать внимания — это был уже другой деплой)
Вот так изменился график CPU:
По этому поводу мы завели issue в Github, на момент написания статьи она была помечена как bug с milestone 3.0.0-preview3.
В заключение
Пока проблема не решена, мы считаем что лучше отказаться от использования стандартной десериализации, особенно, если у вас публичный API. Зная эту проблему, злоумышленник может легко положить ваш сервис, закинув в него кучу подобных невалидных запросов, ведь чем больше ошибочный массив, чем больше Body, тем дольше происходит обработка в JsonInputFormatter.
Артем Асташкин, Руководитель группы разработки
