Задача и её решение
Как-то раз мне понадобилось добавить в своё приложение на Ruby симметричное шифрование. Алгоритм AES показался мне хорошим выбором и я решил найти библиотеку шифрования с поддержкой этого алгоритма. Поскольку я писал на Ruby, то сделал то же самое, что сделал бы на моём месте практически каждый программист на Ruby — пошел в Google и написал запрос «ruby gem aes». Конечно же, Google первой строкой предложил мне gem, называющийся (вот неожиданность!) — «aes». Он был очень прост в использовании:
require 'aes'
message = "Super secret message"
key = "password"
encrypted = AES.encrypt(message, key) # RZhMg/RzyTXK4QKOJDhGJg==$BYAvRONIsfKjX+uYiZ8TCsW7C2Ug9fH7cfRG9mbvx9o=
decrypted = AES.decrypt(encrypted, key) # Super secret messageЕсли вы при расшифровке использовали неверный пароль, gem выбрасывал ошибку:
decrypted = AES.decrypt(encrypted, "Some other password") #=> aes.rb:76:in `final': bad decrypt (OpenSSL::Cipher::CipherError)Ну, отлично. Что же могло пойти не так?
Баг
После подключения gem'a я задейсвовал его функциональность в новой фиче и, просто на всякий случай, написал пару тестов для него — для расшифровки с правильным паролем и для ошибки расшифровки с неверным паролем. Во втором тесте я просто заменил первую букву пароля при расшифровке. Я рассчитывал получить ошибку расшифровки, что являлось бы в данном случае корректно пройденным тестом. И… мой тест провалился! Я не только не получил ошибку декодирования, я даже получил верно расшифрованные данные неверным паролем!
encrypted = AES.encrypt("Super secret message", "password")
decrypted = AES.decrypt(encrypted, "gassword") # "p" => "g"
decrypted #=> Super secret messageНу ничего себе! Возможно, я случайно попал на тот самый редчайший, один на миллиарды, случай, когда мне подошел и другой пароль? Что-то типа коллизии хэш-функций или вроде того. Следующей попыткой я изменил уже два символа в пароле:
encrypted = AES.encrypt("Super secret message", "password")
decrypted = AES.decrypt(encrypted, "ggssword") # "pa" => "gg"
decrypted #=> Super secret messageИ опять-таки получил успешно расшифрованное сообщение! Ну, оставалось лишь одно. Я попробовал совершенно другой пароль:
encrypted = AES.encrypt("Super secret message", "password")
decrypted = AES.decrypt(encrypted, "totally wrong password")
decrypted #=> Super secret messageЭто уже выглядело кричащей дырой в безопасности, так что я решил разобраться, что же здесь происходит.
Отладка
Проблема возникала из-за следующей строки в коде gem'a:
@cipher.key = @key.unpack('a2'*32).map{|x| x.hex}.pack('c'*32)Прежде всего давайте объясним, что делает unpack. В данном случае она разделяет входную строку на массив из 32 строк (см. документацию):
"password".unpack("a2"*32)
=> ["pa", "ss", "wo", "rd", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", ""]Далее, для каждой из полученных строк вызывается метод #hex. String#hex в Ruby конвертирует строки, содержащие hex-числа в целые числа (а если конвертация не удаётся, то в число 0).
'9'.hex #=> 9
'a'.hex #=> 10
'10'.hex #=> 16
'ff'.hex #=> 255
# 0 в случае ошибки конвертации:
'foobar'.hex #=> 0
'zz'.hex #=> 0Таким образом, любая строка, не содержащая в себе корректное hex-число, будет трансформирована в массив из 32 нулей.
"pa".hex #=> 0
"ss".hex #=> 0
"wo".hex #=> 0
"rd".hex #=> 0
"".hex #=> 0
"password".unpack("a2"*32).map { |x| x.hex }
#=> [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
"totally wrong password".unpack("a2"*32).map { |x| x.hex }
#=> [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]То есть мы практически всегда можем расшифровать любое зашифрованное сообщение любым паролем. Я думаю, автор подразумевал, что входным параметром функции шифрования всегда будет hex-число (и в этом случае gem сработал бы надёжно). Однако интерфейс gem'а не предполагает никаких ошибок при шифровании с обычной строкой, что приводит к ложному ощущению надёжности шифрования при его полном фактическом отсутствии.
Выводы
aes — не очень распространённый gem. На момент написания статьи у него на GitHub'е вcего 45 звёзд и 13 форков. Но проблема в том, что Google выдаёт его первым результатом по запросам «aes gem» или «ruby aes gem», а мы часто верим в то, что топовые результаты поисковых запросов ведут на качественные и популярные библиотеки. Часто программисты вообще не задумываются над проверкой и написанием тестов для подключаемых в проект внешних библиотек. Как вы видите из этого примера — такое поведение несёт в себе опасность.
Технические детали:
— Gem: github.com/chicks/aes
— Версия с данной ошибкой: 0.5.0 / 12c3648
Комментарии (20)
sidny_vicious
13.01.2017 18:55Повезло, что тесты сделали, иначе всплыло бы в самый неподходящий момент.
youlose
13.01.2017 19:04+2А зачем гем, если симметричное шифрование есть в stdlib?
ProRunner
13.01.2017 20:17Как видно из обсуждения бага, гем и делался как обертка над OpenSSL::Cipher
nikitasius
14.01.2017 00:46Если мне память не изменяет, то эта тема уже освещалась на хабре. Линк найти не могу, в избранное не добавил.
webhamster
14.01.2017 08:28+2Человек все правильно сделал. Взял готовый инструмент с открытым исходным кодом, воспользовался им и даже протестировал. В результате вот эта статья.
Того, кто работал с криптой должно было сразу напрячь имя key которое далеко не password. Традиционно, ключ шифрования — это не пароль шифрования. И в современном мире даже не MD5 от пароля, а еще и перемешивание битов чем нибудь типа pbkdf2 на 80000 раундов. Человек мог этого не знать, но авторы криптографической библиотеки должны были сделать две вещи: четко написать о значении key в доке, не используя принцип «минимально и достаточно», как это обычно бывает когда разрабы ленятся (а вообще дока была?), и сделать защиту от дурака, а не тупо преобразовывать не-hex символы в нули.
dim2r
14.01.2017 16:16-1Я последнее время насмотрелся творчества програмистов, в проекте порядка гигабайта исходников. Прихожу к выводу, на уровне языка надо вводить семантику типа
if условие then
действе
else
raise exception 'вызывается принудительно на уровне языка
end if
То есть программист должен перечислить все случаи, когда процедура работает. Во всех остальных случаях должна выводиться ошибка.
lorc
Хм, в примере же явно написано «key», а не «password». И в документации написано «key». Почему вы решили что там должен быть пароль?
Конечно, автор гема мог бы проверить, что на вход подается корректная HEX строка.
А вы, в свою очередь, могли бы немного разобраться в криптографии. Потому что бездумное использование криптоалгоритмов не добавит безопасности вашему приложениею.
Кстати, основная проблема этого гема не в этом. А в том — что он не позволяет выбрать режим шифрования. Подозреваю, что если не предоставить ему IV, то он будет шифровать в режиме ECB, что никуда не годится.
tangro
Это не я, это автор оригинальной строки. И он придерживался, в общем-то распространённого паттерна поведения: «скачать, попробовать использовать, если что-то не получается — читать документацию». Просто методы шифрования и дешифрования вроде-бы корректно сработали, ошибок не возникло, расшифровалось именно то, что шифровалось. Очень многие на этом месте решили бы, что всё ок.
lorc
Это ж крипта. Правильнее было бы расшифровать зашифрованное другой реализацией AES.
lorc
А, это я с переводом общался. Прошу прощения. Но в любом случае автор неправ.
Jogger
Прав автор или нет, но отсутствие проверки на корректность входных данных — это недостаток библиотеки. Если конечно это не закрытая библиотека которой пользуется только лично её автор.
nukedsun
Попытка обьяснить автору, что substr оперирует символами, а не байтами и, скажем, если mbstring.func_overload = 2, то его функция в общем случае не работает вообще — ни к чему не привела. Ответ был один — «У меня работает».
artyfarty
Беда в том что зачастую слово key используется как более прикольная/заумная/краткая замена password, поэтому не каждый программист вообще подозревает о разнице, и даже если подозревает, не особо надеется, что она подразумевалась другим программистом.
iroln
Библиотека кривая же, что и выявило тестирование. Именно с точки зрения API. Пользователь не хочет думать, корректная у него hex-строка или не корректная, он хочет просто задать ключ шифрования.
Вот, например, pycrypto для python. key — просто набор байтов.
https://www.dlitz.net/software/pycrypto/api/current/Crypto.Cipher.AES-module.html
И никаких проблем.