docs: add more warnings about security policy defaults #4507
Conversation
lrstewart
changed the title
lrstewart
requested review from
maddeleine,
camshaft and
jmayclin
and removed request for
camshaft
bindings/rust/s2n-tls/src/config.rs
Outdated
| /// | ||
| /// # Warning | ||
| /// | ||
| /// By default, the newly created Config uses the default security policy. |
There was a problem hiding this comment.
| /// By default, the newly created Config uses the default security policy. | |
| /// By default, the newly created Config uses the default security policy. This policy changes across library versions and could break connections. |
Should this short version of the warning mention that the policy changes? Unless I already knew about the default policy/had read the other documentation, I probably wouldn't understand why this was a warning.
There was a problem hiding this comment.
Idk, the changing isn't the only issue. The default policy might also just not have the options they're expecting / wanting. Like, at the moment it still has 1.0 and most customers probably don't want 1.0.
I was hoping I just needed to point out policies even exist so customers might read the other documentation :/
There was a problem hiding this comment.
Ya, if we think the default is going to remain stuck at 1.0, then I wonder if it would be worth exploring the option of totally removing the default from the bindings?
But still think we should merge this PR, since the new warning is definitely better than what we have now 😄
bindings/rust/s2n-tls/src/config.rs
Outdated
| /// # Warning | ||
| /// | ||
| /// The newly created Config will use the default security policy. | ||
| /// Consider changing this depending on your security and availability requirements |
There was a problem hiding this comment.
Is "compatibility" maybe more accurate here or availability what is intended?
| /// Consider changing this depending on your security and availability requirements | |
| /// Consider changing this depending on your security and compatibility requirements |
Description of changes:
Users of the Rust bindings don't necessarily read our usage guide, which is primarily focused on the C library.
I added safety notes about security policies to the Rust bindings so that users are at least aware that security policies exist.
I also tried to make the existing usage guide information on default policies clearer.
Callouts
Anywhere else I should put the warning?
Testing:
I ran cargo doc. We have existing warnings, but none about my new comments.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.