8 ms·

Secure Rust Guidelines

171 points by xmmrm 7y ago

maeln 7y ago
This is published by the ANSSI[1], National Cybersecurity Agency of France. They do quite a good job at publishing security guide. [1] https://www.ssi.gouv.fr/en/ https://www.ssi.gouv.fr/en/
imjasonmiller 7y ago
The context that I was missing at first: > The Agence nationale de la sécurité des systèmes d'information (ANSSI; English: National Cybersecurity Agency of France) is a French service created on 7 July 2009 with responsibility for computer security [1]. I didn't know about "cargo-outdated". I liked having "npm outdated" within the JavaScript ecosystem, so I'll give this a try. 1. https://en.wikipedia.org/wiki/Agence_nationale_de_la_s%C3%A9curit%C3%A9_des_syst%C3%A8mes_d%27information https://en.wikipedia.org/wiki/Agence_nationale_de_la_s%C3%A9...
- jamwaffles 7y ago
  `cargo-outdated` is mint. If you haven't already, consider installing `cargo-edit` too! It adds `cargo add`/`cargo rm` to add/remove crates, as well as `cargo upgrade` to bump semver versions. https://github.com/killercup/cargo-edit https://github.com/killercup/cargo-edit
  - masklinn 7y ago
    There's also cargo-crev which is pretty neat.
CJefferson 7y ago
I personally strongly disagree with: Functions or instructions that can cause the code to panic at runtime must not be used. First of all, they kind of dodge this later by saying "Array indexing must be properly tested" (else it can panic) -- everyone thinks they write code which is "properly tested". Personally, I often write panicing code -- if the code gets in a state where I have no idea how to fix it, I panic. For some code it is important it does quit, but I'd much prefer more code paniced than tried to carry on and ended up creating other problems. Also, in rust if we don't want to panic we need to never use array indexing and never use integer division, just to start. Of course, it's fine to write code which plans to never panic, but I think it would be better to say "Have a formal 'panic plan'", where either one panics early, or tries to never panic.
- masklinn 7y ago
  > First of all, they kind of dodge this later by saying "Array indexing must be properly tested" (else it can panic) -- everyone thinks they write code which is "properly tested". You're skipping half the recommendation though: > Array indexing must be properly tested, or the get method should be used to return an Option. emphasis mine > Also, in rust if we don't want to panic we need to never use array indexing and never use integer division, just to start. I mean, that's literally the block above the one you quote: > Common patterns that can cause panics are: > * using unwrap or expect, > * using assert, > * an unchecked access to an array, > * integer overflow (in debug mode), > * division by zero, > * large allocations, > * string formatting using format!. >> Rule LANG-NOPANIC >> Functions or instructions that can cause the code to panic at runtime must not be used. The bit you quote is really a additional reminder that array indexing is not panic-safe.
  - heavenlyblue 7y ago
    What do you do when your algorithm works with array indexing and you are never supposed to handle than None within the Option? Ie if you have indexed array out of bounds that’s a developer’s mistake and not a “condition to be handled up the stack”. It’s ok to panic if you agree that it’s better to panic rather than end up in a state that will probably cause even more issues down the line
    - masklinn 7y ago
      > What do you do when your algorithm works with array indexing and you are never supposed to handle than None within the Option? That's where the first clause comes into play: > Array indexing must be properly tested Although > Ie if you have indexed array out of bounds that’s a developer’s mistake Which developer would be my question. If it's the person who reified the "algorithm [which] works with array indexing" then fair's fair, it's a bug in the implementation and should not happen, which is why the guideline specifically says: > should never use functions or instructions that can fail and cause the code to panic which, assuming the word is used in the RFC 2119 sense means there can be case where calling panic-ing functions is the right thing to do. If it's the developer who calls the function implementing the algorithm, then either algorithm is failable (and thus so's the function) or the algorithm should take in more specific data structures which statically exclude failing cases. For instance `max` on an empty collection will fail, so either `max` should be failable (which e.g. `Iterator::max` is) or it should only be callable on some sort of `NonEmpty*`.
deleted 7y ago
[deleted]
gardaani 7y ago
I'm having problems fulfilling this requirement in my libs: "Crates providing libraries should never use functions or instructions that can fail and cause the code to panic." The Rust standard library Vec, HashMap etc. can cause a panic in Rust, if the device (such as a mobile phone with a small memory) runs out of memory. C and C++ standard libraries (malloc, std::vector, std::map..) can handle those situations by returning null or throwing an exception. I wish Rust had some easy way to recover from out-of-memory situations when using the standard library. I have been considering writing my own out-of-memory safe Vec, HashMap etc, but it can't be the right way to do it..
- masklinn 7y ago
  Yes that is one of the primary failures of Rust at the moment: to my knowledge it currently has no good way to safely manage allocation failures (it also has serious issues with stack overflows). This is an issue with all heap-allocating construct, not just collections but also Box or Rc. > I wish Rust had some easy way to recover from out-of-memory situations when using the standard library. I have been considering writing my own out-of-memory safe Vec, HashMap etc, but it can't be the right way to do it.. Maybe look at the embedded space there? There might be no_std third-party libraries which handle these issues. Possibly on top of alloc as the (unstable[0] and obviously unsafe) `Alloc` trait does have a concept of allocation failure. [0] https://github.com/rust-lang/rust/issues/32838 https://github.com/rust-lang/rust/issues/32838
  - gardaani 7y ago
    Actually, I have found FallibleVec written by Mozilla [1] but I haven't found anything for other containers, yet. [1] https://github.com/mozilla/mp4parse_fallible https://github.com/mozilla/mp4parse_fallible
  - steveklabnik 7y ago
    > Yes that is one of the primary failures of Rust at the moment: to my knowledge it currently has no good way to safely manage allocation failures So, sort of yes and sort of no. The data structures that allocate in the standard library do not let you handle allocation failure. However, if you write your own, the global allocator lets you determine if failure happened, and then you can do whatever you want with it.
dathinab 7y ago
I found at least one problematic section when scanning: > The environment variables RUSTC, RUSTC_WRAPPER and RUSTFLAGS must not be overriden when using Cargo to build project. This is simply not true at all. Mainly build cache systems like sccache work by wrapping rustc, there is no reason why using a build cache should be forbidden. (Through currently rust support of sccache is still not perfect and there are some limitations, doesn't change that the rules are to broad.) I also wouldn't be surprised if there are some rustc flags not exposed in cargo profiles which allow trigger some security mechanisms in llvm which are not enabled by default but beneficial for your project. Like always the important think is that you understand what you do and want implications it had.

171 points by xmmrm 7y ago

This is published by the ANSSI[1], National Cybersecurity Agency of France. They do quite a good job at publishing security guide. [1] https://www.ssi.gouv.fr/en/ https://www.ssi.gouv.fr/en/

imjasonmiller 7y ago

The context that I was missing at first: > The Agence nationale de la sécurité des systèmes d'information (ANSSI; English: National Cybersecurity Agency of France) is a French service created on 7 July 2009 with responsibility for computer security [1]. I didn't know about "cargo-outdated". I liked having "npm outdated" within the JavaScript ecosystem, so I'll give this a try. 1. https://en.wikipedia.org/wiki/Agence_nationale_de_la_s%C3%A9curit%C3%A9_des_syst%C3%A8mes_d%27information https://en.wikipedia.org/wiki/Agence_nationale_de_la_s%C3%A9...

jamwaffles 7y ago
`cargo-outdated` is mint. If you haven't already, consider installing `cargo-edit` too! It adds `cargo add`/`cargo rm` to add/remove crates, as well as `cargo upgrade` to bump semver versions. https://github.com/killercup/cargo-edit https://github.com/killercup/cargo-edit
- masklinn 7y ago
  There's also cargo-crev which is pretty neat.

CJefferson 7y ago

I personally strongly disagree with: Functions or instructions that can cause the code to panic at runtime must not be used. First of all, they kind of dodge this later by saying "Array indexing must be properly tested" (else it can panic) -- everyone thinks they write code which is "properly tested". Personally, I often write panicing code -- if the code gets in a state where I have no idea how to fix it, I panic. For some code it is important it does quit, but I'd much prefer more code paniced than tried to carry on and ended up creating other problems. Also, in rust if we don't want to panic we need to never use array indexing and never use integer division, just to start. Of course, it's fine to write code which plans to never panic, but I think it would be better to say "Have a formal 'panic plan'", where either one panics early, or tries to never panic.

masklinn 7y ago
> First of all, they kind of dodge this later by saying "Array indexing must be properly tested" (else it can panic) -- everyone thinks they write code which is "properly tested". You're skipping half the recommendation though: > Array indexing must be properly tested, or the get method should be used to return an Option. emphasis mine > Also, in rust if we don't want to panic we need to never use array indexing and never use integer division, just to start. I mean, that's literally the block above the one you quote: > Common patterns that can cause panics are: > * using unwrap or expect, > * using assert, > * an unchecked access to an array, > * integer overflow (in debug mode), > * division by zero, > * large allocations, > * string formatting using format!. >> Rule LANG-NOPANIC >> Functions or instructions that can cause the code to panic at runtime must not be used. The bit you quote is really a additional reminder that array indexing is not panic-safe.
- heavenlyblue 7y ago
  What do you do when your algorithm works with array indexing and you are never supposed to handle than None within the Option? Ie if you have indexed array out of bounds that’s a developer’s mistake and not a “condition to be handled up the stack”. It’s ok to panic if you agree that it’s better to panic rather than end up in a state that will probably cause even more issues down the line
  - masklinn 7y ago
    > What do you do when your algorithm works with array indexing and you are never supposed to handle than None within the Option? That's where the first clause comes into play: > Array indexing must be properly tested Although > Ie if you have indexed array out of bounds that’s a developer’s mistake Which developer would be my question. If it's the person who reified the "algorithm [which] works with array indexing" then fair's fair, it's a bug in the implementation and should not happen, which is why the guideline specifically says: > should never use functions or instructions that can fail and cause the code to panic which, assuming the word is used in the RFC 2119 sense means there can be case where calling panic-ing functions is the right thing to do. If it's the developer who calls the function implementing the algorithm, then either algorithm is failable (and thus so's the function) or the algorithm should take in more specific data structures which statically exclude failing cases. For instance `max` on an empty collection will fail, so either `max` should be failable (which e.g. `Iterator::max` is) or it should only be callable on some sort of `NonEmpty*`.

deleted 7y ago

[deleted]

gardaani 7y ago

I'm having problems fulfilling this requirement in my libs: "Crates providing libraries should never use functions or instructions that can fail and cause the code to panic." The Rust standard library Vec, HashMap etc. can cause a panic in Rust, if the device (such as a mobile phone with a small memory) runs out of memory. C and C++ standard libraries (malloc, std::vector, std::map..) can handle those situations by returning null or throwing an exception. I wish Rust had some easy way to recover from out-of-memory situations when using the standard library. I have been considering writing my own out-of-memory safe Vec, HashMap etc, but it can't be the right way to do it..

masklinn 7y ago
Yes that is one of the primary failures of Rust at the moment: to my knowledge it currently has no good way to safely manage allocation failures (it also has serious issues with stack overflows). This is an issue with all heap-allocating construct, not just collections but also Box or Rc. > I wish Rust had some easy way to recover from out-of-memory situations when using the standard library. I have been considering writing my own out-of-memory safe Vec, HashMap etc, but it can't be the right way to do it.. Maybe look at the embedded space there? There might be no_std third-party libraries which handle these issues. Possibly on top of alloc as the (unstable[0] and obviously unsafe) `Alloc` trait does have a concept of allocation failure. [0] https://github.com/rust-lang/rust/issues/32838 https://github.com/rust-lang/rust/issues/32838
- gardaani 7y ago
  Actually, I have found FallibleVec written by Mozilla [1] but I haven't found anything for other containers, yet. [1] https://github.com/mozilla/mp4parse_fallible https://github.com/mozilla/mp4parse_fallible
- steveklabnik 7y ago
  > Yes that is one of the primary failures of Rust at the moment: to my knowledge it currently has no good way to safely manage allocation failures So, sort of yes and sort of no. The data structures that allocate in the standard library do not let you handle allocation failure. However, if you write your own, the global allocator lets you determine if failure happened, and then you can do whatever you want with it.

dathinab 7y ago

I found at least one problematic section when scanning: > The environment variables RUSTC, RUSTC_WRAPPER and RUSTFLAGS must not be overriden when using Cargo to build project. This is simply not true at all. Mainly build cache systems like sccache work by wrapping rustc, there is no reason why using a build cache should be forbidden. (Through currently rust support of sccache is still not perfect and there are some limitations, doesn't change that the rules are to broad.) I also wouldn't be surprised if there are some rustc flags not exposed in cargo profiles which allow trigger some security mechanisms in llvm which are not enabled by default but beneficial for your project. Like always the important think is that you understand what you do and want implications it had.