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

pkcs_1_pss_decode.c #638

Closed
headscott opened this issue Nov 22, 2023 · 3 comments
Closed

pkcs_1_pss_decode.c #638

headscott opened this issue Nov 22, 2023 · 3 comments

Comments

@headscott
Copy link

I encountered a problem in pkcs_1_pss_decode. It's located at src/pk/pkcs1/pkcs_1_pss_decode.c

While doing some changes for a TLS project I used eduardsui/tlse (from Github) and found a problem in parsing the certificate verify packet. eduardsui used this library for calculating values and to verify data from TLS packets. The parse method used your pkcs_1_pss_decode. Inside this method it fails in the following part:

for (x = 0; x < modulus_len - saltlen - hLen - 2; x++) {
if (DB[x] != 0x00) {
err = CRYPT_INVALID_PACKET;
goto LBL_ERR;
}
}

and returns the error CRYPT_INVALID_PACKET. I am not sure exactly where the problem is, but I found out, that salt is allocated inside this method, but never used. There are just some comments, where salt is mentioned, but there is no usage. Could this be the problem? And if not, can you just remove these lines, where salt is allocated, set free and the memory set to zero?

Thanks in advance.
F. T.

@sjaeckel
Copy link
Member

sjaeckel commented Nov 23, 2023

Hi,

you're right, salt is allocated and never used, but that's not the problem.

Removing the verification that PS starts with 0x0 is absolutely no option.

Looking at the calling code and the TLS RFC I suspect that the salt length is wrong.

Here's a potential patch 0001-Fix-length-of-the-salt.patch.txt which could probably solve the issue, but I couldn't get tlssimple.c to connect to something since the certificates fail to load or I don't know what, so I couldn't test it. (Looks like I'm hitting eduardsui/tlse#88 as well).

Please get this sorted out with @eduardsui

@headscott
Copy link
Author

Hey,

I fixed these lines now. It doesn't fail anymore in the check for DB == 0x00, but it fails here:

if (XMEMCMP(mask, hash, hLen) == 0) { *res = 1; }

i am not sure if this is a problem of the TLS code or of the pkcs_1_pss_decode method. I would be very happy if you could help me here again. @eduardsui is not answering since months.

@headscott
Copy link
Author

I found the solution btw. Thank you anyways. If you are interested for the solution, look at
eduardsui/tlse#89
There is my comment for that issue.

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

No branches or pull requests

2 participants