-
Notifications
You must be signed in to change notification settings - Fork 2.8k
mbedtls_ssl_get_alert(): getter for fatal alerts #10514
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
base: development
Are you sure you want to change the base?
Changes from all commits
d589854
0841cea
f9a734f
13200ab
33bd8f8
6140cfb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| Features | ||
| * Add the function mbedtls_ssl_get_alert() which returns the | ||
| last received fatal error alert type for a more generic | ||
| MBEDTLS_ERR_SSL_FATAL_ALERT_MESSAGE return value from | ||
| mbedtls_ssl_handshake(), mbedtls_ssl_handshake_step() or | ||
| mbedtls_ssl_read(). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1296,6 +1296,9 @@ void mbedtls_ssl_session_reset_msg_layer(mbedtls_ssl_context *ssl, | |
| memset(ssl->in_buf, 0, in_buf_len); | ||
| } | ||
|
|
||
| ssl->in_alert_recv = 0; | ||
| ssl->in_alert_type = 0; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extra whitespace here and we are missing clearing the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm sorry but I can't follow what you mean with the extra whitespace? Is there an extra whitespace in the changeset missing or one that should be removed? Regarding the extra clearing: I followed the way how send alert was implemented (there is no clearing for the type for a reset neither) but added it now with 33bd8f8.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With regards to the whitespace I meant that instead of doing We could have But do not push a change just for that, wait for the second reviewer's comments. Also same applies with rebase, I would recommend to do it after both reviewers have approved a pr, so you do not have to redo it every time the base branch has created a conflict ;) Thanks for getting on it so quickly. |
||
| ssl->send_alert = 0; | ||
|
|
||
| /* Reset outgoing message writing */ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how I feel about it. The change is small to warrant a new discrete allert type for when there is no allert, but on the other side, a valid ssl context containing no allert is technically not a bad input, whereas it is a bad input for
mbedtls_ssl_get_alert.An option would be to return
MBEDTLS_SSL_ALERT_MSG_CLOSE_NOTIFYwhenssl->in_alert_recv != 1but I also does not feel right error as well.@ronald-cron-arm wdyt?