Just like with the Gamepad code [1], we can't link against libudev on Linux. So we need a similar solution that uses dlopen() + dlsym() for the U2F Rust code. [1] http://searchfox.org/mozilla-central/source/dom/gamepad/linux/udev.h

I started with a fork of libudev-sys [1], that was generated by rust-bindgen, and then added the dlopen() and dlsym() code, just keeping the function signatures. Thus I wouldn't really call it a fork anymore... Let me know if you disagree. The version number is 0.1.3 because we have to override libudev-sys:0.1.3 as required by the libudev package. [1] https://github.com/dcuddeback/libudev-sys/blob/master/src/lib.rs

We should put a README in the folder explaining why this override is here, why it's named what it is, and what would happen if the udev package changes upstream. :)

Is this a review for a github repo or something? Or is this going in the tree somewhere?

Agh hit save changes too soon. Anyways, I realize it's in dom/webauthn, I just don't see build files or linkage into the rest of our system with it and I'm not quite sure what the plan is for it.

(In reply to Kyle Machulis [:qdot] [:kmachulis] (if a patch has no decent commit message, automatic r-) from comment #4) > Agh hit save changes too soon. Anyways, I realize it's in dom/webauthn, I > just don't see build files or linkage into the rest of our system with it > and I'm not quite sure what the plan is for it. Basically: There's enough code that we're going to land it / review it in chunks, and then tie it into the build system. This is pretty stand-alone, but it's also weird to make this compile w/o landing other parts too, so it's just going to be dead for a few days. At least, that's our intent.

(In reply to J.C. Jones [:jcj] from comment #2) > We should put a README in the folder explaining why this override is here, > why it's named what it is, and what would happen if the udev package changes > upstream. :) That's definitely a good idea, will do that.

(In reply to Kyle Machulis [:qdot] [:kmachulis] from comment #3) > Is this a review for a github repo or something? Or is this going in the > tree somewhere? I actually started this as a GitHub repo but have removed it since, it's going in the tree anyway. (In reply to Kyle Machulis [:qdot] [:kmachulis] from comment #4) > Agh hit save changes too soon. Anyways, I realize it's in dom/webauthn, I > just don't see build files or linkage into the rest of our system with it > and I'm not quite sure what the plan is for it. What J.C. said, we need this for our main Rust library which isn't quite ready to land yet, so this won't build until we actually land that too. I thought it would be a good idea to split the reviews and get that landed first.

(In reply to J.C. Jones [:jcj] from comment #5) > Basically: There's enough code that we're going to land it / review it in > chunks, and then tie it into the build system. This is pretty stand-alone, > but it's also weird to make this compile w/o landing other parts too, so > it's just going to be dead for a few days. At least, that's our intent. Hmm, ok. The problem is that I'm not really sure how this will tie into the rust side of our build system, and :ted, who I would usually bring in for that (as well as another set of eyes on these bindings, because he wrote the C bindings for gamepad), is on vacation until next Monday. Can these patches hang out a few days until I can make ni?/r? assignments to :ted?

Yeah, no problem at all. We're not in a rush to land this :)

Comment on attachment 8885649 [details] [diff] [review] 0001-Bug-1380270-Add-dlopen-version-of-libudev-sys.patch We need to late bind libudev-sys for the USB portions of USB U2F in rust. Ted, how do you want to deal with this in the rust build system?

(In reply to Tim Taubert [:ttaubert] from comment #9) > Yeah, no problem at all. We're not in a rush to land this :) We're not in a rush, per say, but I really do want to have this in place before August for external testing...

We're getting closer to have the U2F Rust library in shape for review. Ted, can you please give us some feedback about this dependency soon?

Comment on attachment 8885649 [details] [diff] [review] 0001-Bug-1380270-Add-dlopen-version-of-libudev-sys.patch Review of attachment 8885649 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the wait here. I don't see anything that looks bad here. A few questions: 1) Do you think we could potentially upstream this to libudev-sys / libudev as a cargo feature? It would be nice to not have to maintain this, and it might be useful for other people. Also, the fact that you are using a cargo override to replace libudev-sys isn't the best. 2) I've seen code like this in a different -sys crate, I bet there's room for either a helper crate for this pattern or adding support to bindgen to generate bindings that allow dlopening a library and defining dynamically-loaded function pointers like this.

Comment on attachment 8885649 [details] [diff] [review] 0001-Bug-1380270-Add-dlopen-version-of-libudev-sys.patch Review of attachment 8885649 [details] [diff] [review]: ----------------------------------------------------------------- If ted's ok with it I'm ok with it. ted: I'm not aware of any crates to generate for dlopen, but that's a good idea, maybe worth looking into in a followup?

Yeah. It just seems like the kind of thing that might be more generally useful.

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #13) > 1) Do you think we could potentially upstream this to libudev-sys / libudev > as a cargo feature? It would be nice to not have to maintain this, and it > might be useful for other people. Also, the fact that you are using a cargo > override to replace libudev-sys isn't the best. Yeah, we should eventually upstream it. JC filed bug 1395294. > 2) I've seen code like this in a different -sys crate, I bet there's room > for either a helper crate for this pattern or adding support to bindgen to > generate bindings that allow dlopening a library and defining > dynamically-loaded function pointers like this. Yeah, it would be nice to make this easier. We have similar code in tree that can handle both: http://searchfox.org/mozilla-central/source/media/libcubeb/cubeb-pulse-rs/pulse-ffi/src/ffi_funcs.rs

Pushed by ttaubert@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7a5f3c233e1b Add dlopen() version of libudev-sys r=qdot,ted

Hey team I am having a user mention that these are not turned on by default? https://twitter.com/jacobian/status/931319475093225472 Any ideas if that is just github?

Landing Sheriff isn't gonna be able to help you much on that. Redirecting to :jcj.

I replied [1]; U2F we're not planning to ship. Web Authentication is coming along nicely, and currently looks like 59/60 timeframe [2]. [1] https://twitter.com/jamespugjones/status/931324766782332928 [2] https://wiki.mozilla.org/Security/CryptoEngineering#WD-07_Updates