Repository navigation
refactor: create pxp - #311
michaelbeaumont wants to merge 9 commits into
Conversation
|
Thank you @michaelbeaumont! I'm supportive of the change overall, will review this PR more thoroughly as soon as I can - aiming for this or next weekend. What would you think about renaming this drop the |
7b0a7f8 to
97915f9
Compare
Done! |
AlfioEmanueleFresta
left a comment
There was a problem hiding this comment.
Thank you so much for working on this! Looks good to me - a few comments inline to be addressed before merging.
Also, could you rebase this on top of #314 - making sure we apply the same ScanGuard fixes to the ble code in the new create?
| #[derive(Debug)] | ||
| pub struct TunnelHandle { | ||
| /// The connection task: proximity check, handshake, then message exchange. | ||
| pub task: task::JoinHandle<()>, |
There was a problem hiding this comment.
Nit: can we make anything we don't need to be public here private?
There was a problem hiding this comment.
changed visibility of a couple things
libwebauthn-pxppxp
| [package] | ||
| name = "pxp" | ||
| description = "FIDO Proximity Exchange Protocol (PXP, formerly CTAP hybrid / caBLE) client for Linux, written in Rust" | ||
| version = "0.10.0" |
There was a problem hiding this comment.
Do we want to start at version 0.10.0? 🤷
I was pretty aggressive with the renaming but "caBLE" is now two generations back, it's getting hard to find references to it in any specs so it should go, IMO.
I've used the new
libwebauthn-pxpcrate alone for non-CTAP purposes and it works.Fixes #307
One thing that's missing as described in the spec is technically:> Every message transmitted over this socket MUST be prefixed with a 4-byte big-endian length prefix, representing the length of the message payload in bytes.But of course this is a breaking change. Maybe we need a toggle for framing? Otherwise everything should "just work".