<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div><blockquote type="cite" class=""><div class="">On Mar 20, 2017, at 1:04 PM, Gelareh Taban via swift-server-dev <<a href="mailto:swift-server-dev@swift.org" class="">swift-server-dev@swift.org</a>> wrote:</div><div class=""><div class=""><p class=""><b class=""></b></p></div></div></blockquote><div><blockquote type="cite" class=""><b class="">4.1 - TLS service protocol</b><br class=""><br class=""><font size="2" class="">The TLS service protocol describes the methods that the transport layer calls to handle transport-level events for the TLS service object.</font><br class=""></blockquote><br class=""></div><div>I have a bunch of questions about the design you're presenting, and I think many of them ultimately stem from not understanding some of the high-level aspects of the proposal. For instance:</div><div><br class=""></div><div>* What types conform to this protocol? From the diagram, it looks like there's a type for each "engine"—a SecureTransportService, an OpenSSLService, etc.—and each instance represents a particular configuration of that engine. So you create a WhateverTLSService, configure it, and then hand it off (through several layers) to the transport management layer, which calls methods on it to handle various events. The transport management layer then uses that one TLSService to handle many connections. Is that correct?</div><div><br class=""></div><div>* What is the lifecycle of a connection? Does the TLSService create them itself, does the transport management layer create them and hand them off, or does the transport management layer retain control over them from beginning to end?</div><div><br class=""></div><div>* You discuss the higher layers creating a TLSService object and caching it in a property, then ultimately handing it down to the transport management layer, which then attaches it to socket objects. But presumably you can have many socket objects, possibly simultaneously. Are they all served by a single TLSService instance, or by many? If they share a TLSService, how does the TLSService know which socket is talking to it at a given moment? If they have separate ones, how does the transport management layer acquire a new one when it needs it?</div><div><br class=""></div><div>You mention that this proposal is very small in scope, and it's fine to describe some of these details in general ways. For instance, you don't need to describe the interface to a Swift socket or the transport layer in detail. But currently, the description of these surrounding systems is *so* vague that I'm struggling to assess this design.</div><div><br class=""></div><div>Perhaps some of these details have been described in other documents or meetings; if so, they really need to be presented in this document, too.</div><blockquote type="cite" class=""><div class=""><div class=""><p class=""><b class=""><font size="2" class="">- onClientCreate</font></b><br class=""></p></div></div></blockquote><div>Why do these methods all have "on" prefixes? I'm not totally sure I understand the intended usage here, but I see two possibilities:</div><div><br class=""></div><div>* These are imperative commands. `onAccept` says that the TLS engine should accept a connection, `onSend` means it should send some data, etc. In that case, these should not have any prefix—they should just be `accept`, `send`, etc.</div><div><br class=""></div><div>* These are essentially delegate methods notifying the TLS engine of an event. `onAccept` says that the system has accepted a connection and the TLS engine should do what it needs to do with it, `onSend` means the system is about to send data and it needs the TLS engine to modify it, etc. If so, Swift APIs more often use words like `should`, `will`, or `did` than `on`, particularly since they're more precise about the timing of the delegate method compared to the actual event.</div><div><br class=""></div><div>In either case, I don't think "on" is the best naming for these. It needlessly bucks platform conventions.</div><blockquote type="cite" class=""><div class=""><div class=""><p class=""><font size="2" class="">This will be called when a client I/O connection is created and appropriate TLS connection needs to be configured, including context creation, the handshake and connection verification.</font><br class=""><br class=""><font size="2" class="">This is a client only method.</font><br class=""><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> /// Setup the contexts and process the TLSService configurations (certificates, etc)</font><br class=""><font size="2" class=""> ///</font><br class=""><br class=""><font size="2" class=""> func onClientCreate() throws</font><br class=""><b class=""></b></p></div></div></blockquote><blockquote type="cite" class=""><div class=""><div class=""><p class=""><b class=""><font size="2" class="">- onServerCreate</font></b><br class=""><br class=""><font size="2" class="">This will be called when a server I/O connection is created and appropriate TLS connection needs to be setup, including context creation, the handshake and connection verification.</font><br class=""><br class=""><font size="2" class="">This is a server only method.</font><br class=""><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> /// Setup the contexts and process the TLSService configurations (certificates, etc)</font><br class=""><font size="2" class=""> ///</font><br class=""><br class=""><font size="2" class=""> func onServerCreate() throws</font><br class=""><b class=""></b></p></div></div></blockquote><div>What are these methods supposed to do, exactly?</div><div><br class=""></div><div>* Do they put `self` into either a client state or a server state? If so, what happens if you call both, or neither, or call one twice? Would it be better to do this as part of initialization, or to have them make a client TLS object or server TLS object, or to require whatever code hands the TLSService to the TransportManager to pre-configure it as either client or server?</div><div><br class=""></div><div>* Do they create a new instance that's either a client or a server? If so, how do they return it?</div><div><br class=""></div><div>* Do they configure something recently created as either client or server? If so, how do they access whatever they need to configure?</div><div><br class=""></div><div>Basically, what state are these supposed to operate upon?</div><blockquote type="cite" class=""><div class=""><div class=""><p class=""><b class=""><font size="2" class="">- onDestroy</font></b><br class=""><br class=""><font size="2" class="">This will be called when an I/O instance connection is closed and any remaining TLS context needs to be destroyed.</font><br class=""><br class=""><font size="2" class="">This is both a client and server method.</font><br class=""><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> /// Destroy any remaining contexts </font><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> func onDestroy()</font><br class=""></p></div></div></blockquote>Is this called at the end of each connection, or is it called once when the transport management layer is totally finished with the TLSService, or are these the same thing?</div><div><br class=""></div><div>If per-connection, how does it know which connection?</div><div><br class=""></div><div>If during destruction, should we just class-constrain and use `deinit` for this purpose?<br class=""><blockquote type="cite" class=""><div class=""><div class=""><p class=""><b class=""><font size="2" class="">- onAccept</font></b><br class=""><br class=""><font size="2" class="">This will be called once an I/O instance connection has been accepted, to setup the TLS connection, do the handshake and connection verification.</font><br class=""><br class=""><font size="2" class="">This is both a client and server method.</font><br class=""><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> /// Processing on acceptance from a listening connection</font><br class=""><font size="2" class=""> /// </font><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> /// - Parameter IORef: The connected I/O instance</font><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> func onAccept(IORef: TransportManagementDelegate) throws</font><br class=""><b class=""></b></p></div></div></blockquote>I take it the parameter is some sort of abstraction wrapping e.g. a socket. So why is it called a `TransportManagementDelegate`? Shouldn't its name include words like `Connection` or `Socket` or `IOHandle` or something?</div><div><br class=""></div><div>Do we want the parameter to be labeled `IORef`? That's not very idiomatic; it doesn't read well or follow the Swift naming guidelines.</div><div><br class=""></div><div>You say this is for both clients and servers. When does a TLS client have a listening connection that it `accept`s connections on?</div><div><br class=""></div><div>Is it called at different times or in different ways than `onServerCreate`?<br class=""><blockquote type="cite" class=""><div class=""><div class=""><p class=""><b class=""><font size="2" class="">- onConnect</font></b><br class=""><br class=""><font size="2" class="">This will be called once a socket connection has been made, to setup the TLS connection, do the handshake and connection verification.</font><br class=""><br class=""><font size="2" class="">This is both a client and server method.</font><br class=""><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> /// Processing on connection to a listening connection</font><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> /// - Parameter connectionRef: The connected I/O instance</font><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> func onConnect(IORef: TransportManagementDelegate) throws</font><br class=""><b class=""></b></p></div></div></blockquote><div>The same as above, with appropriate substitutions.</div><blockquote type="cite" class=""><div class=""><div class=""><p class=""><b class=""><font size="2" class="">- onSend</font></b><br class=""><br class=""><font size="2" class="">This will be called when data is to be written to an I/O instance. The input data buffer is written to the TLS connection associated with that I/O instance.</font><br class=""><br class=""><font size="2" class="">This is both a client and server method.</font><br class=""><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> /// Low level writer</font><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> /// - Parameters:</font><br class=""><font size="2" class=""> /// - buffer: Buffer pointer</font><br class=""><font size="2" class=""> /// - bufSize: Size of the buffer</font><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> /// - Returns the number of bytes written. Zero indicates TLS shutdown, less than zero indicates error.</font><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> func onSend(buffer: UnsafeRawPointer, bufSize: Int) throws -> Int</font><br class=""><b class=""></b></p></div></div></blockquote><div>Is there a reason you use an UnsafeRawPointer and a buffer size, instead of using an UnsafeRawBufferPointer which would encapsulate both?</div><div><br class=""></div><div>Why is shutdown indicated with zero, rather than the return value being Optional and being nil? Why are errors signaled with negative values instead of being thrown? (Or are you saying that negative returns are invalid? That's different from saying "indicates error".)</div><div><br class=""></div><div>If a TLSService return less than `bufSize`, will the enclosing later try to.re-send the remaining data in subsequent calls?</div><div><br class=""></div><div>This sounds like the TLS engine owns the network connection (at least by this point) and is responsible for writing to it. Does that mean `accept` and `connect` take ownership of the connection and hold on to it? If you have several different simultaneous connections, how do you know which connection this should write to? Or does a given TLSService only own one connection at a time? If so, does the transport management layer create a new TLSService instance for each connection? How? If each TLSService is bound to one connection, shouldn't it be created already knowing the connection it's going to use?</div><blockquote type="cite" class=""><div class=""><div class=""><p class=""><b class=""><font size="2" class="">- onReceive</font></b><br class=""><br class=""><font size="2" class="">This will be called when data is to be read from an I/O instance. Encrypted data is read from TLS connection associated with that I/O instance and decrypted and written to the buffer passed in.</font><br class=""><br class=""><font size="2" class="">This is both a client and server method.</font><br class=""><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> /// Low level reader</font><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> /// - Parameters:</font><br class=""><font size="2" class=""> /// - buffer: Buffer pointer</font><br class=""><font size="2" class=""> /// - bufSize: Size of the buffer</font><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> /// - Returns the number of bytes read. Zero indicates TLS shutdown, less than zero indicates error.</font><br class=""><font size="2" class=""> ///</font><br class=""><font size="2" class=""> func onReceive(buffer: UnsafeMutableRawPointer, bufSize: Int) throws -> Int</font><br class=""><b class=""></b></p></div></div></blockquote><div>If I understand correctly, `buffer` is an uninitialized memory region that the type should fill with data. Is that correct?</div><div><br class=""></div>Otherwise, the same as above, with appropriate substitutions.<br class=""><blockquote type="cite" class=""><div class=""><div class=""><p class=""><b class=""><font size="4" class="">5 - Non-goals</font></b><br class=""><br class=""><font size="2" class="">This proposal:</font><br class=""><br class=""><font size="2" class="">1- DOES NOT describe the TLS service configuration, which includes information on certificate types, formats and chains, cipher suites, etc. We expect this to be specified in a future proposal.</font><br class=""><br class=""><font size="2" class="">2- DOES NOT describe the TLS service trust policies, which define trust and validation policies of the incoming connection. We expect this to be specified in a future proposal.</font><br class=""><br class=""><font size="2" class="">3- DOES NOT describe the interface between the TLS service and the transport layer and any dependencies. We expect this to be specified in a future proposal.</font><br class=""><br class=""></p></div></div></blockquote><div>I feel like #3 in particular really hurts this proposal. It's impossible to evaluate this without at least a general idea of how the TLS service and the transport layer communicate. It's okay to handwave the details—for instance, you could say "Type X represents a network connection, and has methods to read, write, and close it", without describing those methods in detail—but without at least an overview of how this will be used, it's very difficult to evaluate.</div><div><br class=""></div><div>I think this is probably a good design that just isn't being explained very clearly. I hope you can clarify some of these points.</div></div><br class=""><div class="">
<span class="Apple-style-span" style="border-collapse: separate; font-variant-ligatures: normal; font-variant-east-asian: normal; font-variant-position: normal; line-height: normal; border-spacing: 0px;"><div class=""><div style="font-size: 12px; " class="">-- </div><div style="font-size: 12px; " class="">Brent Royal-Gordon</div><div style="font-size: 12px; " class="">Architechies</div></div></span>
</div>
<br class=""></body></html>