Fix FCnt rollover regression introduced in 0.12.0.

See brocaar/chirpstack-application-server#6.

Author: brocaar

docs/changelog.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## 0.12.4
+
+* Fix regression that caused a FCnt roll-over to result in an invalid MIC
+  error. This was caused by validating the MIC before expanding the 16 bit
+  FCnt to the full 32 bit value. (thanks @andrepferreira)
+
 ## 0.12.3
 
 * Relax frame-counter option.

internal/testsuite/uplink_test.go

@@ -79,6 +79,15 @@ func TestUplinkScenarios(t *testing.T) {
 			AppEUI:   [8]byte{8, 7, 6, 5, 4, 3, 2, 1},
 		}
 
+		nsFCntRollOver := session.NodeSession{
+			DevAddr:  [4]byte{1, 2, 3, 4},
+			DevEUI:   [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
+			NwkSKey:  [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16},
+			FCntUp:   65535,
+			FCntDown: 5,
+			AppEUI:   [8]byte{8, 7, 6, 5, 4, 3, 2, 1},
+		}
+
 		nsRelaxFCnt := session.NodeSession{
 			DevAddr:   [4]byte{1, 2, 3, 4},
 			DevEUI:    [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
@@ -171,6 +180,31 @@ func TestUplinkScenarios(t *testing.T) {
 			},
 		}
 
+		expectedApplicationPushDataUpFCntRollOver := &as.HandleDataUpRequest{
+			AppEUI: ns.AppEUI[:],
+			DevEUI: ns.DevEUI[:],
+			FCnt:   65536,
+			FPort:  1,
+			Data:   []byte{1, 2, 3, 4},
+			TxInfo: &as.TXInfo{
+				Frequency: int64(rxInfo.Frequency),
+				DataRate: &as.DataRate{
+					Modulation:   string(rxInfo.DataRate.Modulation),
+					BandWidth:    uint32(rxInfo.DataRate.Bandwidth),
+					SpreadFactor: uint32(rxInfo.DataRate.SpreadFactor),
+					Bitrate:      uint32(rxInfo.DataRate.BitRate),
+				},
+			},
+			RxInfo: []*as.RXInfo{
+				{
+					Mac:     rxInfo.MAC[:],
+					Time:    rxInfo.Time.Format(time.RFC3339Nano),
+					Rssi:    int32(rxInfo.RSSI),
+					LoRaSNR: rxInfo.LoRaSNR,
+				},
+			},
+		}
+
 		expectedApplicationPushDataUpNoData := &as.HandleDataUpRequest{
 			AppEUI: ns.AppEUI[:],
 			DevEUI: ns.DevEUI[:],
@@ -732,6 +766,31 @@ func TestUplinkScenarios(t *testing.T) {
 					ExpectedFCntUp:   10,
 					ExpectedFCntDown: 5,
 				},
+				{
+					Name:        "unconfirmed uplink with FCnt rollover",
+					NodeSession: nsFCntRollOver,
+					RXInfo:      rxInfo,
+					SetMICKey:   ns.NwkSKey,
+					PHYPayload: lorawan.PHYPayload{
+						MHDR: lorawan.MHDR{
+							MType: lorawan.UnconfirmedDataUp,
+							Major: lorawan.LoRaWANR1,
+						},
+						MACPayload: &lorawan.MACPayload{
+							FHDR: lorawan.FHDR{
+								DevAddr: ns.DevAddr,
+								FCnt:    65536,
+							},
+							FPort:      &fPortOne,
+							FRMPayload: []lorawan.Payload{&lorawan.DataPayload{Bytes: []byte{1, 2, 3, 4}}},
+						},
+					},
+					ExpectedControllerHandleRXInfo:  expectedControllerHandleRXInfo,
+					ExpectedApplicationHandleDataUp: expectedApplicationPushDataUpFCntRollOver,
+					ExpectedApplicationGetDataDown:  expectedGetDataDown,
+					ExpectedFCntUp:                  65536,
+					ExpectedFCntDown:                5,
+				},
 			}
 
 			runUplinkTests(ctx, tests)
@@ -1319,10 +1378,17 @@ func runUplinkTests(ctx common.Context, tests []uplinkTestCase) {
 			}
 			So(t.PHYPayload.SetMIC(t.SetMICKey), ShouldBeNil)
 
+			// marshal and unmarshal the PHYPayload to make sure the FCnt gets
+			// truncated to to 16 bit
+			var phy lorawan.PHYPayload
+			b, err := t.PHYPayload.MarshalBinary()
+			So(err, ShouldBeNil)
+			So(phy.UnmarshalBinary(b), ShouldBeNil)
+
 			// create RXPacket and call HandleRXPacket
 			rxPacket := gw.RXPacket{
 				RXInfo:     t.RXInfo,
-				PHYPayload: t.PHYPayload,
+				PHYPayload: phy,
 			}
 			So(uplink.HandleRXPacket(ctx, rxPacket), ShouldResemble, t.ExpectedHandleRXPacketError)
 

internal/uplink/data.go

@@ -32,21 +32,6 @@ func validateAndCollectDataUpRXPacket(ctx common.Context, rxPacket gw.RXPacket)
 		return err
 	}
 
-	// validate MIC
-	micOK, err := rxPacket.PHYPayload.ValidateMIC(ns.NwkSKey)
-	if err != nil {
-		return fmt.Errorf("validate MIC error: %s", err)
-	}
-	if !micOK {
-		ctx.Application.HandleError(context.Background(), &as.HandleErrorRequest{
-			AppEUI: ns.AppEUI[:],
-			DevEUI: ns.DevEUI[:],
-			Type:   as.ErrorType_DATA_UP_MIC,
-			Error:  "invalid MIC",
-		})
-		return errors.New("invalid MIC")
-	}
-
 	// validate and get the full int32 FCnt
 	fullFCnt, ok := session.ValidateAndGetFullFCntUp(ns, macPL.FHDR.FCnt)
 	if !ok {
@@ -81,6 +66,21 @@ func validateAndCollectDataUpRXPacket(ctx common.Context, rxPacket gw.RXPacket)
 	}
 	macPL.FHDR.FCnt = fullFCnt
 
+	// validate MIC
+	micOK, err := rxPacket.PHYPayload.ValidateMIC(ns.NwkSKey)
+	if err != nil {
+		return fmt.Errorf("validate MIC error: %s", err)
+	}
+	if !micOK {
+		ctx.Application.HandleError(context.Background(), &as.HandleErrorRequest{
+			AppEUI: ns.AppEUI[:],
+			DevEUI: ns.DevEUI[:],
+			Type:   as.ErrorType_DATA_UP_MIC,
+			Error:  "invalid MIC",
+		})
+		return errors.New("invalid MIC")
+	}
+
 	if macPL.FPort != nil {
 		if *macPL.FPort == 0 {
 			// decrypt FRMPayload with NwkSKey when FPort == 0
@@ -111,6 +111,10 @@ func handleCollectedDataUpPackets(ctx common.Context, rxPacket models.RXPacket)
 		return err
 	}
 
+	// expand the FCnt, the value itself has already been validated during the
+	// collection, so there is no need to handle the ok value
+	macPL.FHDR.FCnt, _ = session.ValidateAndGetFullFCntUp(ns, macPL.FHDR.FCnt)
+
 	log.WithFields(log.Fields{
 		"dev_eui":  ns.DevEUI,
 		"gw_count": len(macs),

mkdocs.yml

@@ -15,7 +15,7 @@ pages:
   - changelog.md
 
 extra:
-  version: '0.12.3'
+  version: '0.12.4'
   github:
     download_release: true