From f6c1f03d92cbadb98ec51c5af67bd18c2ab15741 Mon Sep 17 00:00:00 2001 From: Maksym Sobolyev Date: Tue, 16 Nov 2021 09:23:14 -0800 Subject: [PATCH 1/2] Make qop parameter of the xyz_challenge() positional-dependent, so that it's possible to generate qop="auth-int,auth" as well as qop="auth,auth-int". Tested @ OpenSIPIt'02. --- lib/digest_auth/dauth_nonce.c | 2 +- modules/auth/api.c | 15 +++++++++++---- modules/auth/challenge.c | 25 ++++++++++++++++++------- modules/auth/challenge.h | 3 ++- parser/digest/digest.c | 4 +++- parser/digest/digest_parser.h | 4 +++- 6 files changed, 38 insertions(+), 15 deletions(-) diff --git a/lib/digest_auth/dauth_nonce.c b/lib/digest_auth/dauth_nonce.c index 0e760aa1260..9d8311d7c41 100644 --- a/lib/digest_auth/dauth_nonce.c +++ b/lib/digest_auth/dauth_nonce.c @@ -59,7 +59,7 @@ _Static_assert(offsetof(struct nonce_context_priv, pub) == 0, struct nonce_payload { int index; - uint64_t qop:2; + uint64_t qop:3; uint64_t alg:3; uint64_t expires_sec:34; uint64_t expires_usec:20; diff --git a/modules/auth/api.c b/modules/auth/api.c index 3b384992059..0d7ced8494b 100644 --- a/modules/auth/api.c +++ b/modules/auth/api.c @@ -229,10 +229,17 @@ auth_result_t pre_auth(struct sip_msg* _m, str* _realm, hdr_types_t _hftype, goto stalenonce; } qop_type_t qop = dcp->qop.qop_parsed; - if ((np.qop != qop) && - (np.qop != QOP_TYPE_BOTH || (qop != QOP_AUTH_D && qop != QOP_AUTHINT_D))) { - LM_DBG("nonce does not match qop\n"); - goto stalenonce; + if (np.qop != qop) { + switch (np.qop) { + case QOP_TYPE_AUTH_AUTH_INT: + case QOP_TYPE_AUTH_INT_AUTH: + if (qop == QOP_AUTH_D || qop == QOP_AUTHINT_D) + break; + /* Fall through */ + default: + LM_DBG("nonce (%d) does not match qop (%d)\n", np.qop, qop); + goto stalenonce; + } } if (is_nonce_stale(&np, nonce_expire)) { LM_DBG("stale nonce value received\n"); diff --git a/modules/auth/challenge.c b/modules/auth/challenge.c index c4dcd0b7df7..56cf6810c04 100644 --- a/modules/auth/challenge.c +++ b/modules/auth/challenge.c @@ -60,7 +60,8 @@ #define QOP_AUTH ", qop=\"" QOP_AUTH_STR "\"" #define QOP_AUTH_INT ", qop=\"" QOP_AUTHINT_STR "\"" -#define QOP_AUTH_BOTH ", qop=\"" QOP_AUTH_STR "," QOP_AUTHINT_STR "\"" +#define QOP_AUTH_BOTH_AAI ", qop=\"" QOP_AUTH_STR "," QOP_AUTHINT_STR "\"" +#define QOP_AUTH_BOTH_AIA ", qop=\"" QOP_AUTHINT_STR "," QOP_AUTH_STR "\"" #define STALE_PARAM ", stale=true" #define DIGEST_REALM ": Digest realm=\"" #define DIGEST_NONCE "\", nonce=\"" @@ -84,12 +85,22 @@ static inline char *build_auth_hf(int _retries, int _stale, struct nonce_params calc_np; if (_qop) { - if (_qop == QOP_TYPE_AUTH) { + switch (_qop) { + case QOP_TYPE_AUTH: qop_param = str_const_init(QOP_AUTH); - } else if (_qop == QOP_TYPE_AUTH_INT) { + break; + case QOP_TYPE_AUTH_INT: qop_param = str_const_init(QOP_AUTH_INT); - } else { - qop_param = str_const_init(QOP_AUTH_BOTH); + break; + case QOP_TYPE_AUTH_AUTH_INT: + qop_param = str_const_init(QOP_AUTH_BOTH_AAI); + break; + case QOP_TYPE_AUTH_INT_AUTH: + qop_param = str_const_init(QOP_AUTH_BOTH_AIA); + break; + default: + LM_ERR("Wrong _qop value: %d\n", _qop); + abort(); } } if (_stale) @@ -264,12 +275,12 @@ int fixup_qop(void** param) for (q = q_csv; q; q = q->next) { if (!str_strcmp(&q->s, const_str(QOP_AUTH_STR))) { if (qop_type == QOP_TYPE_AUTH_INT) - qop_type = QOP_TYPE_BOTH; + qop_type = QOP_TYPE_AUTH_INT_AUTH; else qop_type = QOP_TYPE_AUTH; } else if (!str_strcmp(&q->s, const_str(QOP_AUTHINT_STR))) { if (qop_type == QOP_TYPE_AUTH) - qop_type = QOP_TYPE_BOTH; + qop_type = QOP_TYPE_AUTH_AUTH_INT; else qop_type = QOP_TYPE_AUTH_INT; } else { diff --git a/modules/auth/challenge.h b/modules/auth/challenge.h index 38a79a97b0a..7e32a371077 100644 --- a/modules/auth/challenge.h +++ b/modules/auth/challenge.h @@ -28,7 +28,8 @@ #define QOP_TYPE_AUTH QOP_AUTH_D #define QOP_TYPE_AUTH_INT QOP_AUTHINT_D -#define QOP_TYPE_BOTH (QOP_AUTH_D + QOP_AUTHINT_D) +#define QOP_TYPE_AUTH_INT_AUTH QOP_AUTHINT_AUTH_D +#define QOP_TYPE_AUTH_AUTH_INT QOP_AUTH_AUTHINT_D int fixup_qop(void** param); diff --git a/parser/digest/digest.c b/parser/digest/digest.c index e1a134fe1e0..a957aee6f3b 100644 --- a/parser/digest/digest.c +++ b/parser/digest/digest.c @@ -125,7 +125,7 @@ dig_err_t check_dig_cred(dig_cred_t* _c) /* If QOP parameter is present, some additional * requirements must be met */ - if ((_c->qop.qop_parsed == QOP_AUTH_D) || (_c->qop.qop_parsed == QOP_AUTHINT_D)) { + if (_c->qop.qop_parsed != QOP_UNSPEC_D) { /* CNONCE must be specified */ if (_c->cnonce.s == 0) res |= E_DIG_CNONCE; /* and also nonce count must be specified */ @@ -175,6 +175,8 @@ void print_cred(dig_cred_t* _c) CASE_PRINTENUM(QOP_UNSPEC_D); CASE_PRINTENUM(QOP_AUTH_D); CASE_PRINTENUM(QOP_AUTHINT_D); + CASE_PRINTENUM(QOP_AUTHINT_AUTH_D); + CASE_PRINTENUM(QOP_AUTH_AUTHINT_D); CASE_PRINTENUM(QOP_OTHER_D); } printf("NC = \'%.*s\'\n", _c->nc.len, _c->nc.s); diff --git a/parser/digest/digest_parser.h b/parser/digest/digest_parser.h index f3567206631..9c92ec3f43b 100644 --- a/parser/digest/digest_parser.h +++ b/parser/digest/digest_parser.h @@ -69,7 +69,9 @@ typedef enum qop_type { QOP_UNSPEC_D = 0, /* QOP parameter not present in response */ QOP_AUTH_D = 1, /* Authentication only */ QOP_AUTHINT_D = 2, /* Authentication with integrity checks */ - QOP_OTHER_D = 4 /* Unknown */ + QOP_AUTHINT_AUTH_D = 3, /* Authentication with integrity checks+Authentication only */ + QOP_AUTH_AUTHINT_D = 4, /* Authentication only+Authentication with integrity checks */ + QOP_OTHER_D = 5 /* Unknown */ } qop_type_t; /* Canonical QOP names */ From f90f20cffd93cc808a146007a47ba645070e035e Mon Sep 17 00:00:00 2001 From: Maksym Sobolyev Date: Tue, 16 Nov 2021 20:47:21 -0800 Subject: [PATCH 2/2] o Get rid of the redundant macros QOP_TYPE_XYZ. o Use qop_type_t instead of int where appropriate. o Enforce qop for MD5-sess when generating a challenge. o Use QOP_UNSPEC_D instead of 0 where appropriate. --- modules/auth/api.c | 4 ++-- modules/auth/challenge.c | 36 ++++++++++++++++++------------------ modules/auth/challenge.h | 5 ----- 3 files changed, 20 insertions(+), 25 deletions(-) diff --git a/modules/auth/api.c b/modules/auth/api.c index 0d7ced8494b..d8e0c767d51 100644 --- a/modules/auth/api.c +++ b/modules/auth/api.c @@ -231,8 +231,8 @@ auth_result_t pre_auth(struct sip_msg* _m, str* _realm, hdr_types_t _hftype, qop_type_t qop = dcp->qop.qop_parsed; if (np.qop != qop) { switch (np.qop) { - case QOP_TYPE_AUTH_AUTH_INT: - case QOP_TYPE_AUTH_INT_AUTH: + case QOP_AUTH_AUTHINT_D: + case QOP_AUTHINT_AUTH_D: if (qop == QOP_AUTH_D || qop == QOP_AUTHINT_D) break; /* Fall through */ diff --git a/modules/auth/challenge.c b/modules/auth/challenge.c index 56cf6810c04..b0222deb2e0 100644 --- a/modules/auth/challenge.c +++ b/modules/auth/challenge.c @@ -72,7 +72,7 @@ * Create {WWW,Proxy}-Authenticate header field */ static inline char *build_auth_hf(int _retries, int _stale, - const str_const *_realm, int* _len, int _qop, alg_t alg, + const str_const *_realm, int* _len, qop_type_t _qop, alg_t alg, const str_const *alg_val, const str_const* _hf_name, int index) { @@ -86,16 +86,16 @@ static inline char *build_auth_hf(int _retries, int _stale, if (_qop) { switch (_qop) { - case QOP_TYPE_AUTH: + case QOP_AUTH_D: qop_param = str_const_init(QOP_AUTH); break; - case QOP_TYPE_AUTH_INT: + case QOP_AUTHINT_D: qop_param = str_const_init(QOP_AUTH_INT); break; - case QOP_TYPE_AUTH_AUTH_INT: + case QOP_AUTHINT_AUTH_D: qop_param = str_const_init(QOP_AUTH_BOTH_AAI); break; - case QOP_TYPE_AUTH_INT_AUTH: + case QOP_AUTH_AUTHINT_D: qop_param = str_const_init(QOP_AUTH_BOTH_AIA); break; default: @@ -175,7 +175,7 @@ static inline char *build_auth_hf(int _retries, int _stale, /* * Create and send a challenge */ -static inline int challenge(struct sip_msg* _msg, str *realm, int _qop, +static inline int challenge(struct sip_msg* _msg, str *realm, qop_type_t _qop, int _code, const str *reason, const str_const *_challenge_msg, int algmask) { struct hdr_field* h = NULL; @@ -215,9 +215,9 @@ static inline int challenge(struct sip_msg* _msg, str *realm, int _qop, } nalgs = 0; - if (algmask >= ALGFLG_SHA256 && _qop == 0) { - /* RFC8760 mandates QOP */ - _qop = QOP_TYPE_AUTH; + if (algmask >= ALG_MD5SESS && _qop == QOP_UNSPEC_D) { + /* RFC8760 algos and XYZ-sess mandates QOP */ + _qop = QOP_AUTH_D; } if(!disable_nonce_check) { /* get the nonce index and mark it as used */ @@ -264,7 +264,7 @@ static inline int challenge(struct sip_msg* _msg, str *realm, int _qop, int fixup_qop(void** param) { str *s = (str*)*param; - int qop_type = 0; + qop_type_t qop_type = QOP_UNSPEC_D; csv_record *q_csv, *q; q_csv = parse_csv_record(s); @@ -274,15 +274,15 @@ int fixup_qop(void** param) } for (q = q_csv; q; q = q->next) { if (!str_strcmp(&q->s, const_str(QOP_AUTH_STR))) { - if (qop_type == QOP_TYPE_AUTH_INT) - qop_type = QOP_TYPE_AUTH_INT_AUTH; + if (qop_type == QOP_AUTHINT_D) + qop_type = QOP_AUTHINT_AUTH_D; else - qop_type = QOP_TYPE_AUTH; + qop_type = QOP_AUTH_D; } else if (!str_strcmp(&q->s, const_str(QOP_AUTHINT_STR))) { - if (qop_type == QOP_TYPE_AUTH) - qop_type = QOP_TYPE_AUTH_AUTH_INT; + if (qop_type == QOP_AUTH_D) + qop_type = QOP_AUTH_AUTHINT_D; else - qop_type = QOP_TYPE_AUTH_INT; + qop_type = QOP_AUTHINT_D; } else { LM_ERR("Bad qop type\n"); free_csv_record(q_csv); @@ -302,7 +302,7 @@ int www_challenge(struct sip_msg* _msg, str* _realm, void* _qop, intptr_t algmask) { - return challenge(_msg, _realm, (int)(long)_qop, WWW_AUTH_CODE, + return challenge(_msg, _realm, (qop_type_t)(long)_qop, WWW_AUTH_CODE, &str_init(MESSAGE_401), &str_const_init(WWW_AUTH_HDR), algmask ? algmask : ALGFLG_UNSPEC); } @@ -315,7 +315,7 @@ int proxy_challenge(struct sip_msg* _msg, str* _realm, void* _qop, intptr_t algmask) { - return challenge(_msg, _realm, (int)(long)_qop, PROXY_AUTH_CODE, + return challenge(_msg, _realm, (qop_type_t)(long)_qop, PROXY_AUTH_CODE, &str_init(MESSAGE_407), &str_const_init(PROXY_AUTH_HDR), algmask ? algmask : ALGFLG_UNSPEC); } diff --git a/modules/auth/challenge.h b/modules/auth/challenge.h index 7e32a371077..d5dc9f94f0f 100644 --- a/modules/auth/challenge.h +++ b/modules/auth/challenge.h @@ -26,11 +26,6 @@ #include "../../parser/msg_parser.h" -#define QOP_TYPE_AUTH QOP_AUTH_D -#define QOP_TYPE_AUTH_INT QOP_AUTHINT_D -#define QOP_TYPE_AUTH_INT_AUTH QOP_AUTHINT_AUTH_D -#define QOP_TYPE_AUTH_AUTH_INT QOP_AUTH_AUTHINT_D - int fixup_qop(void** param); /*