Add wireguard psk server

[faern]

Implement the server part of the wireguard psk exchange.

---

This change is 

[mvd-ows]

Review status: 0 of 8 files reviewed at latest revision, 3 unresolved discussions.

---

oqs-kex-rpc/src/server.rs, line 95 at r1 (raw file):

    }

    fn perform_exchange(&self, meta: M, alice_msgs: &[AliceMsg]) -> Result<Vec<BobMsg>> {

What is the effect of allowing errors to pass this boundary?

---

wireguard-psk-exchange/src/server/main.rs, line 109 at r1 (raw file):

#[derive(Debug, Default, Clone, Eq, PartialEq, Hash)]
struct KexMetadata {
    peer: Option<wg::Peer>,

Is the reason for using Option<> here that it allows you to more easily pass the type around? Result would have been my first choice but that doesn't seem to work well with the rpc server lib?

---

wireguard-psk-exchange/src/server/wg.rs, line 55 at r1 (raw file):

}

fn parse_cidrs_to_ips(input: &str) -> ::std::result::Result<Vec<IpAddr>, ()> {

I guess Result<T, ()> is the idiomatic way of declaring that you don't care about the specific error?

---

Comments from Reviewable

[faern]

Review status: 0 of 8 files reviewed at latest revision, 3 unresolved discussions.

---

oqs-kex-rpc/src/server.rs, line 95 at r1 (raw file):

Previously, mvd-ows wrote…

What is the effect of allowing errors to pass this boundary?

As you can see down in the implementation of fn kex(...) any error in perform_exchange is logged and then an "internal error" is sent back to the JSON-RPC client. So for any error on the server it will be logged and then the client will just see an anonymous internal error.

---

wireguard-psk-exchange/src/server/main.rs, line 109 at r1 (raw file):

Previously, mvd-ows wrote…

Is the reason for using Option<> here that it allows you to more easily pass the type around? Result would have been my first choice but that doesn't seem to work well with the rpc server lib?

What would be a reasonable error in the case it was Result<wg::Peer, ???>? I mostly wanted to signal that it was a peer or none.

Or, We could put the error from meta_extractor in that result so we don't just print it to stderr. Then we can have the meta_extractor error be forwarded up. Would you like that? That might be nice actually.

---

Comments from Reviewable

[faern]

Review status: 0 of 8 files reviewed at latest revision, 3 unresolved discussions.

---

wireguard-psk-exchange/src/server/wg.rs, line 55 at r1 (raw file):

Previously, mvd-ows wrote…

I guess Result is the idiomatic way of declaring that you don't care about the specific error?

Ideally the error should be a real error. But here we didn't want to handle the errors in any way, invalid peer entries are just ignored, so then it could be (). We could just as well then just have Option<T> Instead actually. Would work the same. Might be nicer with Option actually, then we also get rid of the ::std::result:: part.

---

Comments from Reviewable

[mvd-ows]

Review status: 0 of 8 files reviewed at latest revision, 3 unresolved discussions.

---

oqs-kex-rpc/src/server.rs, line 95 at r1 (raw file):

Previously, faern (Linus Färnstrand) wrote…

As you can see down in the implementation of `fn kex(...)` any error in `perform_exchange` is logged and then an "internal error" is sent back to the JSON-RPC client. So for any error on the server it will be logged and then the client will just see an anonymous internal error.

Right, that makes sense.

---

wireguard-psk-exchange/src/server/main.rs, line 109 at r1 (raw file):

Previously, faern (Linus Färnstrand) wrote…

What would be a reasonable error in the case it was `Result`? I mostly wanted to signal that it was a peer or none. Or, We could put the error from `meta_extractor` in that result so we don't just print it to stderr. Then we can have the meta_extractor error be forwarded up. Would you like that? That might be nice actually.

I meant Result but from the usage that doesn't seem like an alternative?

---

wireguard-psk-exchange/src/server/wg.rs, line 55 at r1 (raw file):

Previously, faern (Linus Färnstrand) wrote…

Ideally the error should be a real error. But here we didn't want to handle the errors in any way, invalid peer entries are just ignored, so then it could be `()`. We could just as well then just have `Option` Instead actually. Would work the same. Might be nicer with `Option` actually, then we also get rid of the `::std::result::` part.

Would prefer Option, yes.

---

Comments from Reviewable

[mvd-ows]

wireguard-psk-exchange/src/server/wg.rs, line 55 at r1 (raw file):

Previously, mvd-ows wrote…

Would prefer Option, yes.

Test to see if brackets are stripped consistenly: Option

---

Comments from Reviewable

[mvd-ows]

wireguard-psk-exchange/src/server/wg.rs, line 55 at r1 (raw file):

Previously, mvd-ows wrote…

Test to see if brackets are stripped consistenly: Option

WTF...

---

Comments from Reviewable

[mvd-ows]

Reviewed 5 of 9 files at r1, 6 of 6 files at r2. Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

Comments from Reviewable