Remove mbed tls from evsev2g #1008
Conversation
MarzellT
requested review from
corneliusclaussen,
SebaLukas and
james-ctc
as code owners
|
There are some left overs in: dependencies.yaml and module-dependencies.cmake Also please squash commits and provide proper commit description before opening to review |
|
I asked the community about the mbedtls deletion at the Car WG meeting on Wednesday and heard nothing against it. So we can continue here. |
SebaLukas
added
the
post-release
MarzellT
force-pushed
the
feat/remove-mbed-tls-from-evsev2g
branch
from
58e2c7a to
de3f4b1
Compare
Signed-off-by: MarzellT <tobias.marzell@pionix.de>
MarzellT
force-pushed
the
feat/remove-mbed-tls-from-evsev2g
branch
from
de3f4b1 to
36f8b87
Compare
MarzellT
requested review from
hikinggrass,
Pietfried,
maaikez and
AssemblyJohn
as code owners
| if (v2g_ctx->tls_security != TLS_SECURITY_PROHIBIT) { | ||
| v2g_ctx->local_tls_addr = static_cast<sockaddr_in6*>(calloc(1, sizeof(*v2g_ctx->local_tls_addr))); | ||
| connection_ssl_initialize(); | ||
| if (!v2g_ctx->local_tls_addr) { | ||
| dlog(DLOG_LEVEL_ERROR, "Failed to allocate memory for TLS address"); | ||
| return -1; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Is the code not needed? Or was v2g_ctx->local_tls_addr only for mbedtls?
There was a problem hiding this comment.
local_tls_addr is still needed for OpenSSL version.
Background:
connection.cpp does two things:
- sets up a TCP connection (for non-TLS connections)
- sets up the TLS socket (for both mbedtls and openssl)
the reason was to use the tested socket creation code rather than replace it. The new OpenSSL code can use a specified socket (as done here) or create as socket when needed.
The advantage here is that the SDP needs information on the socket to respond to the UDP request and using the common code means that it didn't need changing.
There was a problem hiding this comment.
@MarzellT Then please revert removing this code section.
|
@james-ctc Can you please have a quick look, too? :) |
| @@ -226,15 +144,6 @@ int connection_init(struct v2g_context* v2g_ctx) { | |||
| } | |||
| } | |||
|
|
|||
| if (v2g_ctx->tls_security != TLS_SECURITY_PROHIBIT) { | |||
| v2g_ctx->local_tls_addr = static_cast<sockaddr_in6*>(calloc(1, sizeof(*v2g_ctx->local_tls_addr))); | |||
| connection_ssl_initialize(); | |||
There was a problem hiding this comment.
the call to connection_ssl_initialize(); can be removed, the rest is still needed
| @@ -358,61 +265,37 @@ ssize_t connection_read(struct v2g_connection* conn, unsigned char* buf, size_t | |||
|
|
|||
| int num_of_bytes; | |||
|
|
|||
| if (conn->is_tls_connection) { | |||
There was a problem hiding this comment.
connection_read() should only be used for non-TLS connections.
The removed if statement is a safety check that connection_read() hasn't been accidentally configured for a TLS connection.
| @@ -445,28 +328,13 @@ ssize_t connection_write(struct v2g_connection* conn, unsigned char* buf, size_t | |||
| while (bytes_written < count) { | |||
| int num_of_bytes; | |||
|
|
|||
| if (conn->is_tls_connection) { | |||
There was a problem hiding this comment.
The if statement is needed to ensure that connection_write() is not used on a non-TLS connection
Describe your changes
This Pull Request removes the mbed tls dependency of the EvseV2G module with all its usages. TLS connections will now only be handled by OpenSSL.
Issue ticket number and link
Checklist before requesting a review