You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
While that will catch errors that interrupt parsing with unread data remaining, it won't catch errors that leave the stuffer empty. For example, a trailing "-----BEGIN CERTIFICATE-----" probably should cause an error (and does if it's alone in the file) but is ignored if at least one certificate was previously read. We'd have a similar problem if the final "-----END CERTIFICATE----" was missing. That also means that we could have even read partial base64 certificate data, but the remainder and the final delimiter were missing.
Solution:
Tighten up the error handling when certificate parsing fails. We probably want to distinguish between errors that occur between the begin delimiter and the end delimiter, and errors that occur before the begin or after the end.
Does this change what S2N sends over the wire? If yes, explain.
Does this change any public APIs? If yes, explain.
Which versions of TLS will this impact?
Requirements / Acceptance Criteria:
What must a solution address in order to solve the problem? How do we know the solution is complete?
RFC links: Links to relevant RFC(s)
Related Issues: Link any relevant issues
Will the Usage Guide or other documentation need to be updated?
Testing: How will this change be tested? Call out new integration tests, functional tests, or particularly interesting/important unit tests.
Will this change trigger SAW changes? Changes to the state machine, the s2n_handshake_io code that controls state transitions, the DRBG, or the corking/uncorking logic could trigger SAW failures.
Should this change be fuzz tested? Will it handle untrusted input? Create a separate issue to track the fuzzing work.
Out of scope:
Is there anything the solution will intentionally NOT address?
The text was updated successfully, but these errors were encountered:
Problem:
When we parse certificates, we explicitly error if the first certificate fails to parse:
s2n-tls/crypto/s2n_certificate.c
Lines 54 to 56 in 003412f
But we catch other errors by checking whether or not the stuffer is empty after parsing:
s2n-tls/crypto/s2n_certificate.c
Lines 80 to 84 in 003412f
While that will catch errors that interrupt parsing with unread data remaining, it won't catch errors that leave the stuffer empty. For example, a trailing "-----BEGIN CERTIFICATE-----" probably should cause an error (and does if it's alone in the file) but is ignored if at least one certificate was previously read. We'd have a similar problem if the final "-----END CERTIFICATE----" was missing. That also means that we could have even read partial base64 certificate data, but the remainder and the final delimiter were missing.
Solution:
Tighten up the error handling when certificate parsing fails. We probably want to distinguish between errors that occur between the begin delimiter and the end delimiter, and errors that occur before the begin or after the end.
Requirements / Acceptance Criteria:
What must a solution address in order to solve the problem? How do we know the solution is complete?
Out of scope:
Is there anything the solution will intentionally NOT address?
The text was updated successfully, but these errors were encountered: