Missing Error Check in function SSL_new()

[lc3412]

Function SSL_new() returns NULL if the creation of a new SSL structure failed. However , the return value of function SSL_new() is not checked. See the following code:

line : 1189

trafficserver/tools/http_load/http_load.c

Lines 1189 to 1197 in 5ee6a5f

	connections[cnum].ssl = SSL_new(ssl_ctx);
	SSL_set_fd(connections[cnum].ssl, connections[cnum].conn_fd);
	r = SSL_connect(connections[cnum].ssl);
	if (r <= 0) {
	(void)fprintf(stderr, "%s: SSL connection failed - %d\n", argv0, r);
	ERR_print_errors_fp(stderr);
	close_connection(cnum);
	return;
	}

===============================================================================

We find the return value of this call been checked in openssl project with the version of openssl 1.1.2.
Such as in openssl/apps folder

line : 206
Ref : https://github.com/openssl/openssl/blob/0db957dbbcf6a432086ab913378c23636d8c374c/apps/ciphers.c#L206-L208

line 206:    ssl = SSL_new(ctx);
line 207:    if (ssl == NULL)
line 208:       goto err;

Chi Li, Zuxing Gu, Jiecheng Wu

[bryancall]

Relates to #4292

[bryancall]

@lc3412 Did you want to make a PR for this?

[PSUdaemon]

I've found SSL_new() usage in 9 different places. Sometimes we check that it's non-NULL before taking actions on it, but then we do cleanup (eg SSL_free()) regardless. So I'm not sure if these APIs deal with NULL properly or we need to add more checks and also, how should we properly be handling errors? I think this needs to be handled within the context of the overall ATS SSL arch. Perhaps @shinrich can take a look? It's also possible that nothing at all needs to be done in most cases.

[alokrahul513]

Hi, I want to work on this issue. How to get started?

[bryancall]

@shinrich is going to put up a PR for this