AcornSSL server functionality

The socket functions are blocking unless marked as not, it’s not for you to decide, someone at Xerox in the 1970’s already made the decision.

That’s utterly unhelpful. I’m not proposing to change the Internet module. I’m suggesting that we recognise that RISC OS uses co-operative multi-tasking, and that the AcornSSL module is specific to RISC OS, so it seems convenient to make the resulting handle non-blocking. When you create a socket, what’s the first thing you do? Make it non-blocking. So, for server applications (note, I’m not proposing changing client aspects of AcornSSL), it cuts out one step of app-level programming.

In terms of compatibility:

a) There’s no secure server code out there to fail to be compatible with.

b) If you go ahead and mark it non-blocking yourself, it will work exactly as well. Nothing will break as a result.

c) The only thing that will break, AFAICS, is a secure server app that requires the sockets to be blocking and would malfunction if they were non-blocking. Anyone seen any code meeting that specificaion? No, I thought not.

d) If you really do require blocking secure server sockets, you can perectly well set them blocking as soon as you open them. Remember, there’s no secure server code out there using AcornSSL.

While waiting for a handshake to complete use ENOTCONN (cf. dialup waiting for connection). This is so ported applications will work without requiring extensive modification, since their data reading/sending loop should already be structured such that the loop is retried.

I’m happy to make that change.

Testers, please take this as advance notice of the change – my test builds of AcornSSL from my web site, from today onwards, will have the above change.

Edited: the return codes are up for discussion, please see below.

While waiting for a handshake to complete use ENOTCONN

This is odd to me feels like an error code picked at random.

Handshaking is done in AcornSSL while using read or write. Neither socket read nor socket write should return ENOTCONN so returning ENOTCONN breaks normal read/write loops. I’ve been struggling to think of a rationale for ENOTCONN and I can only think that it is so that a non blocking socket can return. For a non blocking socket EWOULDBLOCK is the obvious reply.

So I would vote for EWOULDBLOCK for nonblocking sockets and ENOTCONN for blocking sockets then normal service can be resumed for non blocking socket users. At the moment I put a wrapper around acornssl_read/write and convert ENOTCONN to EWOULDBLOCK that way there is no changes required to read/write loops. I do think though both should be EWOULDBLOCK yes a blocking socket doesn’t expect EWOULDBLOCK but it doesn’t expect ENOTCONN either so if you have to change your code for ENOTCONN you may as well change it for EWOULDBLOCK and at least the error tells you what is happening.

Interesting – Colin, you make a strong case for EWOULDBLOCK.

Other opinions?

Dave: I’m trying to track down why your handshake works and the existing one doesn’t. Are you just calling mbedtls_ssl_handshake and modifying the reply or is it more complicated than that.

Oh and couldn’t your handshake code be put in handshake_sslhandle in AcornSSL.c.api so that if the ssl_handle is a server it does your code otherwise the existing code is used. that way we don’t need the handshake swi.

Are you just calling mbedtls_ssl_handshake and modifying the reply

That’s exactly what I’m doing. No more, no less.

I did have another look at handshake_sslhandle but a lot of it seems specific to handling the client handshake, so I steered clear of it.

I’m trying to track down why your handshake works and the existing one doesn’t.

Very interesting. I haven’t changed any part of the client handshake, unless there has been some editing accident.

I wonder if some state variable of the handshake process has been left as a global instead of being put into an ssl context? It wouldn’t show a problem unless and until two handshakes were taking place simultaneously. That would also explain why I couldn’t get my server code responding to NetSurf on the same machine, but it worked when NetSurf was on a separate machine.

Ah, I think I should be calling mbedtls_ssl_handshake_step.

Colin, please try the one that’s on my web site now. This handshakes one step at a time.

NOTE TO EVERYBODY: This does NOT change the return codes. I now consider them up for discussion.

(But I did remember to change the date.)

Ah, I think I should be calling mbedtls_ssl_handshake_step

mbedtls_ssl_handshake should be ok and it guards against calling mbedtls_ssl_handshake_step when the handshake s over

I did have another look at handshake_sslhandle but a lot of it seems specific to handling the client handshake, so I steered clear of it.

handshake_sslhandle is just used at the beginning of api_recv and api_send. Just add

if (ssl_handle is a server socket) return my_handshake_function(ssl_handle);

to the beginning of handshake_sslhandle.

It’s difficult at the moment to see why handshake_sslhandle shouldn’t just work for the server code as it stands.

Ah You may need to use mbedtls_ssl_handshake_step to do it nonblocking

You may need to use mbedtls_ssl_handshake_step to do it nonblocking

Exactly.

The worrying part is that AcornSSL_Send and AcornSSL_Recv both call the internal function handshake_sslhandle, which calls mbedtls_ssl_handshake_step in a loop. That’s probably fine in the client case, with one SSL handle, but AFAICS it’s likely to be problematic in the server case with multiple handles. It looks like I need to get them to call different code if the handle is marked as a server.

I’m suggesting that we recognise that RISC OS uses co-operative multi-tasking, and that the AcornSSL module is specific to RISC OS, so it seems convenient to make the resulting handle non-blocking.

Obviously I’m not doing any coding here but that sentence is full of presumptions that things will not change and as this is RISC OS lets do our own thing.

Thats why we have things like 20 year internet stacks etc and hence my vote on anything is lets keep things aligned to what ever is done on other systems and where it makes it easier to do up dates etc.

Mbedtls is a great example of how RISC OS should be doing it as we get updates fairly quickly after they are released.

If we do it with one eye on where we get ported code from it means it is a bit easier to keep up to date and to make use of that functionality with other potentially ported applications etc.

What I would not want to see is a lot of effort being done now that makes it harder to maintain going forward.

Having said all of that it is great that someone is making the effort to further improve functionality on our system so thanks for all the hard work so far.

The worrying part is that AcornSSL_Send and AcornSSL_Recv both call the internal function handshake_sslhandle, which calls mbedtls_ssl_handshake_step in a loop

That’s not a problem. The listening socket never calls read/write. The accepted socket will do the handshake on the first read/write and when complete the ssl.state is OVER and you must never call mbedtls_ssl_handshake_step once the state is OVER.

Good point, thanks.

You may need to use mbedtls_ssl_handshake_step to do it nonblocking

Not necessarily mbedtls_ssl_handshake will return on EWOULDBLOCK. Looking at the client handshake in the existing AcornSSL If I replace that with a simple mbedtls_ssl_handshake then my acornssl client works taskwindow to taskwindow server.

I have thought of a need for the ENOTCONN error separate from EWOULDBLOCK. ssl_read/write returns ENOTCONN every handshake step. If the first action after connecting is ssl_read it does the handshake and returns ENOTCONN after every step of the handshake until the handshake completes. Now if you are relying on the internet event to continue a read it may not work properly during handshaking as some of the steps may not be waiting for a read so I think you need to poll if the error is ENOTCONN when using the internet event. I think a separate handshake swi would have been more explicit and left reads/writes to work like socket read writes.

If you included handshake_sslhandle in your handshake swi so that it can be used by the client then if the client uses it the handshaking will be over before ssl_read/write are called and handshake_sslhandle won’t get called by ssl_read/write and should behave like normal socket read/write

I found the bug in handshake_sslhandle. If you change this section at line 366

	else
		{
			if ((mbederr != MBEDTLS_ERR_SSL_WANT_READ) &&
			    (mbederr != MBEDTLS_ERR_SSL_WANT_WRITE))
			{
				dprintf(("api", "ssl_handshake_step returned %d\n", mbederr));
				err = intl_error(ERR07_HANDSHAKE_ERROR, intl_integer(-mbederr));
				if (mbederr == MBEDTLS_ERR_NET_CONN_RESET)
				{
					/* Fatal, map this to equivalent Unix error */
					dprintf(("api", "connection terminated\n"));
					REALLYSETDCI4ERRNO(err->errnum, ECONNRESET);
				}
				return err;
			}

			/* Not fatal, try again for some more */
			usermode_donothing();
		}

to

	else
		{
			if ((mbederr != MBEDTLS_ERR_SSL_WANT_READ) &&
			    (mbederr != MBEDTLS_ERR_SSL_WANT_WRITE))
			{
				dprintf(("api", "ssl_handshake_step returned %d\n", mbederr));
				err = intl_error(ERR07_HANDSHAKE_ERROR, intl_integer(-mbederr));
				if (mbederr == MBEDTLS_ERR_NET_CONN_RESET)
				{
					/* Fatal, map this to equivalent Unix error */
					dprintf(("api", "connection terminated\n"));
					REALLYSETDCI4ERRNO(err->errnum, ECONNRESET);
				}
				return err;
			}
			else
			{
				/*
				 * Convert WANT_READ and WANT_WRITE to ENOTCONN to.
				 * signify the handshake is not over.
				 * (same error code as mbederr == 0 when handshake isn't over).
				 */
				err = intl_error(ERR07_HANDSHAKE_ERROR, intl_integer(-mbederr));
				REALLYSETDCI4ERRNO(err->errnum, ENOTCONN);
				return err;
			}

			/* Not fatal, try again for some more */
			usermode_donothing();
		}

then it works fine – it was treating WANT_READ AND WANT_WRITE as finished. I just added the else part. I think it should also work with your server version without using the handshake swi. Your handshake swi should just call handshake_handle.

Thanks, Colin. I’ll look at that a bit later, because I’m struggling with something else at the moment.

Maybe you’ve noticed that the call to AcornSSL_Bind has to be made with protocol = 0 in order for a TCP socket to be created. The correct value, for compatibility with the rest of RISC OS’s sockets stuff, should be 1. The discrepancy is between the definitions used by mbedTLS (MBEDTLS_NET_PROTO_TCP = 0, MBEDTLS_NET_PROTO_UDP = 1) and RISC OS (SOCK_STREAM = 1, SOCK_DGRAM = 2). I need to subtract 1 before I call mbedtls_net_bind.

However, while investigating (and being sidetracked by creating socket 0, which I mistakenly thought was wrong!), I’ve seen something that looks wrong to me, and I’d like verdicts from more experienced sockets users. There are calls within mbedTLS.c.net_sockets to socket(-,-,-) where the third argument is either IPPROTO_UDP or IPPROTO_TCP, whereas the examples in PRM 5a and all the sockets code I’ve ever written use 0. What’s right, and why the discrepancy?

Maybe you’ve noticed that the call to AcornSSL_Bind has to be made with protocol = 0

That’s the problem with using the mbedtls interface in the AcornSSL api. There are subtle differences and if you make the interface look like mbedtls but require normal socket values to be used it is a source of confusion. If the api looks like sockets then the values are as expected.

I wouldn’t do arithmetic on the values just map one to the other.

the third argument is either IPPROTO_UDP or IPPROTO_TCP

I think the protocol is checked against tables of supported domain,type,protocol and a protocol of 0 means match any protocol. The protocol for PF_INET, SOCK_STREAM is IPPROTO_TCP and similarly PF_INET, SOCK_DGRAM is IPPROTO_UDP

I just tried to use the gitlab web IDE to make a one-line change to c.net_sockets (to add a missing initialisation), but I find I’m not allowed to. The file is part of Lib.mbedTLS, but the file I’m trying to modify is a RISC OS specific file, i.e. it’s not one that comes from ARM.

I think you’ll need to fork the project and then (using the web IDE) make the edit in your fork.

OK, thanks, Jeffrey. I’ve committed it and requested a merge. Assignee and reviewer are “ROOL”.

I await notification of what I’ve done wrong this time. :-(

Oh, and I’ve credited Colin with finding it.

Colin and I are discussing (by priv-mail) the possibility of incorporating the sort of functionality that Socketwatch provides, into AcornSSL, with some significant improvements. This would make it possible to do everything without knowing any socket handle.

Incorporating SocketWatch functionality seems an excellent idea to me!

Indeed, excellent.

You don’t need to know how to put the engine together to drive a car.