Stream: wg-secure-code

Topic: safety-dance


Shnatsel (Jul 22 2019 at 00:45, on Zulip):

I gave up on naming https://github.com/rust-secure-code/safety-dance and instead started opening issues about crates we want to audit, as well as the ones audited already.
Output of cargo-geiger on reqwest crate is horrifying - it's mostly red, with smallvec, arrayvec, slab and even custom locking primitives with a total of 3 stars on github. I started opening issues just for transitive dependencies of reqwest but had to stop short. So if you ever need more crates to look at, just sift through transitive dependencies of reqwest and open issues on that repo.

Alex Gaynor (Jul 22 2019 at 00:49, on Zulip):

Are you making notes on unsafe patterns that could be done safely -- I'm hopeful we can turn some of these into clippy lints

simulacrum (Jul 22 2019 at 00:51, on Zulip):

@Shnatsel I would ~strongly recommend sticking a license file (even if it's just MIT or something) in the root by the way

Shnatsel (Jul 22 2019 at 00:53, on Zulip):

Oh yeah, good point. @Tony Arcieri could you put a license on the repo and also provide a source / proper credits for the image?

Shnatsel (Jul 22 2019 at 00:55, on Zulip):

Re: notes on patterns: sort of. I guess I should start putting these in the repo itself.
Mostly I'm noticing missing safe abstractions, documented here: https://github.com/rust-secure-code/safety-dance/issues/1#issuecomment-513589145
But there is one cool pattern I've been shown recently: https://github.com/sile/libflate/pull/39/files
Used to be set_len(), then I've opened a PR that started zero-initializing the slice, and then someone else showed me this trick with writing to a vector from a Read impl but still making it bounded

Shnatsel (Jul 22 2019 at 00:59, on Zulip):

Passing a buffer of uninitialized data to read_exact() is very common apparently

Alex Gaynor (Jul 22 2019 at 01:03, on Zulip):

Seems bad -- io::Read's docs say you can't do that

Shnatsel (Jul 22 2019 at 01:05, on Zulip):

Yeah, docs started advising against that just recently, after I complained to RalfJung about that

Shnatsel (Jul 22 2019 at 01:05, on Zulip):

I'm pretty sure reqwest still does it

Tony Arcieri (Jul 22 2019 at 13:55, on Zulip):

re: the image, I know someone who knows someone who knows the artist. can try to vicariously get permission to use it. they don't seem to have social media presence and I don't know their contact details

Tony Arcieri (Jul 22 2019 at 14:20, on Zulip):

I am attempting to (vicariously) ask the artist for permission. If I can't get it I'll remove the image.

Tony Arcieri (Jul 22 2019 at 14:20, on Zulip):

re: licensing, should we just do the standard Apache-2.0 OR MIT?

simulacrum (Jul 22 2019 at 14:20, on Zulip):

sure, or even simply MIT I think would be fine

simulacrum (Jul 22 2019 at 14:20, on Zulip):

_a_ license is the important bit here :)

Shnatsel (Jul 22 2019 at 18:19, on Zulip):

Yeah I don't particularly care about the license either. Apache+MIT sounds good.

Tony Arcieri (Jul 22 2019 at 18:43, on Zulip):

https://github.com/rust-secure-code/safety-dance/pull/10/files

Shnatsel (Jul 22 2019 at 18:46, on Zulip):

Approved. Fire when ready.

Tony Arcieri (Jul 22 2019 at 18:47, on Zulip):

:rocket:

Shnatsel (Jul 22 2019 at 18:58, on Zulip):

Incoming! https://github.com/rust-secure-code/safety-dance/pull/11

Shnatsel (Jul 22 2019 at 19:09, on Zulip):

This effort is now weirdly split between this WG and people hanging out in #black-magic on community Discord

Tony Arcieri (Jul 23 2019 at 00:06, on Zulip):

regarding the Safety Dance logo, I have it on the word of a Rust core team member who contacted the original artist that it is in the public domain

Tony Arcieri (Jul 23 2019 at 00:07, on Zulip):

@Shnatsel in which case, do you think it's ready to tweet? or were you planning on doing a blog post or something first?

Tom Phinney (Jul 23 2019 at 00:39, on Zulip):

Yay! Love the Safety Dance logo. You are all free to use my analogy to dancing across hot coals in explaining why the "dance" metaphor is appropriate.

Tony Arcieri (Jul 23 2019 at 00:48, on Zulip):

haha

Tony Arcieri (Jul 23 2019 at 00:49, on Zulip):

@Shnatsel if you make a blog post, be sure to link to the video :wink: https://www.youtube.com/watch?v=AjPau5QYtYs

RalfJ (Jul 23 2019 at 09:25, on Zulip):

Seems bad -- io::Read's docs say you can't do that

that is, unless you know the Read impl that is being called, and made sure it does not and will not (in the future) read from buf

Tony Arcieri (Jul 23 2019 at 16:07, on Zulip):

welp :wink: https://twitter.com/rustsecurecode/status/1153698020724113409

Shnatsel (Jul 23 2019 at 17:23, on Zulip):

I'm not planning to make a blog post. Sadly I don't have the time, with being offline for a while and all that. I can help out again late August - early September.

Tony Arcieri (Jul 23 2019 at 18:40, on Zulip):

@Shnatsel no worries, I was just curious if I should wait to tweet the link to the blog post first, if there were one. but... too late! already just tweeted the repo, and @RustLang retweeted it, so I think we're good

Tony Arcieri (Jul 23 2019 at 18:40, on Zulip):

any thoughts on this? https://github.com/rust-secure-code/safety-dance/pull/15

Tony Arcieri (Jul 23 2019 at 18:40, on Zulip):

viral marketing! :sweat_smile:

Shnatsel (Jul 23 2019 at 19:11, on Zulip):

Sounds good to me

JP Sugarbroad (Jul 24 2019 at 18:37, on Zulip):

I've seen reference to a number of crates that help people avoid unsafe, like take_mut, owning_ref, and rental. Is safety-dance a good place to start cataloging them, like a "how to avoid unsafe" guide?

Tony Arcieri (Jul 24 2019 at 18:39, on Zulip):

sounds like a good idea to me. maybe put them in the README or perhaps a separate .md file linked from the README...

RalfJ (Jul 24 2019 at 18:54, on Zulip):

does this mean they have been reviewed carefully by someone who is not their primary author to make sure they are not unsound?

Tony Arcieri (Jul 24 2019 at 19:14, on Zulip):

perhaps we should open an issue to compile and discuss which ones should get included in a list

Florian Gilcher (Jul 25 2019 at 09:04, on Zulip):

Are there any qualifications you need for writing that blog post? I could find someone to write it for you and put it on the blog post.

/cc @Tony Arcieri @Shnatsel

Tony Arcieri (Jul 25 2019 at 14:37, on Zulip):

@Florian Gilcher not particularly, just enough background on what the project is and what its goals are to promote it

RalfJ (Aug 26 2019 at 09:21, on Zulip):

Are you guys aware of the list we (the UCG WG) are maintaining at https://github.com/rust-lang/unsafe-code-guidelines/issues/158 ? As part of safety-dance y'all are seeing a lot of real-world unsafe code out there, and I think (if that's something you'd like to do) it would be very helpful to use that to "cartograph" the less clear corners of the Rust unsafe code rules. Don't hesitate to open a thread in the UCG stream here on Zulip (#t-lang/wg-unsafe-code-guidelines), open an issue in the UCG repo or Cc me (@RalfJung on GH) when there are questions about whether some concrete piece of unsafe code is UB or not.

Shnatsel (Aug 31 2019 at 16:28, on Zulip):

I'm going to kick safety-dance into a higher gear by promoting it on Reddit. Before I invite more people to join I want to get the docs and processes up to scratch. Please check this out and let me know if it looks OK to you:
https://github.com/rust-secure-code/safety-dance/issues/20

Tony Arcieri (Aug 31 2019 at 16:33, on Zulip):

@Shnatsel just make sure you wait a little bit... Reddit is currently having a ton of issues due to the us-east-1 outage

Shnatsel (Aug 31 2019 at 16:34, on Zulip):

Good call. It's gonna take a while for me to write some docs anyway. Probably not gonna post until tomorrow

Shnatsel (Aug 31 2019 at 16:36, on Zulip):

The number of people subscribed to safety-dance and immediately responding to issues is already impressive

Shnatsel (Aug 31 2019 at 18:14, on Zulip):

Initial trophy case up: https://github.com/rust-secure-code/safety-dance/pull/23
Just the stuff I've been involved with for now, to establish the structure. Once I merge this everyone is encouraged to add their contributions!

Shnatsel (Aug 31 2019 at 18:22, on Zulip):

OK it's merged. Please open PRs for your contributions!

Shnatsel (Aug 31 2019 at 18:43, on Zulip):

Also my "new advisory" sense is tingling: https://github.com/image-rs/image/pull/985

Tony Arcieri (Aug 31 2019 at 18:57, on Zulip):

indeed

Shnatsel (Sep 01 2019 at 17:53, on Zulip):

I've opened a PR to update mission statement for safety-dance, please let me know if it makes sense or if it can be improved: https://github.com/rust-secure-code/safety-dance/pull/28/files

Shnatsel (Sep 06 2019 at 17:34, on Zulip):

Safety dance is getting so much attention that we're almost done with the crates we've picked for auditing already! We need some more popular crates to look at!

Shnatsel (Sep 06 2019 at 17:35, on Zulip):

And that's before I even started widely promoting it. It's never even been posted to Reddit

Shnatsel (Sep 06 2019 at 17:37, on Zulip):

What I really wanted to say is "Please throw some more crates at it!"

Thom Chiovoloni (Sep 06 2019 at 18:26, on Zulip):

the amount of unsafe relative to the complexity (and the fact that headers are such an easy thing for an attacker to poke at) always made me really worried about https://github.com/hyperium/http/blob/master/src/header/map.rs

Thom Chiovoloni (Sep 06 2019 at 18:27, on Zulip):

i can file an issue for it after lunch i guess, or someone else can

Thom Chiovoloni (Sep 06 2019 at 18:27, on Zulip):

that's most of the unsafe in that crate last time i looked, but it's... large, complex, and the unsafe seems to rely on a bunch of tricky invariants

Shnatsel (Sep 08 2019 at 14:03, on Zulip):

Everyone: looks like safety-dance is almost ready for wider promotion! We just need to pick more crates for auditing - most of what we have picked on the issue tracker is already partially or mostly done.

Shnatsel (Sep 08 2019 at 14:04, on Zulip):

Please add some important crates! (But preferably not async ones because those are under a lot of churn right now due to upcoming async/await)

Tony Arcieri (Sep 11 2019 at 19:32, on Zulip):

@Shnatsel if you're interested in doing a Safety Dance blog post, @nikomatsakis was talking about setting up a shared "Team Blog" where we could promote it

Tony Arcieri (Sep 11 2019 at 19:39, on Zulip):

https://github.com/rust-lang/blog.rust-lang.org/pull/402

Shnatsel (Sep 11 2019 at 22:59, on Zulip):

I do think we could use a WG blog to announce stuff like safety dance. Partly because I'm annoyed by the popups that Medium shows these days.
I'm not 100% confident it's a good idea to put it under rust-lang.org domain. It makes it a bit too official for my liking, too much responsibility.

Shnatsel (Sep 11 2019 at 23:02, on Zulip):

For example: I am mostly single-handedly driving safety-dance, and if I mess up I kinda want it to be just me who messes up, or a relatively obscure WG, and not the entire Rust org as a whole

Shnatsel (Sep 11 2019 at 23:03, on Zulip):

Or maybe I should just get more people to sanity check whatever I'm doing with safety dance, then we'll be fine

Tony Arcieri (Nov 01 2019 at 15:35, on Zulip):

" it would require a fixed-capacity Vec-like view of memory. I'll need to write an RFC for one at some point." @Shnatsel

Tony Arcieri (Nov 01 2019 at 15:35, on Zulip):

what does that mean exactly?

Tony Arcieri (Nov 01 2019 at 15:35, on Zulip):

does heapless::Vec work?

Shnatsel (Nov 01 2019 at 15:59, on Zulip):

Probably. There's a bunch of those around but no definitive implementation. Since it is also needed for a safer Read trait and the impl is very complex I'm pretty sure it needs to be in std.
Other known impls of this idea:
- https://crates.io/crates/buffer
- https://crates.io/crates/uninit
- https://crates.io/crates/static-alloc

Shnatsel (Nov 01 2019 at 16:01, on Zulip):

Actually, heapless::Vec will not work because it's always on the stack while we want to have a non-owning view of arbitrary slice of MaybeUninit<T>

Shnatsel (Nov 01 2019 at 16:34, on Zulip):

This abstraction really needs to be in std because we need multiple crates to agree on it. For example, flate2 would both pass it to miniz_oxide backend and accept such a view from client code, so we have a stack of 3 different crates passing it to each other

Tony Arcieri (Nov 01 2019 at 16:35, on Zulip):

does io::Cursor on a fixed-sized array work?

Shnatsel (Nov 01 2019 at 16:44, on Zulip):

Uuuh, sorta? I'm not sure if it's OK to have MaybeUninit<T> as the backing storage for Cursor.
And you would still need some unsafe to e.g. get the initialized portion as a slice, or apply changed length to Vec when the backing storage came from a Vec, but that might be possible to encapsulate. Still, doesn't sound very obvious or ergonomic to me.

Shnatsel (Nov 01 2019 at 19:06, on Zulip):

It's happening
https://www.reddit.com/r/rust/comments/dq8df4/announcing_safetydance_removing_unnecessary/

Tony Arcieri (Nov 01 2019 at 21:52, on Zulip):

nice post. seems to be garnering a decent amount of attention

Shnatsel (Nov 01 2019 at 22:33, on Zulip):

Yeah, seems to be working. After the flop of the 2019 goals blog post I was afraid I was losing my touch, but apparently not :sweat_smile:

Shnatsel (Nov 01 2019 at 23:41, on Zulip):

Yeah it's #1 link on Rust subreddit now :big_smile:

Shnatsel (Nov 02 2019 at 20:20, on Zulip):

By the way, people seem to like the safety-dance name and logo, so thanks to @Tony Arcieri for finding those!

Jacob Rosenthal (Nov 02 2019 at 23:32, on Zulip):

Heapless offers static
// in a static variable
// (because const-fn has not been fully stabilized you need to use the helper structs in
// the i module, which must be wrapped in a tuple struct)
static mut XS: Vec<u8, U8> = Vec(heapless::i::Vec::new());

Shnatsel (Nov 05 2019 at 19:25, on Zulip):

This is relevant to our interests: https://github.com/rust-lang/rfcs/pull/2802

Last update: Nov 11 2019 at 22:15UTC