Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion Doc/library/ssl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,17 @@ Exceptions
example ``CERTIFICATE_VERIFY_FAILED``. The range of possible
values depends on the OpenSSL version.

.. versionadded:: 3.3
.. attribute:: error_queue

The full OpenSSL error queue, as a list of strings, where the last
item of the list is the top of the queue. If :attr:`reason` is not enough
to diagnose a problem, OpenSSL may report more detail through this queue.
The format of the strings follows that of OpenSSL's
`ERR_error_string <https://docs.openssl.org/master/man3/ERR_error_string/>`_,
and may change between OpenSSL versions (i.e. do not rely on the exact
wording of these messages).

.. versionadded:: 3.16
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI we spell this "versionadded:: next" and the docs automation replaces it with the right version at the right time. (don't worry about it, 3.16 is correct)


.. exception:: SSLZeroReturnError

Expand Down
4 changes: 4 additions & 0 deletions Lib/test/test_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -1909,6 +1909,7 @@ def test_lib_reason(self):
self.assertEqual(cm.exception.library, 'PEM')
regex = "(NO_START_LINE|UNSUPPORTED_PUBLIC_KEY_TYPE)"
self.assertRegex(cm.exception.reason, regex)
self.assertTrue(len(cm.exception.error_queue) >= 1)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertGreaterEqual

s = str(cm.exception)
self.assertIn("NO_START_LINE", s)

Expand Down Expand Up @@ -4688,6 +4689,7 @@ def cb_returning_alert(ssl_sock, server_name, initial_context):
chatty=False,
sni_name='supermessage')
self.assertEqual(cm.exception.reason, 'TLSV1_ALERT_ACCESS_DENIED')
self.assertTrue(len(cm.exception.error_queue) >= 1)

def test_sni_callback_raising(self):
# Raising fails the connection with a TLS handshake failure alert.
Expand All @@ -4708,6 +4710,7 @@ def cb_raising(ssl_sock, server_name, initial_context):
"|SSLV3_ALERT_HANDSHAKE_FAILURE"
"|NO_PRIVATE_VALUE)")
self.assertRegex(cm.exception.reason, regex)
self.assertTrue(len(cm.exception.error_queue) >= 1)
self.assertEqual(catch.unraisable.exc_type, ZeroDivisionError)

def test_sni_callback_wrong_return_type(self):
Expand All @@ -4727,6 +4730,7 @@ def cb_wrong_return_type(ssl_sock, server_name, initial_context):


self.assertEqual(cm.exception.reason, 'TLSV1_ALERT_INTERNAL_ERROR')
self.assertTrue(len(cm.exception.error_queue) >= 1)
self.assertEqual(catch.unraisable.exc_type, TypeError)

def test_shared_ciphers(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Expose OpenSSL's error queue through SSLError.error_queue
51 changes: 51 additions & 0 deletions Modules/_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,8 @@ fill_and_set_sslerror(_sslmodulestate *state,
PyObject *verify_obj = NULL, *verify_code_obj = NULL;
PyObject *init_value, *msg, *key;
PyUnicodeWriter *writer = NULL;
PyObject *error_queue = PyList_New(0);
if (!error_queue) goto fail;

if (ssl_errno == PY_SSL_ERROR_EOF && sslsock != NULL) {
sslsock->got_eof_error = 1;
Expand Down Expand Up @@ -542,6 +544,45 @@ fill_and_set_sslerror(_sslmodulestate *state,
if (errstr == NULL) {
errstr = ERR_reason_error_string(errcode);
}

/* populate error_list from OpenSSL's error queue */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error_queue?

unsigned int q_pos = 0; /* Error queue position */
const char *eq_filename, *eq_func, *eq_data;
char eq_msg[256];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather we did not put large things on the stack. Have it be a pointer set to NULL here. allocate space within the while loop if null before we use it. free it unconditionally when not NULL later on the ways out of the function.

int eq_lineno;
int flags; /* Flags (discarded) */
unsigned long openssl_errorcode;

/* Presumably, we no longer need the OpenSSL error queue after this, so
we can call ERR_get_error (destructive) instead of ERR_peek_error */
while ((openssl_errorcode = ERR_get_error_all(&eq_filename, &eq_lineno, &eq_func,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is OpenSSL >= 3.x only - we also work with some 1.1.1-ish forks such as AWS-LC and can still build and link against 1.1.1 itself so conditional compilation is likely needed here.

&eq_data, &flags))) {
if (q_pos == 0) {
/* errcode should have come from a caller, and should have been
returned from ERR_peek_last_error() */
assert(openssl_errorcode == errcode);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this correct? oldest first in one list vs value in errcode being the last one when there are multiple.

}

ERR_error_string_n(openssl_errorcode, eq_msg, 256);

// Follows [lib reason] error_string extra_data (OpenSSL file:func:line)
PyObject *current_eq_msg = NULL;
if (eq_data != NULL) {
current_eq_msg = PyUnicode_FromFormat(
"%s %s (%s:%s:%d)",
eq_msg, eq_data, eq_filename, eq_func, eq_lineno
);
} else {
current_eq_msg = PyUnicode_FromFormat(
"%s (%s:%s:%d)",
eq_msg, eq_filename, eq_func, eq_lineno
);
}
if (PyList_Append(error_queue, current_eq_msg) != 0) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decref current_eq_msg after this (regardless of success or failure) to avoid a memory leak.

goto fail;
}
q_pos++;
}
}

/* verify code for cert validation error */
Expand Down Expand Up @@ -655,6 +696,12 @@ fill_and_set_sslerror(_sslmodulestate *state,
goto fail;
}

/* Add the full OpenSSL error queue to exception */
if (PyObject_SetAttr(err_value, state->str_error_queue, error_queue) != 0) {
goto fail;
}
Py_DECREF(error_queue);

PyErr_SetObject(type, err_value);
fail:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fail: needs to Py_XDECREF(error_queue);

Py_XDECREF(err_value);
Expand Down Expand Up @@ -7359,6 +7406,10 @@ sslmodule_init_strings(PyObject *module)
if (state->str_verify_code == NULL) {
return -1;
}
state->str_error_queue = PyUnicode_InternFromString("error_queue");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a relevant Py_CLEAR(state->...) for this.

if (state->str_error_queue == NULL) {
return -1;
}
return 0;
}

Expand Down
1 change: 1 addition & 0 deletions Modules/_ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ typedef struct {
PyObject *str_reason;
PyObject *str_verify_code;
PyObject *str_verify_message;
PyObject *str_error_queue;
/* keylog lock */
PyThread_type_lock keylog_lock;
} _sslmodulestate;
Expand Down
Loading