Here is the fixed OAP patch set of those changes not merged yet.
For decode_big_endian(), see the new API comment and my mail from thursday night/friday morning.
Thanks for the review!
~Neels
Add new kitchen sink openbsc/utils.h and libcommon/utils.c to make three so far static functions public (so I can use them in the upcoming OAP code).
A place to put them could have been the gprs_utils.h, but all general functions in there have a gprs_ prefix, and todo markings to move them away. All other libcommon headers are too specific, so I opened up this kitchen sink header.
Replace the implementation of encode_big_endian() with a call to osmo_store64be_ext(). See comments.
Apply the change in Makefiles and C files. --- openbsc/include/openbsc/Makefile.am | 2 +- openbsc/include/openbsc/utils.h | 26 ++++++++++++++++ openbsc/src/gprs/gprs_gsup_messages.c | 30 +----------------- openbsc/src/libcommon/Makefile.am | 2 +- openbsc/src/libcommon/utils.c | 58 +++++++++++++++++++++++++++++++++++ openbsc/src/osmo-bsc_nat/bsc_nat.c | 12 +------- openbsc/tests/gprs/Makefile.am | 3 +- 7 files changed, 90 insertions(+), 43 deletions(-) create mode 100644 openbsc/include/openbsc/utils.h create mode 100644 openbsc/src/libcommon/utils.c
diff --git a/openbsc/include/openbsc/Makefile.am b/openbsc/include/openbsc/Makefile.am index 254f43d..828f5bd 100644 --- a/openbsc/include/openbsc/Makefile.am +++ b/openbsc/include/openbsc/Makefile.am @@ -14,7 +14,7 @@ noinst_HEADERS = abis_nm.h abis_rsl.h db.h gsm_04_08.h gsm_data.h \ osmo_msc_data.h osmo_bsc_grace.h sms_queue.h abis_om2000.h \ bss.h gsm_data_shared.h ipaccess.h mncc_int.h \ arfcn_range_encode.h nat_rewrite_trie.h bsc_nat_callstats.h \ - osmux.h mgcp_transcode.h gprs_utils.h \ + osmux.h mgcp_transcode.h gprs_utils.h utils.h \ gprs_gb_parse.h smpp.h meas_feed.h gprs_gsup_messages.h \ gprs_gsup_client.h bsc_msg_filter.h
diff --git a/openbsc/include/openbsc/utils.h b/openbsc/include/openbsc/utils.h new file mode 100644 index 0000000..d605487 --- /dev/null +++ b/openbsc/include/openbsc/utils.h @@ -0,0 +1,26 @@ +/* OpenBSC kitchen sink */ + +#pragma once + +#include <stdint.h> +#include <stdlib.h> + +/* Compare count bytes of exp to rel. Return 0 if they are identical, 1 + * otherwise. Do not return a mismatch on the first mismatching byte, + * but always compare all bytes, regardless. The idea is that the amount of + * matching bytes cannot be inferred from the time the comparison took.*/ +int constant_time_cmp(const uint8_t *exp, const uint8_t *rel, const int count); + +/* This is like osmo_load64be_ext, except that if data_len is less than + * sizeof(uint64_t), the data is interpreted as the least significant bytes + * (osmo_load64be_ext loads them as the most significant bytes into the + * returned uint64_t). In this way, any integer size up to 64 bits can be + * decoded conveniently by using sizeof(), without the need to call specific + * numbered functions (osmo_load16, 32, ...). */ +uint64_t decode_big_endian(const uint8_t *data, size_t data_len); + +/* This is like osmo_store64be_ext, except that this returns a static buffer of + * the result (for convenience, but not threadsafe). If data_len is less than + * sizeof(uint64_t), only the least significant bytes of value are encoded. */ +uint8_t *encode_big_endian(uint64_t value, size_t data_len); + diff --git a/openbsc/src/gprs/gprs_gsup_messages.c b/openbsc/src/gprs/gprs_gsup_messages.c index bdcff5f..07485f7 100644 --- a/openbsc/src/gprs/gprs_gsup_messages.c +++ b/openbsc/src/gprs/gprs_gsup_messages.c @@ -26,41 +26,13 @@
#include <openbsc/debug.h> #include <openbsc/gprs_utils.h> +#include <openbsc/utils.h>
#include <osmocom/gsm/tlv.h> #include <osmocom/core/msgb.h>
#include <stdint.h>
- -static uint64_t decode_big_endian(const uint8_t *data, size_t data_len) -{ - uint64_t value = 0; - - while (data_len > 0) { - value = (value << 8) + *data; - data += 1; - data_len -= 1; - } - - return value; -} - -static uint8_t *encode_big_endian(uint64_t value, size_t data_len) -{ - static uint8_t buf[sizeof(uint64_t)]; - int idx; - - OSMO_ASSERT(data_len <= ARRAY_SIZE(buf)); - - for (idx = data_len - 1; idx >= 0; idx--) { - buf[idx] = (uint8_t)value; - value = value >> 8; - } - - return buf; -} - static int decode_pdp_info(uint8_t *data, size_t data_len, struct gprs_gsup_pdp_info *pdp_info) { diff --git a/openbsc/src/libcommon/Makefile.am b/openbsc/src/libcommon/Makefile.am index 75f40ee..84c7544 100644 --- a/openbsc/src/libcommon/Makefile.am +++ b/openbsc/src/libcommon/Makefile.am @@ -6,4 +6,4 @@ noinst_LIBRARIES = libcommon.a
libcommon_a_SOURCES = bsc_version.c common_vty.c debug.c gsm_data.c \ gsm_data_shared.c socket.c talloc_ctx.c \ - gsm_subscriber_base.c + gsm_subscriber_base.c utils.c diff --git a/openbsc/src/libcommon/utils.c b/openbsc/src/libcommon/utils.c new file mode 100644 index 0000000..c47dcae --- /dev/null +++ b/openbsc/src/libcommon/utils.c @@ -0,0 +1,58 @@ +/* OpenBSC kitchen sink */ + +/* (C) 2015 by sysmocom s.m.f.c GmbH info@sysmocom.de + * All Rights Reserved + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + * + */ + +#include <openbsc/utils.h> +#include <osmocom/core/utils.h> +#include <osmocom/core/bit64gen.h> + +/* Wishful thinking to generate a constant time compare */ +int constant_time_cmp(const uint8_t *exp, const uint8_t *rel, const int count) +{ + int x = 0, i; + + for (i = 0; i < count; ++i) + x |= exp[i] ^ rel[i]; + + /* if x is zero, all data was identical */ + return x? 1 : 0; +} + + +uint64_t decode_big_endian(const uint8_t *data, size_t data_len) +{ + uint64_t value = 0; + + while (data_len > 0) { + value = (value << 8) + *data; + data += 1; + data_len -= 1; + } + + return value; +} + +uint8_t *encode_big_endian(uint64_t value, size_t data_len) +{ + static uint8_t buf[sizeof(uint64_t)]; + OSMO_ASSERT(data_len <= ARRAY_SIZE(buf)); + osmo_store64be_ext(value, buf, data_len); + return buf; +} + diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c b/openbsc/src/osmo-bsc_nat/bsc_nat.c index 56c6286..18343dd 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c @@ -47,6 +47,7 @@ #include <openbsc/abis_nm.h> #include <openbsc/socket.h> #include <openbsc/vty.h> +#include <openbsc/utils.h>
#include <osmocom/ctrl/control_cmd.h> #include <osmocom/ctrl/control_if.h> @@ -987,17 +988,6 @@ static void ipaccess_close_bsc(void *data) bsc_close_connection(conn); }
-/* Wishful thinking to generate a constant time compare */ -static int constant_time_cmp(const uint8_t *exp, const uint8_t *rel, const int count) -{ - int x = 0, i; - - for (i = 0; i < count; ++i) - x |= exp[i] ^ rel[i]; - - return x != 0; -} - static int verify_key(struct bsc_connection *conn, struct bsc_config *conf, const uint8_t *key, const int keylen) { struct osmo_auth_vector vec; diff --git a/openbsc/tests/gprs/Makefile.am b/openbsc/tests/gprs/Makefile.am index 633c362..b57977b 100644 --- a/openbsc/tests/gprs/Makefile.am +++ b/openbsc/tests/gprs/Makefile.am @@ -6,6 +6,7 @@ EXTRA_DIST = gprs_test.ok noinst_PROGRAMS = gprs_test
gprs_test_SOURCES = gprs_test.c $(top_srcdir)/src/gprs/gprs_utils.c \ - $(top_srcdir)/src/gprs/gprs_gsup_messages.c + $(top_srcdir)/src/gprs/gprs_gsup_messages.c \ + $(top_srcdir)/src/libcommon/utils.c
gprs_test_LDADD = $(LIBOSMOCORE_LIBS) $(LIBOSMOGSM_LIBS)
Add oap.[hc] and oap_messages.[hc].
Sponsored-by: On-Waves ehf --- openbsc/include/openbsc/Makefile.am | 3 +- openbsc/include/openbsc/oap.h | 78 ++++++++++ openbsc/include/openbsc/oap_messages.h | 70 +++++++++ openbsc/src/gprs/Makefile.am | 3 +- openbsc/src/gprs/oap.c | 256 +++++++++++++++++++++++++++++++++ openbsc/src/gprs/oap_messages.c | 178 +++++++++++++++++++++++ 6 files changed, 586 insertions(+), 2 deletions(-) create mode 100644 openbsc/include/openbsc/oap.h create mode 100644 openbsc/include/openbsc/oap_messages.h create mode 100644 openbsc/src/gprs/oap.c create mode 100644 openbsc/src/gprs/oap_messages.c
diff --git a/openbsc/include/openbsc/Makefile.am b/openbsc/include/openbsc/Makefile.am index 828f5bd..8a074c2 100644 --- a/openbsc/include/openbsc/Makefile.am +++ b/openbsc/include/openbsc/Makefile.am @@ -16,7 +16,8 @@ noinst_HEADERS = abis_nm.h abis_rsl.h db.h gsm_04_08.h gsm_data.h \ arfcn_range_encode.h nat_rewrite_trie.h bsc_nat_callstats.h \ osmux.h mgcp_transcode.h gprs_utils.h utils.h \ gprs_gb_parse.h smpp.h meas_feed.h gprs_gsup_messages.h \ - gprs_gsup_client.h bsc_msg_filter.h + gprs_gsup_client.h bsc_msg_filter.h \ + oap.h oap_messages.h
openbsc_HEADERS = gsm_04_08.h meas_rep.h bsc_api.h openbscdir = $(includedir)/openbsc diff --git a/openbsc/include/openbsc/oap.h b/openbsc/include/openbsc/oap.h new file mode 100644 index 0000000..2370cbe --- /dev/null +++ b/openbsc/include/openbsc/oap.h @@ -0,0 +1,78 @@ +/* Osmocom Authentication Protocol API */ + +/* (C) 2015 by Sysmocom s.f.m.c. GmbH + * All Rights Reserved + * + * Author: Neels Hofmeyr + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + * + */ + +#pragma once + +#include <stdint.h> + +struct msgb; +struct oap_message; + +/* This is the config part for vty. It is essentially copied in oap_state, + * where values are copied over once the config is considered valid. */ +struct oap_config { + uint16_t client_id; + int secret_k_present; + uint8_t secret_k[16]; + int secret_opc_present; + uint8_t secret_opc[16]; +}; + +/* The runtime state of the OAP client. client_id and the secrets are in fact + * duplicated from oap_config, so that a separate validation of the config data + * is possible, and so that only a struct oap_state* is passed around. */ +struct oap_state { + enum { + OAP_UNINITIALIZED = 0, /* just allocated. */ + OAP_DISABLED, /* disabled by config. */ + OAP_INITIALIZED, /* enabled, config is valid. */ + OAP_REQUESTED_CHALLENGE, + OAP_SENT_CHALLENGE_RESULT, + OAP_REGISTERED + } state; + uint16_t client_id; + uint8_t secret_k[16]; + uint8_t secret_opc[16]; + int registration_failures; +}; + +/* From config, initialize state. Return 0 on success. */ +int oap_init(struct oap_config *config, struct oap_state *state); + +/* Construct an OAP registration message and return in *msg_tx. Use + * state->client_id and update state->state. + * Return 0 on success, or a negative value on error. + * If an error is returned, *msg_tx is guaranteed to be NULL. */ +int oap_register(struct oap_state *state, struct msgb **msg_tx); + +/* Decode and act on a received OAP message msg_rx. Update state->state. If a + * non-NULL pointer is returned in *msg_tx, that msgb should be sent to the OAP + * server (and freed) by the caller. The received msg_rx is not freed. + * Return 0 on success, or a negative value on error. + * If an error is returned, *msg_tx is guaranteed to be NULL. */ +int oap_handle(struct oap_state *state, const struct msgb *msg_rx, struct msgb **msg_tx); + +/* Allocate a msgb and in it, return the encoded oap_msg. Return NULL on + * error. (Like oap_encode(), but also allocates a msgb.) + * About the name: the idea is do_something(oap_encoded(my_struct)) */ +struct msgb *oap_encoded(const struct oap_message *oap_msg); + diff --git a/openbsc/include/openbsc/oap_messages.h b/openbsc/include/openbsc/oap_messages.h new file mode 100644 index 0000000..a7a254c --- /dev/null +++ b/openbsc/include/openbsc/oap_messages.h @@ -0,0 +1,70 @@ +/* Osmocom Authentication Protocol message encoder/decoder */ + +/* (C) 2015 by Sysmocom s.f.m.c. GmbH + * All Rights Reserved + * + * Author: Neels Hofmeyr + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + * + */ +#pragma once + +#include <stdint.h> +#include <openbsc/gsm_04_08_gprs.h> +#include <openbsc/gsm_data.h> + +/* Some numbers are out of sequence because (so far) they match gprs_gsup_iei. + */ +enum oap_iei { + OAP_CAUSE_IE = 0x02, + OAP_RAND_IE = 0x20, + OAP_AUTN_IE = 0x23, + OAP_XRES_IE = 0x24, + OAP_AUTS_IE = 0x25, + OAP_CLIENT_ID_IE = 0x30, +}; + +enum oap_message_type { + OAP_MSGT_REGISTER_REQUEST = 0b00000100, + OAP_MSGT_REGISTER_ERROR = 0b00000101, + OAP_MSGT_REGISTER_RESULT = 0b00000110, + + OAP_MSGT_CHALLENGE_REQUEST = 0b00001000, + OAP_MSGT_CHALLENGE_ERROR = 0b00001001, + OAP_MSGT_CHALLENGE_RESULT = 0b00001010, + + OAP_MSGT_SYNC_REQUEST = 0b00001100, + OAP_MSGT_SYNC_ERROR = 0b00001101, + OAP_MSGT_SYNC_RESULT = 0b00001110, +}; + +struct oap_message { + enum oap_message_type message_type; + enum gsm48_gmm_cause cause; + uint16_t client_id; + int rand_present; + uint8_t rand[16]; + int autn_present; + uint8_t autn[16]; + int xres_present; + uint8_t xres[8]; + int auts_present; + uint8_t auts[16]; +}; + +int oap_decode(const uint8_t *data, size_t data_len, + struct oap_message *oap_msg); +void oap_encode(struct msgb *msg, const struct oap_message *oap_msg); + diff --git a/openbsc/src/gprs/Makefile.am b/openbsc/src/gprs/Makefile.am index f46a402..0952d41 100644 --- a/openbsc/src/gprs/Makefile.am +++ b/openbsc/src/gprs/Makefile.am @@ -27,7 +27,8 @@ osmo_sgsn_SOURCES = gprs_gmm.c gprs_sgsn.c gprs_sndcp.c gprs_sndcp_vty.c \ gprs_llc.c gprs_llc_parse.c gprs_llc_vty.c crc24.c \ sgsn_ctrl.c sgsn_auth.c gprs_subscriber.c \ gprs_gsup_messages.c gprs_utils.c gprs_gsup_client.c \ - gsm_04_08_gprs.c sgsn_cdr.c sgsn_ares.c + gsm_04_08_gprs.c sgsn_cdr.c sgsn_ares.c \ + oap.c oap_messages.c osmo_sgsn_LDADD = \ $(top_builddir)/src/libcommon/libcommon.a \ -lgtp $(OSMO_LIBS) $(LIBOSMOABIS_LIBS) $(LIBCARES_LIBS) -lrt diff --git a/openbsc/src/gprs/oap.c b/openbsc/src/gprs/oap.c new file mode 100644 index 0000000..1426702 --- /dev/null +++ b/openbsc/src/gprs/oap.c @@ -0,0 +1,256 @@ +/* Osmocom Authentication Protocol API */ + +/* (C) 2015 by Sysmocom s.f.m.c. GmbH + * All Rights Reserved + * + * Author: Neels Hofmeyr + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + * + */ + +#include <osmocom/crypt/auth.h> + +#include <openbsc/oap.h> +#include <openbsc/utils.h> +#include <openbsc/debug.h> +#include <openbsc/oap_messages.h> + +int oap_init(struct oap_config *config, struct oap_state *state) +{ + OSMO_ASSERT(state->state == OAP_UNINITIALIZED); + + if (config->client_id == 0) + goto disable; + + if (config->secret_k_present == 0) { + LOGP(DGPRS, LOGL_NOTICE, "OAP: client ID set, but secret K missing.\n"); + goto disable; + } + + if (config->secret_opc_present == 0) { + LOGP(DGPRS, LOGL_NOTICE, "OAP: client ID set, but secret OPC missing.\n"); + goto disable; + } + + state->client_id = config->client_id; + memcpy(state->secret_k, config->secret_k, sizeof(state->secret_k)); + memcpy(state->secret_opc, config->secret_opc, sizeof(state->secret_opc)); + state->state = OAP_INITIALIZED; + return 0; + +disable: + state->state = OAP_DISABLED; + return 0; +} + +/* From the given state and received RAND and AUTN octets, validate the + * server's authenticity and formulate the matching milenage reply octets in + * *tx_xres. The state is not modified. + * On success, and if tx_res is not NULL, exactly 8 octets will be written to + * *tx_res. If not NULL, tx_res must point at allocated memory of at least 8 + * octets. The caller will want to send XRES back to the server in a challenge + * response message and update the state. + * Return 0 on success; -1 if OAP is disabled; -2 if rx_random and rx_autn fail + * the authentication check; -3 for any other errors. */ +static int oap_evaluate_challenge(const struct oap_state *state, + const uint8_t *rx_random, + const uint8_t *rx_autn, + uint8_t *tx_xres) +{ + osmo_static_assert(sizeof(((struct osmo_sub_auth_data*)0)->u.umts.k) + == sizeof(state->secret_k), _secret_k_size_match); + osmo_static_assert(sizeof(((struct osmo_sub_auth_data*)0)->u.umts.opc) + == sizeof(state->secret_opc), _secret_opc_size_match); + + switch(state->state) { + case OAP_UNINITIALIZED: + case OAP_DISABLED: + return -1; + default: + break; + } + + struct osmo_auth_vector vec; + + struct osmo_sub_auth_data auth = { + .type = OSMO_AUTH_TYPE_UMTS, + .algo = OSMO_AUTH_ALG_MILENAGE, + }; + + memcpy(auth.u.umts.k, state->secret_k, sizeof(auth.u.umts.k)); + memcpy(auth.u.umts.opc, state->secret_opc, sizeof(auth.u.umts.opc)); + memset(auth.u.umts.amf, '\0', sizeof(auth.u.umts.amf)); + auth.u.umts.sqn = 42; /* TODO use incrementing sequence nr */ + + memset(&vec, 0, sizeof(vec)); + osmo_auth_gen_vec(&vec, &auth, rx_random); + + if (vec.res_len != 8) { + LOGP(DGPRS, LOGL_ERROR, "OAP: Expected XRES to be 8 octets, got %d\n", + vec.res_len); + return -3; + } + + if (constant_time_cmp(vec.autn, rx_autn, sizeof(vec.autn)) != 0) { + LOGP(DGPRS, LOGL_ERROR, "OAP: AUTN mismatch!\n"); + LOGP(DGPRS, LOGL_INFO, "OAP: AUTN from server: %s\n", + osmo_hexdump_nospc(rx_autn, sizeof(vec.autn))); + LOGP(DGPRS, LOGL_INFO, "OAP: AUTN expected: %s\n", + osmo_hexdump_nospc(vec.autn, sizeof(vec.autn))); + return -2; + } + + if (tx_xres != NULL) + memcpy(tx_xres, vec.res, 8); + return 0; +} + +struct msgb *oap_encoded(const struct oap_message *oap_msg) +{ + struct msgb *msg = msgb_alloc_headroom(1000, 64, __func__); + OSMO_ASSERT(msg); + oap_encode(msg, oap_msg); + return msg; +} + +/* Create a new msgb containing an OAP registration message. + * On error, return NULL. */ +static struct msgb* oap_msg_register(uint16_t client_id) +{ + if (client_id < 1) { + LOGP(DGPRS, LOGL_ERROR, "OAP: Invalid client ID: %d\n", client_id); + return NULL; + } + + struct oap_message oap_msg = {0}; + oap_msg.message_type = OAP_MSGT_REGISTER_REQUEST; + oap_msg.client_id = client_id; + return oap_encoded(&oap_msg); +} + +int oap_register(struct oap_state *state, struct msgb **msg_tx) +{ + *msg_tx = oap_msg_register(state->client_id); + if (!(*msg_tx)) + return -1; + + state->state = OAP_REQUESTED_CHALLENGE; + return 0; +} + +/* Create a new msgb containing an OAP challenge response message. + * xres must point at 8 octets to return as challenge response. + * On error, return NULL. */ +static struct msgb* oap_msg_challenge_response(uint8_t *xres) +{ + struct oap_message oap_reply = {0}; + + oap_reply.message_type = OAP_MSGT_CHALLENGE_RESULT; + memcpy(oap_reply.xres, xres, sizeof(oap_reply.xres)); + oap_reply.xres_present = 1; + return oap_encoded(&oap_reply); +} + +static int handle_challenge(struct oap_state *state, + struct oap_message *oap_rx, + struct msgb **msg_tx) +{ + int rc; + if (!(oap_rx->rand_present && oap_rx->autn_present)) { + LOGP(DGPRS, LOGL_ERROR, + "OAP challenge incomplete (rand_present: %d, autn_present: %d)\n", + oap_rx->rand_present, oap_rx->autn_present); + rc = -2; + goto failure; + } + + uint8_t xres[8]; + rc = oap_evaluate_challenge(state, + oap_rx->rand, + oap_rx->autn, + xres); + if (rc < 0) + goto failure; + + *msg_tx = oap_msg_challenge_response(xres); + if ((*msg_tx) == NULL) { + rc = -1; + goto failure; + } + + state->state = OAP_SENT_CHALLENGE_RESULT; + return 0; + +failure: + OSMO_ASSERT(rc < 0); + state->state = OAP_INITIALIZED; + return rc; +} + +int oap_handle(struct oap_state *state, const struct msgb *msg_rx, struct msgb **msg_tx) +{ + *msg_tx = NULL; + + uint8_t *data = msgb_l2(msg_rx); + size_t data_len = msgb_l2len(msg_rx); + int rc = 0; + + struct oap_message oap_msg = {0}; + + OSMO_ASSERT(data); + + rc = oap_decode(data, data_len, &oap_msg); + if (rc < 0) { + LOGP(DGPRS, LOGL_ERROR, + "Decoding OAP message failed with error '%s' (%d)\n", + get_value_string(gsm48_gmm_cause_names, -rc), -rc); + return -10; + } + + switch (oap_msg.message_type) { + case OAP_MSGT_CHALLENGE_REQUEST: + return handle_challenge(state, &oap_msg, msg_tx); + + case OAP_MSGT_REGISTER_RESULT: + /* successfully registered */ + state->state = OAP_REGISTERED; + break; + + case OAP_MSGT_REGISTER_ERROR: + LOGP(DGPRS, LOGL_ERROR, + "OAP registration failed\n"); + state->state = OAP_INITIALIZED; + if (state->registration_failures < 3) { + state->registration_failures ++; + return oap_register(state, msg_tx); + } + return -11; + + case OAP_MSGT_REGISTER_REQUEST: + case OAP_MSGT_CHALLENGE_RESULT: + LOGP(DGPRS, LOGL_ERROR, + "Received invalid OAP message type for OAP client side: %d\n", + (int)oap_msg.message_type); + return -12; + + default: + LOGP(DGPRS, LOGL_ERROR, + "Unknown OAP message type: %d\n", + (int)oap_msg.message_type); + return -13; + } + + return 0; +} diff --git a/openbsc/src/gprs/oap_messages.c b/openbsc/src/gprs/oap_messages.c new file mode 100644 index 0000000..eb52053 --- /dev/null +++ b/openbsc/src/gprs/oap_messages.c @@ -0,0 +1,178 @@ +/* Osmocom Authentication Protocol message encoder/decoder */ + +/* (C) 2015 by Sysmocom s.f.m.c. GmbH + * All Rights Reserved + * + * Author: Neels Hofmeyr + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + * + */ + +#include <openbsc/oap_messages.h> + +#include <openbsc/debug.h> +#include <openbsc/gprs_utils.h> +#include <openbsc/utils.h> + +#include <osmocom/gsm/tlv.h> +#include <osmocom/core/msgb.h> + +#include <stdint.h> + + +int oap_decode(const uint8_t *const_data, size_t data_len, + struct oap_message *oap_msg) +{ + int rc; + uint8_t tag; + /* the shift/match functions expect non-const pointers, but we'll + * either copy the data or cast pointers back to const before returning + * them + */ + uint8_t *data = (uint8_t *)const_data; + uint8_t *value; + size_t value_len; + + memset(oap_msg, 0, sizeof(*oap_msg)); + + /* message type */ + rc = gprs_shift_v_fixed(&data, &data_len, 1, &value); + if (rc < 0) + return -GMM_CAUSE_INV_MAND_INFO; + oap_msg->message_type = decode_big_endian(value, 1); + + /* specific parts */ + while (data_len > 0) { + enum oap_iei iei; + + rc = gprs_shift_tlv(&data, &data_len, &tag, &value, &value_len); + if (rc < 0) + return -GMM_CAUSE_PROTO_ERR_UNSPEC; + + iei = tag; + + switch (iei) { + case OAP_CLIENT_ID_IE: + if (value_len != 2) { + LOGP(DGPRS, LOGL_NOTICE, + "OAP IE type client ID (%d) should be 2 octets, but has %d\n", + (int)iei, (int)value_len); + return -GMM_CAUSE_PROTO_ERR_UNSPEC; + } + + oap_msg->client_id = decode_big_endian(value, value_len); + + if (oap_msg->client_id == 0) { + LOGP(DGPRS, LOGL_NOTICE, + "OAP IE type client ID (%d): client ID must be nonzero.\n", + (int)iei); + return -GMM_CAUSE_PROTO_ERR_UNSPEC; + } + break; + + case OAP_AUTN_IE: + if (value_len != sizeof(oap_msg->autn)) { + LOGP(DGPRS, LOGL_NOTICE, + "OAP IE type AUTN (%d) should be %d octets, but has %d\n", + (int)iei, (int)sizeof(oap_msg->autn), (int)value_len); + return -GMM_CAUSE_PROTO_ERR_UNSPEC; + } + memcpy(oap_msg->autn, value, value_len); + oap_msg->autn_present = value_len; + break; + + case OAP_RAND_IE: + if (value_len != sizeof(oap_msg->rand)) { + LOGP(DGPRS, LOGL_NOTICE, + "OAP IE type RAND (%d) should be %d octets, but has %d\n", + (int)iei, (int)sizeof(oap_msg->rand), (int)value_len); + return -GMM_CAUSE_PROTO_ERR_UNSPEC; + } + memcpy(oap_msg->rand, value, value_len); + oap_msg->rand_present = value_len; + break; + + case OAP_XRES_IE: + if (value_len != sizeof(oap_msg->xres)) { + LOGP(DGPRS, LOGL_NOTICE, + "OAP IE type XRES (%d) should be %d octets, but has %d\n", + (int)iei, (int)sizeof(oap_msg->xres), (int)value_len); + return -GMM_CAUSE_PROTO_ERR_UNSPEC; + } + memcpy(oap_msg->xres, value, value_len); + oap_msg->xres_present = value_len; + break; + + case OAP_AUTS_IE: + if (value_len != sizeof(oap_msg->auts)) { + LOGP(DGPRS, LOGL_NOTICE, + "OAP IE type AUTS (%d) should be %d octets, but has %d\n", + (int)iei, (int)sizeof(oap_msg->auts), (int)value_len); + return -GMM_CAUSE_PROTO_ERR_UNSPEC; + } + memcpy(oap_msg->auts, value, value_len); + oap_msg->auts_present = value_len; + break; + + case OAP_CAUSE_IE: + if (value_len > 1) { + LOGP(DGPRS, LOGL_ERROR, + "OAP cause may not exceed one octet, is %d", (int)value_len); + return -GMM_CAUSE_PROTO_ERR_UNSPEC; + } + oap_msg->cause = *value; + break; + + default: + LOGP(DGPRS, LOGL_NOTICE, + "OAP IE type %d unknown\n", iei); + continue; + } + } + + return 0; +} + +void oap_encode(struct msgb *msg, const struct oap_message *oap_msg) +{ + uint8_t u8; + + /* generic part */ + OSMO_ASSERT(oap_msg->message_type); + msgb_v_put(msg, (uint8_t)oap_msg->message_type); + + /* specific parts */ + if ((u8 = oap_msg->cause)) + msgb_tlv_put(msg, OAP_CAUSE_IE, sizeof(u8), &u8); + + if (oap_msg->client_id > 0) + msgb_tlv_put(msg, OAP_CLIENT_ID_IE, sizeof(oap_msg->client_id), + encode_big_endian(oap_msg->client_id, sizeof(oap_msg->client_id))); + + if (oap_msg->rand_present) + msgb_tlv_put(msg, OAP_RAND_IE, sizeof(oap_msg->rand), oap_msg->rand); + + if (oap_msg->autn_present) + msgb_tlv_put(msg, OAP_AUTN_IE, sizeof(oap_msg->autn), oap_msg->autn); + + if (oap_msg->auts_present) + msgb_tlv_put(msg, OAP_AUTS_IE, sizeof(oap_msg->auts), oap_msg->auts); + + if (oap_msg->xres_present) + msgb_tlv_put(msg, OAP_XRES_IE, sizeof(oap_msg->xres), oap_msg->xres); + + msg->l2h = msg->data; +} +
Sponsored-by: On-Waves ehf --- openbsc/.gitignore | 1 + openbsc/configure.ac | 1 + openbsc/tests/Makefile.am | 2 +- openbsc/tests/oap/Makefile.am | 19 ++++++++++++++ openbsc/tests/oap/oap_test.c | 58 +++++++++++++++++++++++++++++++++++++++++++ openbsc/tests/oap/oap_test.ok | 2 ++ openbsc/tests/testsuite.at | 7 ++++++ 7 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 openbsc/tests/oap/Makefile.am create mode 100644 openbsc/tests/oap/oap_test.c create mode 100644 openbsc/tests/oap/oap_test.ok
diff --git a/openbsc/.gitignore b/openbsc/.gitignore index 2210c47..ca73db6 100644 --- a/openbsc/.gitignore +++ b/openbsc/.gitignore @@ -77,6 +77,7 @@ tests/trau/trau_test tests/mgcp/mgcp_transcoding_test tests/sgsn/sgsn_test tests/subscr/subscr_test +tests/oap/oap_test
tests/atconfig tests/atlocal diff --git a/openbsc/configure.ac b/openbsc/configure.ac index fc30b5e..8b7ce62 100644 --- a/openbsc/configure.ac +++ b/openbsc/configure.ac @@ -209,6 +209,7 @@ AC_OUTPUT( tests/trau/Makefile tests/sgsn/Makefile tests/subscr/Makefile + tests/oap/Makefile doc/Makefile doc/examples/Makefile Makefile) diff --git a/openbsc/tests/Makefile.am b/openbsc/tests/Makefile.am index 773830b..1b557d4 100644 --- a/openbsc/tests/Makefile.am +++ b/openbsc/tests/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = gsm0408 db channel mgcp gprs abis gbproxy trau subscr +SUBDIRS = gsm0408 db channel mgcp gprs abis gbproxy trau subscr oap
if BUILD_NAT SUBDIRS += bsc-nat bsc-nat-trie diff --git a/openbsc/tests/oap/Makefile.am b/openbsc/tests/oap/Makefile.am new file mode 100644 index 0000000..e160902 --- /dev/null +++ b/openbsc/tests/oap/Makefile.am @@ -0,0 +1,19 @@ +AM_CPPFLAGS = $(all_includes) -I$(top_srcdir)/include +AM_CFLAGS=-Wall -ggdb3 $(LIBOSMOCORE_CFLAGS) $(LIBOSMOGSM_CFLAGS) + +EXTRA_DIST = oap_test.ok + +noinst_PROGRAMS = oap_test + +oap_test_SOURCES = oap_test.c + +oap_test_LDADD = \ + $(top_builddir)/src/gprs/oap.o \ + $(top_builddir)/src/gprs/oap_messages.o \ + $(top_builddir)/src/gprs/gprs_utils.o \ + $(top_builddir)/src/gprs/gsm_04_08_gprs.o \ + $(top_builddir)/src/libcommon/libcommon.a \ + $(LIBOSMOCORE_LIBS) \ + $(LIBOSMOGSM_LIBS) \ + -lrt + diff --git a/openbsc/tests/oap/oap_test.c b/openbsc/tests/oap/oap_test.c new file mode 100644 index 0000000..b03c8dc --- /dev/null +++ b/openbsc/tests/oap/oap_test.c @@ -0,0 +1,58 @@ +/* Test Osmocom Authentication Protocol */ +/* + * (C) 2015 by sysmocom s.f.m.c. GmbH + * All Rights Reserved + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + * + */ + +#include <osmocom/core/application.h> + +#include <openbsc/debug.h> + +#include <openbsc/oap.h> +#include <openbsc/oap_messages.h> + +#include <stdio.h> + + +static void test_oap(void) +{ + printf("not implemented\n"); +} + +static struct log_info_cat gprs_categories[] = { + [DGPRS] = { + .name = "DGPRS", + .description = "GPRS Packet Service", + .enabled = 1, .loglevel = LOGL_DEBUG, + }, +}; + +static struct log_info info = { + .cat = gprs_categories, + .num_cat = ARRAY_SIZE(gprs_categories), +}; + +int main(int argc, char **argv) +{ + osmo_init_logging(&info); + + test_oap(); + printf("Done\n"); + + return 0; +} + diff --git a/openbsc/tests/oap/oap_test.ok b/openbsc/tests/oap/oap_test.ok new file mode 100644 index 0000000..e411ad9 --- /dev/null +++ b/openbsc/tests/oap/oap_test.ok @@ -0,0 +1,2 @@ +not implemented +Done diff --git a/openbsc/tests/testsuite.at b/openbsc/tests/testsuite.at index 74aaef0..78aa47e 100644 --- a/openbsc/tests/testsuite.at +++ b/openbsc/tests/testsuite.at @@ -103,3 +103,10 @@ AT_CHECK([test "$enable_sgsn_test" != no || exit 77]) cat $abs_srcdir/sgsn/sgsn_test.ok > expout AT_CHECK([$abs_top_builddir/tests/sgsn/sgsn_test], [], [expout], [ignore]) AT_CLEANUP + +AT_SETUP([oap]) +AT_KEYWORDS([oap]) +AT_CHECK([test "$enable_oap_test" != no || exit 77]) +cat $abs_srcdir/oap/oap_test.ok > expout +AT_CHECK([$abs_top_builddir/tests/oap/oap_test], [], [expout], [ignore]) +AT_CLEANUP
Sponsored-by: On-Waves ehf --- openbsc/tests/oap/oap_test.c | 193 +++++++++++++++++++++++++++++++++++++++++- openbsc/tests/oap/oap_test.ok | 7 +- 2 files changed, 195 insertions(+), 5 deletions(-)
diff --git a/openbsc/tests/oap/oap_test.c b/openbsc/tests/oap/oap_test.c index b03c8dc..9a2063b 100644 --- a/openbsc/tests/oap/oap_test.c +++ b/openbsc/tests/oap/oap_test.c @@ -27,10 +27,195 @@
#include <stdio.h>
- -static void test_oap(void) +static void test_oap_api(void) { - printf("not implemented\n"); + printf("Testing OAP API\n - Config parsing\n"); + + struct oap_config _config; + struct oap_config *config = &_config; + + struct oap_state _state; + struct oap_state *state = &_state; + + memset(config, 0, sizeof(*config)); + memset(state, 0, sizeof(*state)); + + OSMO_ASSERT(osmo_hexparse("0102030405060708090a0b0c0d0e0f10", config->secret_k, 16) == 16); + OSMO_ASSERT(osmo_hexparse("1112131415161718191a1b1c1d1e1f20", config->secret_opc, 16) == 16); + + /* make sure filling with zeros means uninitialized */ + OSMO_ASSERT(state->state == OAP_UNINITIALIZED); + + /* invalid client_id and shared secret */ + config->client_id = 0; + config->secret_k_present = 0; + config->secret_opc_present = 0; + OSMO_ASSERT( oap_init(config, state) == 0 ); + OSMO_ASSERT(state->state == OAP_DISABLED); + + /* reset state */ + memset(state, 0, sizeof(*state)); + + /* only client_id is invalid */ + config->client_id = 0; + config->secret_k_present = 1; + config->secret_opc_present = 1; + OSMO_ASSERT( oap_init(config, state) == 0 ); + OSMO_ASSERT(state->state == OAP_DISABLED); + + memset(state, 0, sizeof(*state)); + + /* valid id, but omitted shared_secret (1/2) */ + config->client_id = 12345; + config->secret_k_present = 0; + config->secret_opc_present = 1; + OSMO_ASSERT( oap_init(config, state) == 0 ); + OSMO_ASSERT(state->state == OAP_DISABLED); + + memset(state, 0, sizeof(*state)); + + /* valid id, but omitted shared_secret (2/2) */ + config->client_id = 12345; + config->secret_k_present = 1; + config->secret_opc_present = 0; + OSMO_ASSERT( oap_init(config, state) == 0 ); + OSMO_ASSERT(state->state == OAP_DISABLED); + + memset(state, 0, sizeof(*state)); + + + /* mint configuration */ + config->client_id = 12345; + config->secret_k_present = 1; + config->secret_opc_present = 1; + /*config->secret_* buffers are still set from the top */ + OSMO_ASSERT( oap_init(config, state) == 0 ); + OSMO_ASSERT(state->state == OAP_INITIALIZED); + + printf(" - AUTN failures\n"); + + struct oap_message oap_rx; + struct oap_message oap_tx; + struct msgb *msg_rx; + struct msgb *msg_tx; + + memset(&oap_rx, 0, sizeof(oap_rx)); + + /* Missing challenge data */ + oap_rx.message_type = OAP_MSGT_CHALLENGE_REQUEST; + oap_rx.rand_present = 0; + oap_rx.autn_present = 0; + msg_rx = oap_encoded(&oap_rx); + OSMO_ASSERT(oap_handle(state, msg_rx, &msg_tx) == -2); + msgb_free(msg_rx); + OSMO_ASSERT(!msg_tx); + + /* AUTN missing */ + osmo_hexparse("0102030405060708090a0b0c0d0e0f10", + oap_rx.rand, 16); + oap_rx.rand_present = 1; + msg_rx = oap_encoded(&oap_rx); + OSMO_ASSERT(oap_handle(state, msg_rx, &msg_tx) == -2); + msgb_free(msg_rx); + OSMO_ASSERT(!msg_tx); + + /* RAND missing */ + oap_rx.rand_present = 0; + osmo_hexparse("cec4e3848a33000086781158ca40f136", + oap_rx.autn, 16); + oap_rx.autn_present = 1; + msg_rx = oap_encoded(&oap_rx); + OSMO_ASSERT(oap_handle(state, msg_rx, &msg_tx) == -2); + msgb_free(msg_rx); + OSMO_ASSERT(!msg_tx); + + /* wrong autn (by one bit) */ + osmo_hexparse("0102030405060708090a0b0c0d0e0f10", + oap_rx.rand, 16); + osmo_hexparse("dec4e3848a33000086781158ca40f136", + oap_rx.autn, 16); + oap_rx.rand_present = 1; + oap_rx.autn_present = 1; + msg_rx = oap_encoded(&oap_rx); + OSMO_ASSERT(oap_handle(state, msg_rx, &msg_tx) == -2); + msgb_free(msg_rx); + OSMO_ASSERT(!msg_tx); + + /* all data correct */ + osmo_hexparse("cec4e3848a33000086781158ca40f136", + oap_rx.autn, 16); + msg_rx = oap_encoded(&oap_rx); + + /* but refuse to evaluate in uninitialized state */ + OSMO_ASSERT(state->state == OAP_INITIALIZED); + + state->state = OAP_UNINITIALIZED; + OSMO_ASSERT(oap_handle(state, msg_rx, &msg_tx) == -1); + OSMO_ASSERT(!msg_tx); + + state->state = OAP_DISABLED; + OSMO_ASSERT(oap_handle(state, msg_rx, &msg_tx) == -1); + OSMO_ASSERT(!msg_tx); + + state->state = OAP_INITIALIZED; + + /* now everything is correct */ + printf(" - AUTN success\n"); + /* a successful return value here indicates correct autn */ + OSMO_ASSERT(oap_handle(state, msg_rx, &msg_tx) == 0); + msgb_free(msg_rx); + + /* Expect the challenge response in msg_tx */ + OSMO_ASSERT(msg_tx); + OSMO_ASSERT(oap_decode(msg_tx->data, msg_tx->len, &oap_tx) == 0); + OSMO_ASSERT(oap_tx.message_type == OAP_MSGT_CHALLENGE_RESULT); + OSMO_ASSERT(strcmp("e2d05b598c61d9ba", + osmo_hexdump_nospc(oap_tx.xres, sizeof(oap_tx.xres))) + == 0); + OSMO_ASSERT(state->state == OAP_SENT_CHALLENGE_RESULT); + msgb_free(msg_tx); + msg_tx = 0; + + struct oap_state saved_state = _state; + + printf(" - Registration failure\n"); + + memset(&oap_rx, 0, sizeof(oap_rx)); + oap_rx.message_type = OAP_MSGT_REGISTER_ERROR; + oap_rx.cause = GMM_CAUSE_PROTO_ERR_UNSPEC; + msg_rx = oap_encoded(&oap_rx); + + /* Receive registration error for the first time. */ + OSMO_ASSERT(state->registration_failures == 0); + OSMO_ASSERT(oap_handle(state, msg_rx, &msg_tx) == 0); + OSMO_ASSERT(state->registration_failures == 1); + OSMO_ASSERT(msg_tx); + OSMO_ASSERT(oap_decode(msg_tx->data, msg_tx->len, &oap_tx) == 0); + OSMO_ASSERT(oap_tx.message_type == OAP_MSGT_REGISTER_REQUEST); + OSMO_ASSERT(state->state == OAP_REQUESTED_CHALLENGE); + msgb_free(msg_tx); + msg_tx = 0; + + /* Receive registration error for the Nth time. */ + state->registration_failures = 999; + OSMO_ASSERT(oap_handle(state, msg_rx, &msg_tx) == -11); + OSMO_ASSERT(!msg_tx); + OSMO_ASSERT(state->state == OAP_INITIALIZED); + msgb_free(msg_tx); + msg_tx = 0; + + msgb_free(msg_rx); + + printf(" - Registration success\n"); + + _state = saved_state; + memset(&oap_rx, 0, sizeof(oap_rx)); + oap_rx.message_type = OAP_MSGT_REGISTER_RESULT; + msg_rx = oap_encoded(&oap_rx); + OSMO_ASSERT(oap_handle(state, msg_rx, &msg_tx) == 0); + OSMO_ASSERT(!msg_tx); + OSMO_ASSERT(state->state == OAP_REGISTERED); + msgb_free(msg_rx); }
static struct log_info_cat gprs_categories[] = { @@ -50,7 +235,7 @@ int main(int argc, char **argv) { osmo_init_logging(&info);
- test_oap(); + test_oap_api(); printf("Done\n");
return 0; diff --git a/openbsc/tests/oap/oap_test.ok b/openbsc/tests/oap/oap_test.ok index e411ad9..2e9ecf0 100644 --- a/openbsc/tests/oap/oap_test.ok +++ b/openbsc/tests/oap/oap_test.ok @@ -1,2 +1,7 @@ -not implemented +Testing OAP API + - Config parsing + - AUTN failures + - AUTN success + - Registration failure + - Registration success Done
Trigger an OAP registration upon IPA connect. Feed incoming OAP messages to oap_handle() and send replies returned by it.
Add oap_config to sgsn_config (todo: vty).
Sponsored-by: On-Waves ehf --- openbsc/include/openbsc/gprs_gsup_client.h | 7 +++- openbsc/include/openbsc/sgsn.h | 3 ++ openbsc/src/gprs/gprs_gsup_client.c | 58 ++++++++++++++++++++++++++---- openbsc/src/gprs/gprs_subscriber.c | 3 +- openbsc/tests/sgsn/Makefile.am | 2 ++ 5 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/openbsc/include/openbsc/gprs_gsup_client.h b/openbsc/include/openbsc/gprs_gsup_client.h index 9537db4..ccfa388 100644 --- a/openbsc/include/openbsc/gprs_gsup_client.h +++ b/openbsc/include/openbsc/gprs_gsup_client.h @@ -23,6 +23,8 @@
#include <osmocom/core/timer.h>
+#include <openbsc/oap.h> + #define GPRS_GSUP_RECONNECT_INTERVAL 10 #define GPRS_GSUP_PING_INTERVAL 20
@@ -38,6 +40,8 @@ struct gprs_gsup_client { gprs_gsup_read_cb_t read_cb; void *data;
+ struct oap_state oap_state; + struct osmo_timer_list ping_timer; struct osmo_timer_list connect_timer; int is_connected; @@ -46,7 +50,8 @@ struct gprs_gsup_client {
struct gprs_gsup_client *gprs_gsup_client_create(const char *ip_addr, unsigned int tcp_port, - gprs_gsup_read_cb_t read_cb); + gprs_gsup_read_cb_t read_cb, + struct oap_config *oap_config);
void gprs_gsup_client_destroy(struct gprs_gsup_client *gsupc); int gprs_gsup_client_send(struct gprs_gsup_client *gsupc, struct msgb *msg); diff --git a/openbsc/include/openbsc/sgsn.h b/openbsc/include/openbsc/sgsn.h index d4f9913..2b1b97c 100644 --- a/openbsc/include/openbsc/sgsn.h +++ b/openbsc/include/openbsc/sgsn.h @@ -6,6 +6,7 @@
#include <osmocom/gprs/gprs_ns.h> #include <openbsc/gprs_sgsn.h> +#include <openbsc/oap.h>
#include <ares.h>
@@ -61,6 +62,8 @@ struct sgsn_config { } timers;
int dynamic_lookup; + + struct oap_config oap; };
struct sgsn_instance { diff --git a/openbsc/src/gprs/gprs_gsup_client.c b/openbsc/src/gprs/gprs_gsup_client.c index 1f9e34c..a332fa7 100644 --- a/openbsc/src/gprs/gprs_gsup_client.c +++ b/openbsc/src/gprs/gprs_gsup_client.c @@ -108,6 +108,20 @@ static void gsup_client_send(struct gprs_gsup_client *gsupc, int proto_ext, stru /* msg_tx is now queued and will be freed. */ }
+static void gsup_client_oap_register(struct gprs_gsup_client *gsupc) +{ + struct msgb *msg_tx; + int rc; + rc = oap_register(&gsupc->oap_state, &msg_tx); + + if ((rc < 0) || (!msg_tx)) { + LOGP(DGPRS, LOGL_ERROR, "GSUP OAP set up, but cannot register.\n"); + return; + } + + gsup_client_send(gsupc, IPAC_PROTO_EXT_OAP, msg_tx); +} + static void gsup_client_updown_cb(struct ipa_client_conn *link, int up) { struct gprs_gsup_client *gsupc = link->data; @@ -120,6 +134,9 @@ static void gsup_client_updown_cb(struct ipa_client_conn *link, int up) if (up) { start_test_procedure(gsupc);
+ if (gsupc->oap_state.state == OAP_INITIALIZED) + gsup_client_oap_register(gsupc); + osmo_timer_del(&gsupc->connect_timer); } else { osmo_timer_del(&gsupc->ping_timer); @@ -129,6 +146,22 @@ static void gsup_client_updown_cb(struct ipa_client_conn *link, int up) } }
+static int gsup_client_oap_handle(struct gprs_gsup_client *gsupc, struct msgb *msg_rx) +{ + int rc; + struct msgb *msg_tx; + + rc = oap_handle(&gsupc->oap_state, msg_rx, &msg_tx); + msgb_free(msg_rx); + if (rc < 0) + return rc; + + if (msg_tx) + gsup_client_send(gsupc, IPAC_PROTO_EXT_OAP, msg_tx); + + return 0; +} + static int gsup_client_read_cb(struct ipa_client_conn *link, struct msgb *msg) { struct ipaccess_head *hh = (struct ipaccess_head *) msg->data; @@ -168,16 +201,24 @@ static int gsup_client_read_cb(struct ipa_client_conn *link, struct msgb *msg) if (hh->proto != IPAC_PROTO_OSMO) goto invalid;
- if (!he || msgb_l2len(msg) < sizeof(*he) || - he->proto != IPAC_PROTO_EXT_GSUP) + if (!he || msgb_l2len(msg) < sizeof(*he)) goto invalid;
msg->l2h = &he->data[0];
- OSMO_ASSERT(gsupc->read_cb != NULL); - gsupc->read_cb(gsupc, msg); + if (he->proto == IPAC_PROTO_EXT_GSUP) { + OSMO_ASSERT(gsupc->read_cb != NULL); + gsupc->read_cb(gsupc, msg); + /* expecting read_cb() to free msg */ + } + else + if (he->proto == IPAC_PROTO_EXT_OAP) { + return gsup_client_oap_handle(gsupc, msg); + /* gsup_client_oap_handle frees msg */ + } + else + goto invalid;
- /* Not freeing msg here, because that must be done by the read_cb. */ return 0;
invalid: @@ -222,7 +263,8 @@ static void start_test_procedure(struct gprs_gsup_client *gsupc)
struct gprs_gsup_client *gprs_gsup_client_create(const char *ip_addr, unsigned int tcp_port, - gprs_gsup_read_cb_t read_cb) + gprs_gsup_read_cb_t read_cb, + struct oap_config *oap_config) { struct gprs_gsup_client *gsupc; int rc; @@ -230,6 +272,10 @@ struct gprs_gsup_client *gprs_gsup_client_create(const char *ip_addr, gsupc = talloc_zero(tall_bsc_ctx, struct gprs_gsup_client); OSMO_ASSERT(gsupc);
+ rc = oap_init(oap_config, &gsupc->oap_state); + if (rc != 0) + goto failed; + gsupc->link = ipa_client_conn_create(gsupc, /* no e1inp */ NULL, 0, diff --git a/openbsc/src/gprs/gprs_subscriber.c b/openbsc/src/gprs/gprs_subscriber.c index 8231e8c..3467293 100644 --- a/openbsc/src/gprs/gprs_subscriber.c +++ b/openbsc/src/gprs/gprs_subscriber.c @@ -63,7 +63,8 @@ int gprs_subscr_init(struct sgsn_instance *sgi)
sgi->gsup_client = gprs_gsup_client_create( addr_str, sgi->cfg.gsup_server_port, - &gsup_read_cb); + &gsup_read_cb, + &sgi->cfg.oap);
if (!sgi->gsup_client) return -1; diff --git a/openbsc/tests/sgsn/Makefile.am b/openbsc/tests/sgsn/Makefile.am index 3c202dd..24504c2 100644 --- a/openbsc/tests/sgsn/Makefile.am +++ b/openbsc/tests/sgsn/Makefile.am @@ -28,6 +28,8 @@ sgsn_test_LDADD = \ $(top_builddir)/src/gprs/gprs_utils.o \ $(top_builddir)/src/gprs/gprs_subscriber.o \ $(top_builddir)/src/gprs/gsm_04_08_gprs.o \ + $(top_builddir)/src/gprs/oap.o \ + $(top_builddir)/src/gprs/oap_messages.o \ $(top_builddir)/src/libcommon/libcommon.a \ $(LIBOSMOABIS_LIBS) \ $(LIBOSMOCORE_LIBS) \
On 12 Oct 2015, at 11:57, Neels Hofmeyr nhofmeyr@sysmocom.de wrote:
Dear Neels,
diff --git a/openbsc/src/gprs/gprs_gsup_client.c b/openbsc/src/gprs/gprs_gsup_client.c index 1f9e34c..a332fa7 100644 --- a/openbsc/src/gprs/gprs_gsup_client.c +++ b/openbsc/src/gprs/gprs_gsup_client.c
static int gsup_client_read_cb(struct ipa_client_conn *link, struct msgb *msg) {
- OSMO_ASSERT(gsupc->read_cb != NULL);
- gsupc->read_cb(gsupc, msg);
- if (he->proto == IPAC_PROTO_EXT_GSUP) {
OSMO_ASSERT(gsupc->read_cb != NULL);gsupc->read_cb(gsupc, msg);/* expecting read_cb() to free msg */- }
- else
- if (he->proto == IPAC_PROTO_EXT_OAP) {
return gsup_client_oap_handle(gsupc, msg);/* gsup_client_oap_handle frees msg */- }
- else
goto invalid;
the coding style would not have else and if on two different lines. I will fix this myself right now.
cheers holger
On Mon, Nov 02, 2015 at 11:33:50AM +0100, Holger Freyther wrote:
static int gsup_client_read_cb(struct ipa_client_conn *link, struct msgb *msg) {
- OSMO_ASSERT(gsupc->read_cb != NULL);
- gsupc->read_cb(gsupc, msg);
- if (he->proto == IPAC_PROTO_EXT_GSUP) {
OSMO_ASSERT(gsupc->read_cb != NULL);gsupc->read_cb(gsupc, msg);/* expecting read_cb() to free msg */- }
- else
- if (he->proto == IPAC_PROTO_EXT_OAP) {
return gsup_client_oap_handle(gsupc, msg);/* gsup_client_oap_handle frees msg */- }
- else
goto invalid;the coding style would not have else and if on two different lines. I will fix this myself right now.
Yes, indeed. I'm doing that on purpose... the logical idea is that the if conditions all start on the same column.
That's my personal dialect where I'm puzzled that no-one else seems to do it this way. IMHO it's the only way to do it nicely ;)
that said ... do I really have to lose that bit of personal dialect?
Admitted, in a bunch of code where I'm editing, I should always adhere to the local style. But my question is also for the gtphub development. Sure it's nice to have all styles match in osmo code ... I guess I need someone to push me to write "else if" on the same line. Against the styles in openggsn, this style deviation is harmless ... right?? ;)
~Neels
On Mon, Nov 02, 2015 at 03:06:29PM +0100, Neels Hofmeyr wrote:
On Mon, Nov 02, 2015 at 11:33:50AM +0100, Holger Freyther wrote:
static int gsup_client_read_cb(struct ipa_client_conn *link, struct msgb *msg) {
- OSMO_ASSERT(gsupc->read_cb != NULL);
- gsupc->read_cb(gsupc, msg);
- if (he->proto == IPAC_PROTO_EXT_GSUP) {
OSMO_ASSERT(gsupc->read_cb != NULL);gsupc->read_cb(gsupc, msg);/* expecting read_cb() to free msg */- }
- else
- if (he->proto == IPAC_PROTO_EXT_OAP) {
return gsup_client_oap_handle(gsupc, msg);/* gsup_client_oap_handle frees msg */- }
- else
goto invalid;the coding style would not have else and if on two different lines. I will fix this myself right now.
Yes, indeed. I'm doing that on purpose... the logical idea is that the if conditions all start on the same column.
Now that I'm actually looking at the commit, let's name it:
I wrote:
if (a) { frizziply(); } else if (b) { frobnicate(); } else goto invalid;
and you committed
if (a) { frizziply(); } else if (b) { frobnicate(); } else goto invalid;
I personally find it mildly ugly, but let's avoid the discussion.
To reiterate, I'd like to know whether gtphub should/really must ;) be changed to the latter style.
~Neels
On 02 Nov 2015, at 19:18, Neels Hofmeyr nhofmeyr@sysmocom.de wrote:
I personally find it mildly ugly, but let's avoid the discussion.
To reiterate, I'd like to know whether gtphub should/really must ;) be changed to the latter style.
yes.
On Mon, Nov 02, 2015 at 08:20:39PM +0100, Holger Freyther wrote:
On 02 Nov 2015, at 19:18, Neels Hofmeyr nhofmeyr@sysmocom.de wrote:
I personally find it mildly ugly, but let's avoid the discussion.
To reiterate, I'd like to know whether gtphub should/really must ;) be changed to the latter style.
yes.
heh, was actually just four places in gtphub. And the script fixed five others elsewhere while at it, sent in a separate patch mail.
~Neels
Sponsored-by: On-Waves ehf --- openbsc/src/gprs/sgsn_vty.c | 91 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+)
diff --git a/openbsc/src/gprs/sgsn_vty.c b/openbsc/src/gprs/sgsn_vty.c index b74d01a..838644e 100644 --- a/openbsc/src/gprs/sgsn_vty.c +++ b/openbsc/src/gprs/sgsn_vty.c @@ -214,6 +214,18 @@ static int config_write_sgsn(struct vty *vty) if (g_cfg->gsup_server_port) vty_out(vty, " gsup remote-port %d%s", g_cfg->gsup_server_port, VTY_NEWLINE); + + vty_out(vty, " gsup oap-id %d%s", + (int)g_cfg->oap.client_id, VTY_NEWLINE); + if (g_cfg->oap.secret_k_present != 0) + vty_out(vty, " gsup oap-k %s%s", + osmo_hexdump_nospc(g_cfg->oap.secret_k, sizeof(g_cfg->oap.secret_k)), + VTY_NEWLINE); + if (g_cfg->oap.secret_opc_present != 0) + vty_out(vty, " gsup oap-opc %s%s", + osmo_hexdump_nospc(g_cfg->oap.secret_opc, sizeof(g_cfg->oap.secret_opc)), + VTY_NEWLINE); + llist_for_each_entry(acl, &g_cfg->imsi_acl, list) vty_out(vty, " imsi-acl add %s%s", acl->imsi, VTY_NEWLINE);
@@ -895,6 +907,82 @@ DEFUN(cfg_gsup_remote_port, cfg_gsup_remote_port_cmd, return CMD_SUCCESS; }
+DEFUN(cfg_gsup_oap_id, cfg_gsup_oap_id_cmd, + "gsup oap-id <0-65535>", + "GSUP Parameters\n" + "Set the SGSN's OAP client ID\nOAP client ID (0 == disabled)\n") +{ + /* VTY ensures range */ + g_cfg->oap.client_id = (uint16_t)atoi(argv[0]); + return CMD_SUCCESS; +} + +DEFUN(cfg_gsup_oap_k, cfg_gsup_oap_k_cmd, + "gsup oap-k K", + "GSUP Parameters\n" + "Set the OAP shared secret K\nK value (16 byte) hex\n") +{ + const char *k = argv[0]; + + g_cfg->oap.secret_k_present = 0; + + if ((!k) || (strlen(k) == 0)) + goto disable; + + int k_len = osmo_hexparse(k, + g_cfg->oap.secret_k, + sizeof(g_cfg->oap.secret_k)); + if (k_len != 16) { + vty_out(vty, "%% need exactly 16 octets for oap-k, got %d.%s", + k_len, VTY_NEWLINE); + goto disable; + } + + g_cfg->oap.secret_k_present = 1; + return CMD_SUCCESS; + +disable: + if (g_cfg->oap.client_id > 0) { + vty_out(vty, "%% OAP client ID set, but invalid oap-k value disables OAP.%s", + VTY_NEWLINE); + return CMD_WARNING; + } + return CMD_SUCCESS; +} + +DEFUN(cfg_gsup_oap_opc, cfg_gsup_oap_opc_cmd, + "gsup oap-opc OPC", + "GSUP Parameters\n" + "Set the OAP shared secret OPC\nOPC value (16 byte) hex\n") +{ + const char *opc = argv[0]; + + g_cfg->oap.secret_opc_present = 0; + + if ((!opc) || (strlen(opc) == 0)) + goto disable; + + int opc_len = osmo_hexparse(opc, + g_cfg->oap.secret_opc, + sizeof(g_cfg->oap.secret_opc)); + if (opc_len != 16) { + vty_out(vty, "%% need exactly 16 octets for oap-opc, got %d.%s", + opc_len, VTY_NEWLINE); + goto disable; + } + + g_cfg->oap.secret_opc_present = 1; + return CMD_SUCCESS; + +disable: + if (g_cfg->oap.client_id > 0) { + vty_out(vty, "%% OAP client ID set, but invalid oap-opc value disables OAP.%s", + VTY_NEWLINE); + return CMD_WARNING; + } + return CMD_SUCCESS; +} + DEFUN(cfg_apn_name, cfg_apn_name_cmd, "access-point-name NAME", "Configure a global list of allowed APNs\n" @@ -969,6 +1057,9 @@ int sgsn_vty_init(void) install_element(SGSN_NODE, &cfg_auth_policy_cmd); install_element(SGSN_NODE, &cfg_gsup_remote_ip_cmd); install_element(SGSN_NODE, &cfg_gsup_remote_port_cmd); + install_element(SGSN_NODE, &cfg_gsup_oap_id_cmd); + install_element(SGSN_NODE, &cfg_gsup_oap_k_cmd); + install_element(SGSN_NODE, &cfg_gsup_oap_opc_cmd); install_element(SGSN_NODE, &cfg_apn_ggsn_cmd); install_element(SGSN_NODE, &cfg_apn_imsi_ggsn_cmd); install_element(SGSN_NODE, &cfg_apn_name_cmd);