Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: add information about s2n-tls software architecture #4868

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

boquan-fang
Copy link
Contributor

Resolved issues:

The usage guide doesn't have documentations for general s2n-tls software architecture.

Description of changes:

I have updated our usage-guide to include documentation about s2n-tls high level software architecture. Since the architecture mostly involves s2n_connection and s2n_config, I decided to create a new markdown file for software architecture before the connection usage guide. Therefore, I need to rename those documentations after the connection usage guide.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

* add software architecture md file
* insert the file before connection doc
@@ -0,0 +1,21 @@
# Software Architecture

User interaction with s2n-tls happens primarily through the `s2n_connection` and `s2n_config` structures.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure you currently have enough new information for this to be an entirely new chapter. Is there a way you can work this information into the existing chapters? Maybe connection.md because it's one of the first chapters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One other way that I think would work is to combine the software architecture, connection, and config chapters together and name it Software Architecture. I think both connection and config are parts of the software architecture. I don't think adding general software architecture documentation to connection.md alone is appropriate. Hence, I would say either we use three chapters to describe software architecture, or we combine all three chapters into one. I prefer the former, since it makes each chapter to serve one purpose. It also follows a logical flow where we discussed a general concept, and then gets down to the specific about connection and config.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Madeleine - I don't see a reason for an entirely new section for this. It seems like most of the new content can be added to either the connection or config sections.

@@ -0,0 +1,21 @@
# Software Architecture

User interaction with s2n-tls happens primarily through the `s2n_connection` and `s2n_config` structures.
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Madeleine - I don't see a reason for an entirely new section for this. It seems like most of the new content can be added to either the connection or config sections.

docs/usage-guide/topics/ch04-software-architecture.md Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch04-software-architecture.md Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch04-software-architecture.md Outdated Show resolved Hide resolved
* move those general docs to config and connection chapters
* move config chapter in front of the connection chapter
docs/usage-guide/topics/SUMMARY.md Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch04-config.md Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch04-config.md Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch04-config.md Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch05-connection.md Outdated Show resolved Hide resolved
* remove some repeating sentences
* rephrase some sentences to make them read more precise
docs/usage-guide/topics/ch04-connection.md Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch05-config.md Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch05-config.md Outdated Show resolved Hide resolved
* make doc wording more precise based on comments
* add software architecture information on top of the connection chapter
* resolve a nit
Comment on lines 3 to 4
Users interact with s2n-tls primarily through the `s2n_connection` and `s2n_config` structures. Users should build a `s2n_config` object and `s2n_connection` objects, associate the config with those connections, and then calling `s2n_negotiate()` on those connections until the TLS handshakes are completed. Users should then call `s2n_send`/`s2n_recv` on those connections to send and receive application data.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not really sure what the goal of this paragraph is or that its inclusion makes the documentation better. This section is on connections, and technically you don't have to create a config to use a connection. I'm also not sure how useful the very brief mention of negotiate and send/receive is-- if we're going to mention them, we should probably at least link to the usage guide section that goes into more details.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure what the goal of this paragraph is or that its inclusion makes the documentation better.

The initial intent of this paragraph is to provide a general overview of S2N software architecture. I think such a high level overview does make the documentation better, since it provides a general idea of how S2N-TLS usually works. It also connects the following chapters together. This is also suggested by this comment.

technically you don't have to create a config to use a connection.

I would change the wording as In general, users build a s2n_config ... to indicate how S2N is generally configured. That change implies there can be exceptions.

I'm also not sure how useful the very brief mention of negotiate and send/receive is-- if we're going to mention them, we should probably at least link to the usage guide section that goes into more details.

I think the process of negotiation and send/receive is a part of how users would typically interact with S2N-TLS. I would keep this part. I agree that we need to link chapter 7 - sending and receiving to this part.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that paragraph achieves that goal, and currently isn't particularly helpful. Right now, I look in the "connection" section and before even explaining what a connection is or what it's for, you're redirecting me to configs and IO methods.

You may need to think about reorganizing things. Introduce what a connection is, then direct customers to the config section for configuration options and the IO section for how to use IO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You may need to think about reorganizing things. Introduce what a connection is, then direct customers to the config section for configuration options and the IO section for how to use IO.

I want to set up a subsection under # TLS Connections called ## Software Architecture, and move the paragraph into that subsection. Hence, the s2n_connection introduction will go before the general overview of software architecture.

Moreover, ## Connection Memory subsection talks about freeing memory after handshake.

A connection struct has memory allocated specifically for the TLS handshake. Memory-constrained users can free that memory by calling `s2n_connection_free_handshake()` after the handshake is successfully negotiated. Note that the handshake memory can be reused for another connection if `s2n_connection_wipe()` is called, so freeing it may result in more memory allocations later. Additionally some functions that print information about the handshake may not produce meaningful results after the handshake memory is freed.

Hence, introducing the software architecture after # TLS Connections and right before ## Connection Memory should helps the doc to explain when the sequence of events usually take place.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not convinced that "Software Architecture" really makes sense as its own section. There's not much to talk about other than connections with software architecture :P

I'd go for:

  1. what is a connection
  2. how is a connection used, high level with links to details
  3. how do connections interact with configs, high level with links to details

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wrote a general introduction about TLS connections and configs, which includes answers to those questions. We can revise it based on the new commit.

docs/usage-guide/topics/ch11-resumption.md Outdated Show resolved Hide resolved
docs/usage-guide/topics/ch05-config.md Outdated Show resolved Hide resolved
* remove repetitive sentences, and make wording more precise.
* link relevant sections to software architecture's overview.
* change wording
* move software architecture section to proper position
* give general introductions about connection and config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants