Skip to content

Conversation

@rlubos
Copy link
Contributor

@rlubos rlubos commented Nov 15, 2018

This PR fixes secure sockets implementation by removing invalid typecasts between socket descriptor and net_context. Note, that this PR does not add full support for POSIX APIs for secure sockets (like read/write), it's just a bugfix.

With FD table introduction, net_context can no longer be reached by typecasting socket descriptor to net_context, and vice versa. Instead, file descriptor API have to be used to reach the net_context.

As reverse conversion is not possible (net_context -> file descriptor), we need to provide file descriptor to functions that call zsock_* API (i.e. mbedTLS bio functions).

Additionally, second commit fixes a bug with getsockopt/setsockopt reuturn values, spotted during rework.

ztls_setsockopt and ztls_getsockopt returned error codes instead of
setting errno in particular cases. This commit fixes it.

Signed-off-by: Robert Lubos <[email protected]>
@rlubos rlubos changed the title Fix secure socket after FD table introduction Fix secure sockets after FD table introduction Nov 15, 2018
@codecov-io
Copy link

codecov-io commented Nov 15, 2018

Codecov Report

Merging #11405 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #11405   +/-   ##
=======================================
  Coverage   48.37%   48.37%           
=======================================
  Files         265      265           
  Lines       42188    42188           
  Branches    10137    10137           
=======================================
  Hits        20408    20408           
  Misses      17703    17703           
  Partials     4077     4077

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7d1ce6...b53e57a. Read the comment docs.

With FD table introduction, net_context can no longer be reached by
typecasting socket descriptor. Instead, file descriptor API have to be
used.

Signed-off-by: Robert Lubos <[email protected]>
@rlubos rlubos force-pushed the make-tls-sockets-work-with-fd-table branch from 1d2e749 to b53e57a Compare November 15, 2018 12:10

static inline struct net_context *sock_to_net_ctx(int sock)
{
return z_get_fd_obj(sock, NULL, ENOTSOCK);
Copy link
Contributor

Choose a reason for hiding this comment

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

I hope we both understand that passing vtable=NULL is a hack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we do. I'm already working on a solution for secure sockets, according to this comment.

if (tls_proto != 0) {
/* If TLS protocol is used, allocate TLS context */
struct net_context *context = INT_TO_POINTER(sock);
struct net_context *context = sock_to_net_ctx(sock);
Copy link
Contributor

Choose a reason for hiding this comment

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

No NULL check here, unlike done in other places.

Copy link
Contributor

@pfalcon pfalcon left a comment

Choose a reason for hiding this comment

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

Let me invoke "Trivial patch" spell and approve.

@pfalcon pfalcon added the Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. label Nov 15, 2018
@nashif
Copy link
Member

nashif commented Nov 15, 2018

Let me invoke "Trivial patch" spell and approve.

note that trivial sets minimum review time, it does not mean it needs to be expedited. hope that was clear in the proposal.

@nashif nashif merged commit 824d0bd into zephyrproject-rtos:master Nov 15, 2018
@pfalcon
Copy link
Contributor

pfalcon commented Nov 16, 2018

@nashif :

note that trivial sets minimum review time, it does not mean it needs to be expedited. hope that was clear in the proposal.

For me, those are quite related. Having "Trivial" category from my PoV is effectively a pledge along the lines of: "Dear fellow developer, you see that we're stuck at 250+ unprocessed patches, and that number is only growing. Then you may wonder, if you see any c%ap in our code, does it make sense to try to fix it, or it's useless, and it's better to just stick to work on what your boss told you. We have good news for you - if you submit us an easy to review patch, we'll actually try review it as soon as 4 hrs."

If that's (improving the process for contributors, again, as was announced by you 3 weeks ago) not the intention behind all this, then well, d'oh.

@rlubos rlubos deleted the make-tls-sockets-work-with-fd-table branch November 29, 2018 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Networking Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants