Change in osmo-msc[master]: vlr: auth_fsm: clarify success/failure result

2022-10-27 Thread neels 
neels has submitted this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/29697 )

Change subject: vlr: auth_fsm: clarify success/failure result
..

vlr: auth_fsm: clarify success/failure result

Explicitly send distinct parent events on auth success and failure. So
far determining success depended only on the data pointer passed on with
the event. Distinct events clarify the logging and the FSM code.

This prepares for a third FSM outcome to be added in a subsequent patch,
to separately signal when the HLR has no auth data.

No functional change.

Related: OS#4830
Change-Id: I02776dfe6785983f2ebe398f57867f5ceb288ba0
---
M include/osmocom/msc/vlr.h
M src/libvlr/vlr_access_req_fsm.c
M src/libvlr/vlr_auth_fsm.c
M src/libvlr/vlr_auth_fsm.h
M src/libvlr/vlr_lu_fsm.c
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
M tests/msc_vlr/msc_vlr_test_hlr_reject.err
M tests/msc_vlr/msc_vlr_test_umts_authen.err
8 files changed, 66 insertions(+), 36 deletions(-)

Approvals:
  Jenkins Builder: Verified
  neels: Looks good to me, approved
  pespin: Looks good to me, but someone else must approve
  laforge: Looks good to me, but someone else must approve



diff --git a/include/osmocom/msc/vlr.h b/include/osmocom/msc/vlr.h
index dd0af7b..b3b658b 100644
--- a/include/osmocom/msc/vlr.h
+++ b/include/osmocom/msc/vlr.h
@@ -67,7 +67,8 @@
VLR_ULA_E_UPDATE_LA,/* Initial trigger (LU from MS) */
VLR_ULA_E_SEND_ID_ACK,  /* Result of Send-ID from PVLR */
VLR_ULA_E_SEND_ID_NACK, /* Result of Send-ID from PVLR */
-   VLR_ULA_E_AUTH_RES, /* Result of auth procedure */
+   VLR_ULA_E_AUTH_RES, /* Successful result of auth procedure */
+   VLR_ULA_E_AUTH_FAILURE, /* Auth procedure failed */
VLR_ULA_E_CIPH_RES, /* Result of Ciphering Mode Command */
VLR_ULA_E_ID_IMSI,  /* IMSI received from MS */
VLR_ULA_E_ID_IMEI,  /* IMEI received from MS */
@@ -437,6 +438,7 @@
PR_ARQ_E_START,
PR_ARQ_E_ID_IMSI,
PR_ARQ_E_AUTH_RES,
+   PR_ARQ_E_AUTH_FAILURE,
PR_ARQ_E_CIPH_RES,
PR_ARQ_E_UPD_LOC_RES,
PR_ARQ_E_TRACE_RES,
diff --git a/src/libvlr/vlr_access_req_fsm.c b/src/libvlr/vlr_access_req_fsm.c
index 5f4795f..bcf589e 100644
--- a/src/libvlr/vlr_access_req_fsm.c
+++ b/src/libvlr/vlr_access_req_fsm.c
@@ -40,6 +40,7 @@
OSMO_VALUE_STRING(PR_ARQ_E_START),
OSMO_VALUE_STRING(PR_ARQ_E_ID_IMSI),
OSMO_VALUE_STRING(PR_ARQ_E_AUTH_RES),
+   OSMO_VALUE_STRING(PR_ARQ_E_AUTH_FAILURE),
OSMO_VALUE_STRING(PR_ARQ_E_CIPH_RES),
OSMO_VALUE_STRING(PR_ARQ_E_UPD_LOC_RES),
OSMO_VALUE_STRING(PR_ARQ_E_TRACE_RES),
@@ -344,6 +345,7 @@
0, 0);
vsub->auth_fsm = auth_fsm_start(vsub, fi,
PR_ARQ_E_AUTH_RES,
+   PR_ARQ_E_AUTH_FAILURE,
par->is_r99,
par->is_utran);
} else {
@@ -438,15 +440,19 @@
 {
enum gsm48_reject_value *cause = data;

-   OSMO_ASSERT(event == PR_ARQ_E_AUTH_RES);
+   switch (event) {
+   case PR_ARQ_E_AUTH_RES:
+   /* Node 2 */
+   _proc_arq_vlr_node2(fi);
+   return;

-   if (!cause || *cause) {
+   case PR_ARQ_E_AUTH_FAILURE:
proc_arq_fsm_done(fi, cause? *cause : 
GSM48_REJECT_NETWORK_FAILURE);
return;
-   }

-   /* Node 2 */
-   _proc_arq_vlr_node2(fi);
+   default:
+   OSMO_ASSERT(false);
+   }
 }

 static void proc_arq_vlr_fn_w_ciph(struct osmo_fsm_inst *fi,
@@ -551,7 +557,8 @@
},
[PR_ARQ_S_WAIT_AUTH] = {
.name = OSMO_STRINGIFY(PR_ARQ_S_WAIT_AUTH),
-   .in_event_mask = S(PR_ARQ_E_AUTH_RES),
+   .in_event_mask = S(PR_ARQ_E_AUTH_RES) |
+S(PR_ARQ_E_AUTH_FAILURE),
.out_state_mask = S(PR_ARQ_S_DONE) |
  S(PR_ARQ_S_WAIT_CIPH) |
  S(PR_ARQ_S_WAIT_UPD_LOC_CHILD) |
diff --git a/src/libvlr/vlr_auth_fsm.c b/src/libvlr/vlr_auth_fsm.c
index a6b5cd2..0583d7c 100644
--- a/src/libvlr/vlr_auth_fsm.c
+++ b/src/libvlr/vlr_auth_fsm.c
@@ -51,6 +51,9 @@
bool auth_requested;

int auth_tuple_max_reuse_count; /* see vlr->cfg instead */
+
+   uint32_t parent_event_success;
+   uint32_t parent_event_failure;
 };

 /***
@@ -240,14 +243,21 @@
 /* Terminate the Auth FSM Instance and notify parent */
 static void auth_fsm_term(struct osmo_fsm_inst *fi, enum gsm48_reject_value 
result)
 {
+   struct auth_fsm_priv *afp = fi->priv;
+
LOGPFSM(fi, "Authentication terminating with result %s\n",
vlr_auth_fs

Change in osmo-msc[master]: vlr: auth_fsm: clarify success/failure result

2022-10-27 Thread neels 
Attention is currently required from: pespin.
neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/29697 )

Change subject: vlr: auth_fsm: clarify success/failure result
..


Patch Set 2: Code-Review+2

(1 comment)

Patchset:

PS2:
combine



--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/29697
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I02776dfe6785983f2ebe398f57867f5ceb288ba0
Gerrit-Change-Number: 29697
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Attention: pespin 
Gerrit-Comment-Date: Thu, 27 Oct 2022 21:15:54 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in osmo-msc[master]: vlr: auth_fsm: clarify success/failure result

2022-10-26 Thread neels 
Attention is currently required from: pespin.
neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/29697 )

Change subject: vlr: auth_fsm: clarify success/failure result
..


Patch Set 1:

(1 comment)

File include/osmocom/msc/vlr.h:

https://gerrit.osmocom.org/c/osmo-msc/+/29697/comment/49b30334_d611ccc8
PS1, Line 70:   VLR_ULA_E_AUTH_RES, /* Successful result of auth procedure 
*/
> Can you then change this one to _SUCCESS?
adding separate patch



--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/29697
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I02776dfe6785983f2ebe398f57867f5ceb288ba0
Gerrit-Change-Number: 29697
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: pespin 
Gerrit-Attention: pespin 
Gerrit-Comment-Date: Wed, 26 Oct 2022 16:13:44 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment

Change in osmo-msc[master]: vlr: auth_fsm: clarify success/failure result

2022-10-25 Thread laforge 
Attention is currently required from: neels.
laforge has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/29697 )

Change subject: vlr: auth_fsm: clarify success/failure result
..


Patch Set 1: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/29697
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I02776dfe6785983f2ebe398f57867f5ceb288ba0
Gerrit-Change-Number: 29697
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: pespin 
Gerrit-Attention: neels 
Gerrit-Comment-Date: Tue, 25 Oct 2022 15:42:39 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in osmo-msc[master]: vlr: auth_fsm: clarify success/failure result

2022-10-12 Thread pespin 
Attention is currently required from: neels.
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/29697 )

Change subject: vlr: auth_fsm: clarify success/failure result
..


Patch Set 1: Code-Review+1

(1 comment)

File include/osmocom/msc/vlr.h:

https://gerrit.osmocom.org/c/osmo-msc/+/29697/comment/b69e038c_a2fab734
PS1, Line 70:   VLR_ULA_E_AUTH_RES, /* Successful result of auth procedure 
*/
Can you then change this one to _SUCCESS?



--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/29697
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I02776dfe6785983f2ebe398f57867f5ceb288ba0
Gerrit-Change-Number: 29697
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin 
Gerrit-Attention: neels 
Gerrit-Comment-Date: Wed, 12 Oct 2022 13:51:09 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in osmo-msc[master]: vlr: auth_fsm: clarify success/failure result

2022-10-11 Thread Jenkins Builder 
Jenkins Builder has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/29697 )

Change subject: vlr: auth_fsm: clarify success/failure result
..


Patch Set 1: Verified+1

5 passed:
  [build] 
https://jenkins.osmocom.org/jenkins/job/gerrit-osmo-msc-build/IU=--enable-iu,WITH_MANUALS=0,a3=default,a4=default,label=osmocom-gerrit-debian9/5/consoleFull
  [build] 
https://jenkins.osmocom.org/jenkins/job/gerrit-osmo-msc-build/IU=--disable-iu,WITH_MANUALS=1,a3=default,a4=default,label=osmocom-gerrit-debian9/5/consoleFull
  [rpm] 
https://jenkins.osmocom.org/jenkins/job/gerrit-binpkgs-rpm/29/consoleFull
  [lint] https://jenkins.osmocom.org/jenkins/job/gerrit-lint/45/consoleFull
  [deb] 
https://jenkins.osmocom.org/jenkins/job/gerrit-binpkgs-deb/27/consoleFull

Build Successful


--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/29697
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I02776dfe6785983f2ebe398f57867f5ceb288ba0
Gerrit-Change-Number: 29697
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Comment-Date: Tue, 11 Oct 2022 22:33:47 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in osmo-msc[master]: vlr: auth_fsm: clarify success/failure result

2022-10-11 Thread neels 
neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/29697 )


Change subject: vlr: auth_fsm: clarify success/failure result
..

vlr: auth_fsm: clarify success/failure result

Explicitly send distinct parent events on auth success and failure. So
far determining success depended only on the data pointer passed on with
the event. Distinct events clarify the logging and the FSM code.

This prepares for a third FSM outcome to be added in a subsequent patch,
to separately signal when the HLR has no auth data.

No functional change.

Related: OS#4830
Change-Id: I02776dfe6785983f2ebe398f57867f5ceb288ba0
---
M include/osmocom/msc/vlr.h
M src/libvlr/vlr_access_req_fsm.c
M src/libvlr/vlr_auth_fsm.c
M src/libvlr/vlr_auth_fsm.h
M src/libvlr/vlr_lu_fsm.c
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
M tests/msc_vlr/msc_vlr_test_hlr_reject.err
M tests/msc_vlr/msc_vlr_test_umts_authen.err
8 files changed, 66 insertions(+), 36 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/97/29697/1

diff --git a/include/osmocom/msc/vlr.h b/include/osmocom/msc/vlr.h
index 8f2417a..2517693 100644
--- a/include/osmocom/msc/vlr.h
+++ b/include/osmocom/msc/vlr.h
@@ -67,7 +67,8 @@
VLR_ULA_E_UPDATE_LA,/* Initial trigger (LU from MS) */
VLR_ULA_E_SEND_ID_ACK,  /* Result of Send-ID from PVLR */
VLR_ULA_E_SEND_ID_NACK, /* Result of Send-ID from PVLR */
-   VLR_ULA_E_AUTH_RES, /* Result of auth procedure */
+   VLR_ULA_E_AUTH_RES, /* Successful result of auth procedure */
+   VLR_ULA_E_AUTH_FAILURE, /* Auth procedure failed */
VLR_ULA_E_CIPH_RES, /* Result of Ciphering Mode Command */
VLR_ULA_E_ID_IMSI,  /* IMSI received from MS */
VLR_ULA_E_ID_IMEI,  /* IMEI received from MS */
@@ -437,6 +438,7 @@
PR_ARQ_E_START,
PR_ARQ_E_ID_IMSI,
PR_ARQ_E_AUTH_RES,
+   PR_ARQ_E_AUTH_FAILURE,
PR_ARQ_E_CIPH_RES,
PR_ARQ_E_UPD_LOC_RES,
PR_ARQ_E_TRACE_RES,
diff --git a/src/libvlr/vlr_access_req_fsm.c b/src/libvlr/vlr_access_req_fsm.c
index 956b540..438e0d7 100644
--- a/src/libvlr/vlr_access_req_fsm.c
+++ b/src/libvlr/vlr_access_req_fsm.c
@@ -40,6 +40,7 @@
OSMO_VALUE_STRING(PR_ARQ_E_START),
OSMO_VALUE_STRING(PR_ARQ_E_ID_IMSI),
OSMO_VALUE_STRING(PR_ARQ_E_AUTH_RES),
+   OSMO_VALUE_STRING(PR_ARQ_E_AUTH_FAILURE),
OSMO_VALUE_STRING(PR_ARQ_E_CIPH_RES),
OSMO_VALUE_STRING(PR_ARQ_E_UPD_LOC_RES),
OSMO_VALUE_STRING(PR_ARQ_E_TRACE_RES),
@@ -340,6 +341,7 @@
0, 0);
vsub->auth_fsm = auth_fsm_start(vsub, fi,
PR_ARQ_E_AUTH_RES,
+   PR_ARQ_E_AUTH_FAILURE,
par->is_r99,
par->is_utran);
} else {
@@ -434,15 +436,19 @@
 {
enum gsm48_reject_value *cause = data;

-   OSMO_ASSERT(event == PR_ARQ_E_AUTH_RES);
+   switch (event) {
+   case PR_ARQ_E_AUTH_RES:
+   /* Node 2 */
+   _proc_arq_vlr_node2(fi);
+   return;

-   if (!cause || *cause) {
+   case PR_ARQ_E_AUTH_FAILURE:
proc_arq_fsm_done(fi, cause? *cause : 
GSM48_REJECT_NETWORK_FAILURE);
return;
-   }

-   /* Node 2 */
-   _proc_arq_vlr_node2(fi);
+   default:
+   OSMO_ASSERT(false);
+   }
 }
 
 static void proc_arq_vlr_fn_w_ciph(struct osmo_fsm_inst *fi,
@@ -547,7 +553,8 @@
},
[PR_ARQ_S_WAIT_AUTH] = {
.name = OSMO_STRINGIFY(PR_ARQ_S_WAIT_AUTH),
-   .in_event_mask = S(PR_ARQ_E_AUTH_RES),
+   .in_event_mask = S(PR_ARQ_E_AUTH_RES) |
+S(PR_ARQ_E_AUTH_FAILURE),
.out_state_mask = S(PR_ARQ_S_DONE) |
  S(PR_ARQ_S_WAIT_CIPH) |
  S(PR_ARQ_S_WAIT_UPD_LOC_CHILD) |
diff --git a/src/libvlr/vlr_auth_fsm.c b/src/libvlr/vlr_auth_fsm.c
index a6b5cd2..0583d7c 100644
--- a/src/libvlr/vlr_auth_fsm.c
+++ b/src/libvlr/vlr_auth_fsm.c
@@ -51,6 +51,9 @@
bool auth_requested;

int auth_tuple_max_reuse_count; /* see vlr->cfg instead */
+
+   uint32_t parent_event_success;
+   uint32_t parent_event_failure;
 };

 /***
@@ -240,14 +243,21 @@
 /* Terminate the Auth FSM Instance and notify parent */
 static void auth_fsm_term(struct osmo_fsm_inst *fi, enum gsm48_reject_value 
result)
 {
+   struct auth_fsm_priv *afp = fi->priv;
+
LOGPFSM(fi, "Authentication terminating with result %s\n",
vlr_auth_fsm_result_name(result));

-   /* Do one final state transition (mostly for logging purpose) */
-