chirpstack / chirpstack-gateway-bridge

ChirpStack Gateway Bridge abstracts Packet Forwarder protocols into Protobuf or JSON over MQTT.
https://www.chirpstack.io
MIT License
422 stars 270 forks source link

basicstation - validate client certificate CommonName against EUI #125

Closed sa-wilson closed 5 years ago

sa-wilson commented 5 years ago

Is this a bug or a feature request?

Feature Request

What did you expect?

Allow for the possibility when using the BasicStation backend with client certificates to make sure that the gateway has it's own EUI as the CommonName on the certificate, to prevent a gateway masquerading as another. This is the setup we're currently using with the v2 bridge, bridges on each gateway (which we'd like to eliminate with BasicStation having TLS and auth capability), and mosquitto-auth-plug pattern ACLs with required client certificates.

What happened?

Certificates appear to only be verified that they're signed by the CA.

I do have to admit that I haven't tested this so far, and have only looked at the code, so if I've missed something I apologise. I'm in the process of getting BasicStation up and running on our gateways and will be testing it with the v3 bridge soon before updating everything to v3 once it's working.

sa-wilson commented 5 years ago

I've just opened a pull request for this. #129

Have you had any luck getting BasicStation to work properly otherwise? I had to comment out the JoinEUI filter lists to get it to not crash, and it seems to drop the socket and then die every time the ping gets sent and basicstation ignores it.

brocaar commented 5 years ago

I did report both issues as I experienced the JoinEUI related crash too: https://github.com/lorabasics/basicstation/issues?utf8=%E2%9C%93&q=author%3Abrocaar.

Since the Basic Station ignores the pings, I recommend to set the read_timeout="1h". The Ping / Pong is to generate a response (which the Basic Station currently does not support) but as long there is data being received by the gateway within the configured read_timeout, this timer will be reset so that LoRa Gateway Bridge does not close the connection.

sa-wilson commented 5 years ago

Ahh, good to know. I missed that option when I skimmed over the config file earlier.

It looks like the NetID filter needs to be left out of the JSON when empty as well as the JoinEUI filter, otherwise BasicStation ends up filtering for 00000001. It seems to get set to FFFFFFFF when left out and it gets messages from all devices. I should probably open another issue for this, but it seems like setting the JSON fields to omitempty and leaving them out entirely when left empty would be the best move.

brocaar commented 5 years ago

I did see it working, so it might also be a specific race-condition when the Basic Station crashes.

It looks like the NetID filter needs to be left out of the JSON when empty as well as the JoinEUI filter

From the documentation:

The fields NetID and JoinEui are used to filter LoRa frames received by Station. NetID is a list of NetID values that are accepted. Any LoRa data frame carrying a NetID other than the listed ones will be dropped. JoinEui is a list of pairs of integer values encoding ranges of join EUIs. Join request frames will be dropped by Station unless the field JoinEui in the message satisfies the condition BegEui<=JoinEui<=EndEui for at least one pair [BegEui,EndEui].

From the docs (https://doc.sm.tc/station/tcproto.html) it is not really clear if an empty list vs no list should make a difference (in my opinion the behavior should be the same). When following the docs strictly, any uplink would be rejected as it would not match any JoinEUI or NetID. This might as well be something to report here: https://github.com/lorabasics/basicstation/issues.

brocaar commented 5 years ago

(closing this issue as the validate CN has been merged)