Skip to content

Commit 1fd7855

Browse files
brocaarbrocaar
committed
Fix uplink FCnt security issue.
1 parent 0000c96 commit 1fd7855

File tree

6 files changed

+41
-32
lines changed

6 files changed

+41
-32
lines changed

‎cmd/loraserver/main.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -217,12 +217,12 @@ func mustGetTransportCredentials(tlsCert, tlsKey, caCert string, verifyClientCer
217217
RootCAs: caCertPool,
218218
ClientAuth: tls.RequireAndVerifyClientCert,
219219
})
220-
} else {
221-
return credentials.NewTLS(&tls.Config{
222-
Certificates: []tls.Certificate{cert},
223-
RootCAs: caCertPool,
224-
})
225220
}
221+
222+
return credentials.NewTLS(&tls.Config{
223+
Certificates: []tls.Certificate{cert},
224+
RootCAs: caCertPool,
225+
})
226226
}
227227

228228
func main() {

‎docs/changelog.md

+9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
# Changelog
22

3+
## 0.12.5
4+
5+
**Security:**
6+
7+
* This release fixes a `FCnt` related security issue. Instead of keeping the
8+
uplink `FCnt` value in sync with the `FCnt` of the uplink transmission, it
9+
is incremented (uplink `FCnt + 1`) after it has been processed by
10+
LoRa Server.
11+
312
## 0.12.4
413

514
* Fix regression that caused a FCnt roll-over to result in an invalid MIC

‎internal/session/session_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,9 @@ func TestNodeSession(t *testing.T) {
8181
FullFCnt uint32
8282
Valid bool
8383
}{
84-
{0, 1, 1, true}, // ideal case counter was incremented
85-
{1, 1, 1, true}, // re-transmission
86-
{2, 1, 0, false}, // old packet received
84+
{0, 1, 1, true}, // one packet was lost
85+
{1, 1, 1, true}, // ideal case, the FCnt has the expected value
86+
{2, 1, 0, false}, // old packet received or re-transmission
8787
{0, common.Band.MaxFCntGap, 0, false}, // gap should be less than MaxFCntGap
8888
{0, common.Band.MaxFCntGap - 1, common.Band.MaxFCntGap - 1, true}, // gap is exactly within the allowed MaxFCntGap
8989
{65536, common.Band.MaxFCntGap - 1, common.Band.MaxFCntGap - 1 + 65536, true}, // roll-over happened, gap ix exactly within allowed MaxFCntGap

‎internal/testsuite/uplink_test.go

+21-21
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ func TestUplinkScenarios(t *testing.T) {
413413
ExpectedControllerHandleRXInfo: expectedControllerHandleRXInfo,
414414
ExpectedApplicationHandleDataUp: expectedApplicationPushDataUpNoData,
415415
ExpectedApplicationGetDataDown: expectedGetDataDown,
416-
ExpectedFCntUp: 0,
416+
ExpectedFCntUp: 1,
417417
ExpectedFCntDown: 0,
418418
},
419419
}
@@ -446,7 +446,7 @@ func TestUplinkScenarios(t *testing.T) {
446446
ExpectedControllerHandleRXInfo: expectedControllerHandleRXInfo,
447447
ExpectedApplicationHandleDataUp: expectedApplicationPushDataUp,
448448
ExpectedApplicationGetDataDown: expectedGetDataDown,
449-
ExpectedFCntUp: 10,
449+
ExpectedFCntUp: 11,
450450
ExpectedFCntDown: 5,
451451
},
452452
{
@@ -470,7 +470,7 @@ func TestUplinkScenarios(t *testing.T) {
470470
ExpectedControllerHandleRXInfo: expectedControllerHandleRXInfo,
471471
ExpectedApplicationHandleDataUp: expectedApplicationPushDataUpNoData,
472472
ExpectedApplicationGetDataDown: expectedGetDataDown,
473-
ExpectedFCntUp: 10,
473+
ExpectedFCntUp: 11,
474474
ExpectedFCntDown: 5,
475475
},
476476
{
@@ -516,7 +516,7 @@ func TestUplinkScenarios(t *testing.T) {
516516
},
517517
},
518518
},
519-
ExpectedFCntUp: 10,
519+
ExpectedFCntUp: 11,
520520
ExpectedFCntDown: 6,
521521
},
522522
{
@@ -561,7 +561,7 @@ func TestUplinkScenarios(t *testing.T) {
561561
},
562562
},
563563
},
564-
ExpectedFCntUp: 10,
564+
ExpectedFCntUp: 11,
565565
ExpectedFCntDown: 6,
566566
},
567567
{
@@ -606,7 +606,7 @@ func TestUplinkScenarios(t *testing.T) {
606606
},
607607
},
608608
},
609-
ExpectedFCntUp: 10,
609+
ExpectedFCntUp: 11,
610610
ExpectedFCntDown: 6,
611611
},
612612
{
@@ -656,7 +656,7 @@ func TestUplinkScenarios(t *testing.T) {
656656
},
657657
},
658658
},
659-
ExpectedFCntUp: 10,
659+
ExpectedFCntUp: 11,
660660
ExpectedFCntDown: 6,
661661
},
662662
{
@@ -701,7 +701,7 @@ func TestUplinkScenarios(t *testing.T) {
701701
},
702702
},
703703
},
704-
ExpectedFCntUp: 10,
704+
ExpectedFCntUp: 11,
705705
ExpectedFCntDown: 6,
706706
},
707707
{
@@ -731,7 +731,7 @@ func TestUplinkScenarios(t *testing.T) {
731731
{AppEUI: ns.AppEUI[:], DevEUI: ns.DevEUI[:], Data: []byte{2}},
732732
{AppEUI: ns.AppEUI[:], DevEUI: ns.DevEUI[:], Data: []byte{3, 1}},
733733
},
734-
ExpectedFCntUp: 10,
734+
ExpectedFCntUp: 11,
735735
ExpectedFCntDown: 5,
736736
},
737737
{
@@ -763,7 +763,7 @@ func TestUplinkScenarios(t *testing.T) {
763763
{AppEUI: ns.AppEUI[:], DevEUI: ns.DevEUI[:], FrmPayload: true, Data: []byte{2}},
764764
{AppEUI: ns.AppEUI[:], DevEUI: ns.DevEUI[:], FrmPayload: true, Data: []byte{3, 1}},
765765
},
766-
ExpectedFCntUp: 10,
766+
ExpectedFCntUp: 11,
767767
ExpectedFCntDown: 5,
768768
},
769769
{
@@ -788,7 +788,7 @@ func TestUplinkScenarios(t *testing.T) {
788788
ExpectedControllerHandleRXInfo: expectedControllerHandleRXInfo,
789789
ExpectedApplicationHandleDataUp: expectedApplicationPushDataUpFCntRollOver,
790790
ExpectedApplicationGetDataDown: expectedGetDataDown,
791-
ExpectedFCntUp: 65536,
791+
ExpectedFCntUp: 65537,
792792
ExpectedFCntDown: 5,
793793
},
794794
}
@@ -848,7 +848,7 @@ func TestUplinkScenarios(t *testing.T) {
848848
},
849849
},
850850
},
851-
ExpectedFCntUp: 10,
851+
ExpectedFCntUp: 11,
852852
ExpectedFCntDown: 6,
853853
},
854854
{
@@ -907,7 +907,7 @@ func TestUplinkScenarios(t *testing.T) {
907907
},
908908
},
909909
},
910-
ExpectedFCntUp: 10,
910+
ExpectedFCntUp: 11,
911911
ExpectedFCntDown: 6,
912912
},
913913
{
@@ -960,7 +960,7 @@ func TestUplinkScenarios(t *testing.T) {
960960
},
961961
},
962962
},
963-
ExpectedFCntUp: 10,
963+
ExpectedFCntUp: 11,
964964
ExpectedFCntDown: 6,
965965
},
966966
{
@@ -1022,7 +1022,7 @@ func TestUplinkScenarios(t *testing.T) {
10221022
{DevEUI: ns.DevEUI, FRMPayload: true, Data: []byte{6}},
10231023
{DevEUI: ns.DevEUI, FRMPayload: true, Data: []byte{8, 3}},
10241024
},
1025-
ExpectedFCntUp: 10,
1025+
ExpectedFCntUp: 11,
10261026
ExpectedFCntDown: 6,
10271027
},
10281028
{
@@ -1096,7 +1096,7 @@ func TestUplinkScenarios(t *testing.T) {
10961096
ExpectedTXMACPayloadQueue: []queue.MACPayload{
10971097
{DevEUI: ns.DevEUI, Data: []byte{2, 10, 6}},
10981098
},
1099-
ExpectedFCntUp: 10,
1099+
ExpectedFCntUp: 11,
11001100
ExpectedFCntDown: 6,
11011101
},
11021102
}
@@ -1155,7 +1155,7 @@ func TestUplinkScenarios(t *testing.T) {
11551155
},
11561156
},
11571157
},
1158-
ExpectedFCntUp: 10,
1158+
ExpectedFCntUp: 11,
11591159
ExpectedFCntDown: 6,
11601160
},
11611161
{
@@ -1209,7 +1209,7 @@ func TestUplinkScenarios(t *testing.T) {
12091209
},
12101210
},
12111211
},
1212-
ExpectedFCntUp: 10,
1212+
ExpectedFCntUp: 11,
12131213
ExpectedFCntDown: 6,
12141214
},
12151215
{
@@ -1260,7 +1260,7 @@ func TestUplinkScenarios(t *testing.T) {
12601260
},
12611261
},
12621262
},
1263-
ExpectedFCntUp: 10,
1263+
ExpectedFCntUp: 11,
12641264
ExpectedFCntDown: 5, // will be incremented after the node ACKs the frame
12651265
},
12661266
{
@@ -1288,7 +1288,7 @@ func TestUplinkScenarios(t *testing.T) {
12881288
ExpectedControllerHandleRXInfo: expectedControllerHandleRXInfo,
12891289
ExpectedApplicationHandleDataUp: expectedApplicationPushDataUpNoData,
12901290
ExpectedApplicationGetDataDown: expectedGetDataDown,
1291-
ExpectedFCntUp: 10,
1291+
ExpectedFCntUp: 11,
12921292
ExpectedFCntDown: 5, // payload has been discarded, nothing to transmit
12931293
},
12941294
{
@@ -1344,7 +1344,7 @@ func TestUplinkScenarios(t *testing.T) {
13441344
},
13451345
},
13461346
},
1347-
ExpectedFCntUp: 10,
1347+
ExpectedFCntUp: 11,
13481348
ExpectedFCntDown: 6,
13491349
ExpectedApplicationGetDataDown: expectedGetDataDown,
13501350
ExpectedTXMACPayloadQueue: []queue.MACPayload{

‎internal/uplink/data.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,8 @@ func handleCollectedDataUpPackets(ctx common.Context, rxPacket models.RXPacket)
173173
}
174174
}
175175

176-
// sync counter with that of the device
177-
ns.FCntUp = macPL.FHDR.FCnt
176+
// sync counter with that of the device + 1
177+
ns.FCntUp = macPL.FHDR.FCnt + 1
178178
if err := session.SaveNodeSession(ctx.RedisPool, ns); err != nil {
179179
return err
180180
}

‎mkdocs.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ pages:
1515
- changelog.md
1616

1717
extra:
18-
version: '0.12.4'
18+
version: '0.12.5'
1919
github:
2020
download_release: true
2121

0 commit comments

Comments
 (0)