Skip to content

Commit

Permalink
Merge pull request #7447 from TheThingsNetwork/fix/7376-linkadrreq-ad…
Browse files Browse the repository at this point in the history 
…r-bit-check

Check ADR bit in `LinkADRAns`
  • Loading branch information
@halimi
halimi authored Jan 6, 2025
2 parents 7848310 + 822e453 commit 013943d
Show file tree
Hide file tree
Showing 5 changed files with 202 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ For details about compatibility between different releases, see the **Commitment

- Enforce default page limit on AS and NS List RPCs if a value is not provided in the request.
- Swapped field order in `RelayNotifyNewEndDeviceReq` MAC command.
- `LinkADRAns` MAC command verification when the end device does not support ADR.

### Security

Expand Down
10 changes: 9 additions & 1 deletion pkg/networkserver/grpc_gsns.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,15 @@ macLoop:
break
}
cmds = cmds[dupCount:]
evs, err = mac.HandleLinkADRAns(ctx, dev, pld, uint(dupCount), cmacFMatchResult.FullFCnt, fps)
evs, err = mac.HandleLinkADRAns(
ctx,
dev,
pld,
uint(dupCount), // nolint: gosec
cmacFMatchResult.FullFCnt,
fps,
up.GetPayload().GetMacPayload().GetFHdr().GetFCtrl().GetAdr(),
)
case ttnpb.MACCommandIdentifier_CID_DUTY_CYCLE:
evs, err = mac.HandleDutyCycleAns(ctx, dev)
case ttnpb.MACCommandIdentifier_CID_RX_PARAM_SETUP:
Expand Down
19 changes: 18 additions & 1 deletion pkg/networkserver/mac/link_adr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ func HandleLinkADRAns(
dupCount uint,
fCntUp uint32,
fps *frequencyplans.Store,
adrEnabled bool,
) (events.Builders, error) {
if pld == nil {
return nil, ErrNoPayload.New()
Expand All @@ -321,7 +322,23 @@ func HandleLinkADRAns(
}

ev := EvtReceiveLinkADRAccept
if !pld.ChannelMaskAck || !pld.DataRateIndexAck || !pld.TxPowerIndexAck {
rejected := false

// LoRaWAN 1.0.4 spec L534-538:
// An end-device SHOULD accept the channel mask controls present in LinkADRReq, even
// when the ADR bit is not set. The end-device SHALL respond to all LinkADRReq commands
// with a LinkADRAns indicating which command elements were accepted and which were
// rejected. This behavior differs from when the uplink ADR bit is set, in which case the end-
// device accepts or rejects the entire command.
if macspec.LinkADRReqRejected(macState.LorawanVersion) {
rejected = !pld.ChannelMaskAck ||
(adrEnabled && !pld.DataRateIndexAck) ||
(adrEnabled && !pld.TxPowerIndexAck)
} else {
rejected = !pld.ChannelMaskAck || !pld.DataRateIndexAck || !pld.TxPowerIndexAck
}

if rejected {
ev = EvtReceiveLinkADRReject

// See "Table 6: LinkADRAns status bits signification" of LoRaWAN 1.1 specification
Expand Down
170 changes: 167 additions & 3 deletions pkg/networkserver/mac/link_adr_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ func TestHandleLinkADRAns(t *testing.T) {
DupCount uint
Events events.Builders
Error error
AdrEnabled bool
}{
{
Name: "nil payload",
Expand All @@ -451,7 +452,8 @@ func TestHandleLinkADRAns(t *testing.T) {
LorawanVersion: ttnpb.MACVersion_MAC_V1_1,
},
},
Error: ErrNoPayload,
Error: ErrNoPayload,
AdrEnabled: true,
},
{
Name: "no request",
Expand Down Expand Up @@ -481,7 +483,163 @@ func TestHandleLinkADRAns(t *testing.T) {
TxPowerIndexAck: true,
})),
},
Error: ErrRequestNotFound.WithAttributes("cid", ttnpb.MACCommandIdentifier_CID_LINK_ADR),
Error: ErrRequestNotFound.WithAttributes("cid", ttnpb.MACCommandIdentifier_CID_LINK_ADR),
AdrEnabled: true,
},
{
Name: "1.0.2/channel mask on/adr enabled/rejected",
Device: &ttnpb.EndDevice{
FrequencyPlanId: test.EUFrequencyPlanID,
LorawanPhyVersion: ttnpb.PHYVersion_RP001_V1_0_2_REV_B,
MacState: &ttnpb.MACState{
LorawanVersion: ttnpb.MACVersion_MAC_V1_0_2,
},
},
Expected: &ttnpb.EndDevice{
FrequencyPlanId: test.EUFrequencyPlanID,
LorawanPhyVersion: ttnpb.PHYVersion_RP001_V1_0_2_REV_B,
MacState: &ttnpb.MACState{
LorawanVersion: ttnpb.MACVersion_MAC_V1_0_2,
},
},
Payload: &ttnpb.MACCommand_LinkADRAns{
ChannelMaskAck: true,
DataRateIndexAck: false,
TxPowerIndexAck: false,
},
Events: events.Builders{
EvtReceiveLinkADRReject.With(events.WithData(&ttnpb.MACCommand_LinkADRAns{
ChannelMaskAck: true,
DataRateIndexAck: false,
TxPowerIndexAck: false,
})),
},
Error: ErrRequestNotFound.WithAttributes("cid", ttnpb.MACCommandIdentifier_CID_LINK_ADR),
AdrEnabled: true,
},
{
Name: "1.0.4/channel mask on/adr disabled/accepted",
Device: &ttnpb.EndDevice{
FrequencyPlanId: test.EUFrequencyPlanID,
LorawanPhyVersion: ttnpb.PHYVersion_RP002_V1_0_4,
MacState: &ttnpb.MACState{
LorawanVersion: ttnpb.MACVersion_MAC_V1_0_4,
},
},
Expected: &ttnpb.EndDevice{
FrequencyPlanId: test.EUFrequencyPlanID,
LorawanPhyVersion: ttnpb.PHYVersion_RP002_V1_0_4,
MacState: &ttnpb.MACState{
LorawanVersion: ttnpb.MACVersion_MAC_V1_0_4,
},
},
Payload: &ttnpb.MACCommand_LinkADRAns{
ChannelMaskAck: true,
DataRateIndexAck: false,
TxPowerIndexAck: false,
},
Events: events.Builders{
EvtReceiveLinkADRAccept.With(events.WithData(&ttnpb.MACCommand_LinkADRAns{
ChannelMaskAck: true,
DataRateIndexAck: false,
TxPowerIndexAck: false,
})),
},
Error: ErrRequestNotFound.WithAttributes("cid", ttnpb.MACCommandIdentifier_CID_LINK_ADR),
AdrEnabled: false,
},
{
Name: "1.0.4/channel mask off/adr disabled/rejected",
Device: &ttnpb.EndDevice{
FrequencyPlanId: test.EUFrequencyPlanID,
LorawanPhyVersion: ttnpb.PHYVersion_RP002_V1_0_4,
MacState: &ttnpb.MACState{
LorawanVersion: ttnpb.MACVersion_MAC_V1_0_4,
},
},
Expected: &ttnpb.EndDevice{
FrequencyPlanId: test.EUFrequencyPlanID,
LorawanPhyVersion: ttnpb.PHYVersion_RP002_V1_0_4,
MacState: &ttnpb.MACState{
LorawanVersion: ttnpb.MACVersion_MAC_V1_0_4,
},
},
Payload: &ttnpb.MACCommand_LinkADRAns{
ChannelMaskAck: false,
DataRateIndexAck: false,
TxPowerIndexAck: false,
},
Events: events.Builders{
EvtReceiveLinkADRReject.With(events.WithData(&ttnpb.MACCommand_LinkADRAns{
ChannelMaskAck: false,
DataRateIndexAck: false,
TxPowerIndexAck: false,
})),
},
Error: ErrRequestNotFound.WithAttributes("cid", ttnpb.MACCommandIdentifier_CID_LINK_ADR),
AdrEnabled: false,
},
{
Name: "1.0.4/channel mask on/adr enabled/rejected",
Device: &ttnpb.EndDevice{
FrequencyPlanId: test.EUFrequencyPlanID,
LorawanPhyVersion: ttnpb.PHYVersion_RP002_V1_0_4,
MacState: &ttnpb.MACState{
LorawanVersion: ttnpb.MACVersion_MAC_V1_0_4,
},
},
Expected: &ttnpb.EndDevice{
FrequencyPlanId: test.EUFrequencyPlanID,
LorawanPhyVersion: ttnpb.PHYVersion_RP002_V1_0_4,
MacState: &ttnpb.MACState{
LorawanVersion: ttnpb.MACVersion_MAC_V1_0_4,
},
},
Payload: &ttnpb.MACCommand_LinkADRAns{
ChannelMaskAck: true,
DataRateIndexAck: false,
TxPowerIndexAck: false,
},
Events: events.Builders{
EvtReceiveLinkADRReject.With(events.WithData(&ttnpb.MACCommand_LinkADRAns{
ChannelMaskAck: true,
DataRateIndexAck: false,
TxPowerIndexAck: false,
})),
},
Error: ErrRequestNotFound.WithAttributes("cid", ttnpb.MACCommandIdentifier_CID_LINK_ADR),
AdrEnabled: true,
},
{
Name: "1.0.4/channel mask off/adr enabled/rejected",
Device: &ttnpb.EndDevice{
FrequencyPlanId: test.EUFrequencyPlanID,
LorawanPhyVersion: ttnpb.PHYVersion_RP002_V1_0_4,
MacState: &ttnpb.MACState{
LorawanVersion: ttnpb.MACVersion_MAC_V1_0_4,
},
},
Expected: &ttnpb.EndDevice{
FrequencyPlanId: test.EUFrequencyPlanID,
LorawanPhyVersion: ttnpb.PHYVersion_RP002_V1_0_4,
MacState: &ttnpb.MACState{
LorawanVersion: ttnpb.MACVersion_MAC_V1_0_4,
},
},
Payload: &ttnpb.MACCommand_LinkADRAns{
ChannelMaskAck: false,
DataRateIndexAck: false,
TxPowerIndexAck: false,
},
Events: events.Builders{
EvtReceiveLinkADRReject.With(events.WithData(&ttnpb.MACCommand_LinkADRAns{
ChannelMaskAck: false,
DataRateIndexAck: false,
TxPowerIndexAck: false,
})),
},
Error: ErrRequestNotFound.WithAttributes("cid", ttnpb.MACCommandIdentifier_CID_LINK_ADR),
AdrEnabled: true,
},
{
Name: "1 request/all ack",
Expand Down Expand Up @@ -547,6 +705,7 @@ func TestHandleLinkADRAns(t *testing.T) {
TxPowerIndexAck: true,
})),
},
AdrEnabled: true,
},
{
Name: "1.1/2 requests/all ack",
Expand Down Expand Up @@ -636,6 +795,7 @@ func TestHandleLinkADRAns(t *testing.T) {
TxPowerIndexAck: true,
})),
},
AdrEnabled: true,
},
{
Name: "1.0.2/2 requests/all ack",
Expand Down Expand Up @@ -726,6 +886,7 @@ func TestHandleLinkADRAns(t *testing.T) {
TxPowerIndexAck: true,
})),
},
AdrEnabled: true,
},
{
Name: "1.0/2 requests/all ack",
Expand Down Expand Up @@ -825,6 +986,7 @@ func TestHandleLinkADRAns(t *testing.T) {
TxPowerIndexAck: true,
})),
},
AdrEnabled: true,
},
{
Name: "1.0.2/2 requests/US915 FSB2",
Expand Down Expand Up @@ -888,6 +1050,7 @@ func TestHandleLinkADRAns(t *testing.T) {
TxPowerIndexAck: true,
})),
},
AdrEnabled: true,
},
} {
tc := tc
Expand All @@ -897,7 +1060,8 @@ func TestHandleLinkADRAns(t *testing.T) {
Func: func(ctx context.Context, t *testing.T, a *assertions.Assertion) {
dev := ttnpb.Clone(tc.Device)

evs, err := HandleLinkADRAns(ctx, dev, tc.Payload, tc.DupCount, fCntUp, frequencyplans.NewStore(test.FrequencyPlansFetcher))
fps := frequencyplans.NewStore(test.FrequencyPlansFetcher)
evs, err := HandleLinkADRAns(ctx, dev, tc.Payload, tc.DupCount, fCntUp, fps, tc.AdrEnabled)
if tc.Error != nil && !a.So(err, should.EqualErrorOrDefinition, tc.Error) ||
tc.Error == nil && !a.So(err, should.BeNil) {
t.FailNow()
Expand Down
7 changes: 7 additions & 0 deletions pkg/specification/macspec/specification.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,3 +237,10 @@ func EncryptionOptions(v ttnpb.MACVersion, frameType FrameType, fPort uint32, cm
func ValidateUplinkPayloadSize(v ttnpb.MACVersion) bool {
return compareMACVersion(v, ttnpb.MACVersion_MAC_V1_1) >= 0
}

// LinkADRReqRejected reports whether v uses the ADR bit in the FCtrl field
// to modify which ACK bits needs to check in the LinkADRAns.
func LinkADRReqRejected(v ttnpb.MACVersion) bool {
return compareMACVersion(v, ttnpb.MACVersion_MAC_V1_0_3) >= 0 &&
compareMACVersion(v, ttnpb.MACVersion_MAC_V1_0_4) <= 0
}

0 comments on commit 013943d

Please sign in to comment.