Skip to content

Commit f149cdc

Browse files
Alonza0314Alonza0314
authored
Merge pull request #65 from HiImPeggy/fix-validate-ike-messageId
fix: validate IKE Message ID
2 parents 5907be4 + d8fcb4a commit f149cdc

5 files changed

Lines changed: 130 additions & 9 deletions

File tree

‎internal/ike/dispatcher.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ func (s *Server) Dispatch(
2323
}
2424
}()
2525

26+
if ikeSA != nil && ikeMessage.ExchangeType != ike_message.IKE_SA_INIT &&
27+
!hasExpectedMessageID(ikeMessage, ikeSA) {
28+
return
29+
}
30+
2631
switch ikeMessage.ExchangeType {
2732
case ike_message.IKE_SA_INIT:
2833
s.HandleIKESAINIT(udpConn, localAddr, remoteAddr, ikeMessage, msg)
@@ -36,3 +41,25 @@ func (s *Server) Dispatch(
3641
ikeLog.Warnf("Unimplemented IKE message type, exchange type: %d", ikeMessage.ExchangeType)
3742
}
3843
}
44+
45+
func hasExpectedMessageID(
46+
ikeMessage *ike_message.IKEMessage,
47+
ikeSA *n3iwf_context.IKESecurityAssociation,
48+
) bool {
49+
ikeLog := logger.IKELog
50+
if !ikeMessage.IsResponse() {
51+
if ikeMessage.MessageID != ikeSA.InitiatorMessageID {
52+
ikeLog.Warnf("Unexpected request MessageID. Expected %d, got %d",
53+
ikeSA.InitiatorMessageID, ikeMessage.MessageID)
54+
return false
55+
}
56+
return true
57+
}
58+
59+
if ikeMessage.MessageID != ikeSA.ResponderMessageID {
60+
ikeLog.Warnf("Unexpected response MessageID. Expected %d, got %d",
61+
ikeSA.ResponderMessageID, ikeMessage.MessageID)
62+
return false
63+
}
64+
return true
65+
}

‎internal/ike/dispatcher_test.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package ike
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
8+
ike_message "github.com/free5gc/ike/message"
9+
n3iwf_context "github.com/free5gc/n3iwf/internal/context"
10+
)
11+
12+
func TestHasExpectedMessageID(t *testing.T) {
13+
ikeSA := &n3iwf_context.IKESecurityAssociation{
14+
InitiatorMessageID: 3,
15+
ResponderMessageID: 7,
16+
}
17+
18+
tests := []struct {
19+
name string
20+
isResponse bool
21+
messageID uint32
22+
expected bool
23+
}{
24+
{name: "request exact match", messageID: 3, expected: true},
25+
{name: "request stale", messageID: 2, expected: false},
26+
{name: "request fast forward", messageID: 4, expected: false},
27+
{name: "response exact match", isResponse: true, messageID: 7, expected: true},
28+
{name: "response stale", isResponse: true, messageID: 6, expected: false},
29+
{name: "response fast forward", isResponse: true, messageID: 8, expected: false},
30+
}
31+
32+
for _, tt := range tests {
33+
t.Run(tt.name, func(t *testing.T) {
34+
flags := uint8(0)
35+
if tt.isResponse {
36+
flags = ike_message.ResponseBitCheck
37+
}
38+
message := &ike_message.IKEMessage{
39+
IKEHeader: &ike_message.IKEHeader{
40+
Flags: flags,
41+
MessageID: tt.messageID,
42+
},
43+
}
44+
45+
require.Equal(t, tt.expected, hasExpectedMessageID(message, ikeSA))
46+
require.Equal(t, uint32(3), ikeSA.InitiatorMessageID)
47+
require.Equal(t, uint32(7), ikeSA.ResponderMessageID)
48+
})
49+
}
50+
}

‎internal/ike/handler.go‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ func (s *Server) HandleIKESAINIT(
4040
ikeLog := logger.IKELog
4141
ikeLog.Infoln("Handle IKE_SA_INIT")
4242

43+
// Validate before allocating an IKE SA so an invalid request cannot leak state.
44+
if message.MessageID != 0 {
45+
ikeLog.Warnf("Invalid IKE_SA_INIT MessageID: got %d, expected 0", message.MessageID)
46+
return
47+
}
48+
4349
// Used to receive value from peer
4450
var securityAssociation *ike_message.SecurityAssociation
4551
var keyExcahge *ike_message.KeyExchange
@@ -152,8 +158,25 @@ func (s *Server) HandleIKESAINIT(
152158

153159
// Create new IKE security association
154160
ikeSecurityAssociation := n3iwfCtx.NewIKESecurityAssociation()
161+
if ikeSecurityAssociation == nil {
162+
ikeLog.Error("Failed to create IKE security association")
163+
return
164+
}
165+
keepIKESA := false
166+
defer func() {
167+
if !keepIKESA {
168+
n3iwfCtx.DeleteIKESecurityAssociation(ikeSecurityAssociation.LocalSPI)
169+
}
170+
}()
171+
155172
ikeSecurityAssociation.RemoteSPI = message.InitiatorSPI
156-
ikeSecurityAssociation.InitiatorMessageID = message.MessageID
173+
174+
// The next request after IKE_SA_INIT must use Message ID 1.
175+
ikeSecurityAssociation.InitiatorMessageID = 1
176+
177+
// The first N3IWF-initiated request uses Message ID 0.
178+
179+
ikeSecurityAssociation.ResponderMessageID = 0
157180

158181
ikeSecurityAssociation.IKESAKey, localPublicValue, err = ike_security.NewIKESAKey(chooseProposal[0],
159182
keyExcahge.KeyExchangeData, concatenatedNonce,
@@ -221,7 +244,9 @@ func (s *Server) HandleIKESAINIT(
221244
err = SendIKEMessageToUE(udpConn, n3iwfAddr, ueAddr, responseIKEMessage, nil)
222245
if err != nil {
223246
ikeLog.Errorf("HandleIKESAINIT(): %v", err)
247+
return
224248
}
249+
keepIKESA = true
225250
}
226251

227252
const (
@@ -248,6 +273,8 @@ func (s *Server) HandleIKEAUTH(
248273
cfg := s.Config()
249274
ipsecGwAddr := cfg.GetIPSecGatewayAddr()
250275

276+
ikeSecurityAssociation.InitiatorMessageID = message.MessageID + 1
277+
251278
// Used for response
252279
var responseIKEPayload ike_message.IKEPayloadContainer
253280

@@ -290,8 +317,6 @@ func (s *Server) HandleIKEAUTH(
290317
}
291318
}
292319

293-
ikeSecurityAssociation.InitiatorMessageID = message.MessageID
294-
295320
switch ikeSecurityAssociation.State {
296321
case PreSignalling:
297322
if initiatorID != nil {
@@ -629,8 +654,6 @@ func (s *Server) HandleIKEAUTH(
629654
N3IWFAddr: n3iwfAddr,
630655
UEAddr: ueAddr,
631656
}
632-
633-
ikeSecurityAssociation.InitiatorMessageID = message.MessageID
634657
} else {
635658
ikeLog.Error("EAP is nil")
636659
}
@@ -1183,11 +1206,13 @@ func (s *Server) HandleInformational(
11831206
}
11841207

11851208
if message.IsResponse() {
1209+
// Receive response from UE
11861210
ikeSecurityAssociation.ResponderMessageID++
11871211
} else { // Get Request message
11881212
SendUEInformationExchange(ikeSecurityAssociation, ikeSecurityAssociation.IKESAKey,
11891213
responseIKEPayload, false, true, message.MessageID,
11901214
udpConn, ueAddr, n3iwfAddr)
1215+
ikeSecurityAssociation.InitiatorMessageID++
11911216
}
11921217
}
11931218

@@ -1275,7 +1300,7 @@ func (s *Server) HandleSendEAP5GFailureMsg(ikeEvt n3iwf_context.IkeEvt) {
12751300

12761301
// Build IKE message
12771302
responseIKEMessage := ike_message.NewMessage(ikeSecurityAssociation.RemoteSPI, ikeSecurityAssociation.LocalSPI,
1278-
ike_message.IKE_AUTH, true, false, ikeSecurityAssociation.InitiatorMessageID, responseIKEPayload)
1303+
ike_message.IKE_AUTH, true, false, ikeSecurityAssociation.InitiatorMessageID-1, responseIKEPayload)
12791304

12801305
// Send IKE message to UE
12811306
err = SendIKEMessageToUE(ikeSecurityAssociation.IKEConnection.Conn,
@@ -1327,7 +1352,7 @@ func (s *Server) HandleSendEAPSuccessMsg(ikeEvt n3iwf_context.IkeEvt) {
13271352
// Build IKE message
13281353
responseIKEMessage := ike_message.NewMessage(ikeSecurityAssociation.RemoteSPI,
13291354
ikeSecurityAssociation.LocalSPI, ike_message.IKE_AUTH, true, false,
1330-
ikeSecurityAssociation.InitiatorMessageID, responseIKEPayload)
1355+
ikeSecurityAssociation.InitiatorMessageID-1, responseIKEPayload)
13311356

13321357
// Send IKE message to UE
13331358
err = SendIKEMessageToUE(ikeSecurityAssociation.IKEConnection.Conn,
@@ -1379,7 +1404,7 @@ func (s *Server) HandleSendEAPNASMsg(ikeEvt n3iwf_context.IkeEvt) {
13791404
// Build IKE message
13801405
responseIKEMessage := ike_message.NewMessage(ikeSecurityAssociation.RemoteSPI,
13811406
ikeSecurityAssociation.LocalSPI, ike_message.IKE_AUTH, true, false,
1382-
ikeSecurityAssociation.InitiatorMessageID, responseIKEPayload)
1407+
ikeSecurityAssociation.InitiatorMessageID-1, responseIKEPayload)
13831408

13841409
// Send IKE message to UE
13851410
err = SendIKEMessageToUE(ikeSecurityAssociation.IKEConnection.Conn,

‎internal/ike/handler_test.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,25 @@ func TestRemoveIkeUe(t *testing.T) {
6363
require.False(t, ok)
6464
}
6565

66+
func TestInvalidIKESAInitMessageIDDoesNotAllocateSA(t *testing.T) {
67+
n3iwf, err := NewN3iwfTestApp(&factory.Config{})
68+
require.NoError(t, err)
69+
70+
n3iwf.ikeServer, err = NewServer(n3iwf)
71+
require.NoError(t, err)
72+
73+
message := ike_message.NewMessage(
74+
1, 0, ike_message.IKE_SA_INIT, false, false, 1, nil)
75+
n3iwf.ikeServer.HandleIKESAINIT(nil, nil, nil, message, nil)
76+
77+
allocated := 0
78+
n3iwf.n3iwfCtx.IKESA.Range(func(_, _ interface{}) bool {
79+
allocated++
80+
return true
81+
})
82+
require.Zero(t, allocated)
83+
}
84+
6685
func TestGenerateNATDetectHash(t *testing.T) {
6786
n3iwf, err := NewN3iwfTestApp(&factory.Config{})
6887
require.NoError(t, err)

‎internal/ngap/handler.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2910,7 +2910,7 @@ func printCriticalityDiagnostics(
29102910
}
29112911
}
29122912
} else {
2913-
ngapLog.Error("IEsCriticalityDiagnostics is nil")
2913+
ngapLog.Debug("IEsCriticalityDiagnostics is nil")
29142914
}
29152915
return
29162916
}

0 commit comments

Comments
 (0)