Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix a Message.signedWithCerts property content #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zavla
Copy link

@zavla zavla commented May 18, 2023

Доброго здоров'я.
Маю один p7s файл і щось не перевіряється підпис, помилки не виводиться, в ньому коли використовую ваш github.com/dstucrypt/agent.
Перевіряю в режимі --verify --ocsp lax. З'ясував що саме перевірка --ocsp викликає помилку, яка є коли в об'єкт Message, в поле signedWithCerts, що є масивом, записувалось три об'єкта що представляють сертифікати, які читаються з цього p7s файла. Першій 'сертифікат' був з полем serialNumber, інші два об'єкта були тільки з полем keyId. Я пошукав що там в поле keyId зачиталось з p7s файла.
Використав http://lapo.it/asn1js щоб подивитись байти з keyId у файлі p7s. Маю картинку де видно ці байти в файлі і які ідентифікатори asn1 їм відповідають. Я так зробив висновок що то пакет asn1.js помилково розбирає p7s.
В цьому PR я пропоную корекцію к полю signedWithCerts що
б взагалі відсікати такі прочитані сертифікати в яких немає поля serialNumber. Тести проходять ті що в вас є в jkurwa.
Але я не впевнений що зроблено вірно для всіх, але мою помилку виправило і також коректно перевіряє p7s файли ті що раніше і так перевірялись.

@zavla
Copy link
Author

zavla commented May 18, 2023

то что в keyId

@muromec
Copy link
Member

muromec commented May 19, 2023

То воно получається не буде перевіряти один з сертифікатів, так? Це нехорошо.

@muromec
Copy link
Member

muromec commented May 19, 2023

Я так зробив висновок що то пакет asn1.js помилково розбирає p7s.

Я файла не бачив, але підозрюю що розбирає він все правильно і там дійсно вказан keyid, а serial не вказан. Це нормально. Треба дивитись на ctx.jsL317~318 чого воно не може знайти той сертифікат по keyid.

@zavla
Copy link
Author

zavla commented May 19, 2023

То воно получається не буде перевіряти один з сертифікатів, так? Це нехорошо.

В даному проблемному p7s, доречи сертифікат виданий uakey.com.ua АЦСК Україна, вказано що дані підписано одним сертифікатом. Тобто в групі байтів що відповідають по asn1 идентифікатору certificates є тілки один елемент. Це можна бачити на малюнку, зверху приблизно 9-тий рядок.
А от jkurwa після розбору даних в Message вважає що дані підписано трьома сертифікатами. Другий и третій елементи масиву signedWithCerts jkurwa каже мають лише одне поле keyId і воно однаково у цих двох елементів. Але це поле, 32 байти, можно бачити на малюнку, обведено червоним, воно належить ідентификатору asn1 contentTimestamp і знаходиться воно, тобто ці байти що вважаються keyId, знаходяться в полях SignerInfos/SignerInfo/signedAttrs п'ятй атрибут той що contentTimestamp.
Я сам не знаю, але хіба це keyId? contentTimestamp - може це якесь значення часу а не keyId? Оці 32 байти що зайшли в jkurwa в список сертифікатів що підписали, вони на малюнку окреслені червоним.

@muromec
Copy link
Member

muromec commented May 20, 2023

Я сам не знаю, але хіба це keyId? contentTimestamp - може це якесь значення часу а не keyId?

Це просто ще один p7s контейнер, а в ньому позначка часу пидписана TSP сервером, тому функція і рекурсивна.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants