From a7d54a8af148e61cb55b90ebae86eb25eadce060 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Sat, 11 Jul 2020 10:52:56 +0100 Subject: Disable weak (RFC2069) Digest by default, unless NE_AUTH_WEAK_DIGEST is used. Closes #23. * src/ne_auth.h: Define NE_AUTH_WEAK_DIGEST. * src/ne_auth.c (digest_challenge): Fail if qop=auth is not present and WEAK_DIGEST is not enabled. (auth_register): NE_AUTH_WEAK_DIGEST implies NE_AUTH_DIGEST to retain binary backwards compatibility. * test/auth.c (test_digest, digest, digest_failures): Test for 2069-style auth with WEAK_DIGEST, test compat mode, test for new failure mode. --- NEWS | 11 ++++++++--- src/ne_auth.c | 13 +++++++++++++ src/ne_auth.h | 11 +++++++++-- test/auth.c | 34 ++++++++++++++++++++++------------ 4 files changed, 52 insertions(+), 17 deletions(-) diff --git a/NEWS b/NEWS index 8eb8ad4..35ac696 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,9 @@ Changes in release 0.32.0: * Interface changes: - - none, API and ABI backwards-compatible with 0.27.x and later + - API and ABI backwards-compatible with 0.27.x and later + - NE_AUTH_DIGEST now only enables RFC 2617/7616 auth by default; + to enable weaker RFC 2069 Digest, use NE_AUTH_WEAK_DIGEST + (this is not treated as an API/ABI break) * API clarification: - for ne_auth.h, the character encoding of the username provided by a credentials callback was unspecified. Callers providing @@ -9,8 +12,10 @@ Changes in release 0.32.0: if the username cannot be sent safely. * New interfaces and features: - ne_string.h: added ne_strhash, ne_vstrhash, ne_strparam - - added RFC 7616 (Digest authentication) support, including - userhash, username*, SHA-256, SHA-512-256 algorithms + - ne_auth.h: added RFC 7616 (Digest authentication) support, + including userhash=, username*= and SHA-2-256/512-256 algorithms + (SHA-2 requires GnuTLS/OpenSSL). added NE_AUTH_WEAK_DIGEST + to re-enable RFC 2069 Digest support Changes in release 0.31.0: * Interface changes: diff --git a/src/ne_auth.c b/src/ne_auth.c index ad44842..9015f21 100644 --- a/src/ne_auth.c +++ b/src/ne_auth.c @@ -846,6 +846,11 @@ static int digest_challenge(auth_session *sess, int attempt, challenge_error(errmsg, _("stale Digest challenge with new algorithm or realm")); return -1; } + else if (!parms->got_qop + && (parms->handler->protomask & NE_AUTH_WEAK_DIGEST) == 0) { + challenge_error(errmsg, _("weak Digest challenge not supported")); + return -1; + } hash = alg_to_hash[parms->alg]; p = ne_strhash(hash, "", NULL); @@ -1668,6 +1673,14 @@ static void auth_register(ne_session *sess, int isproxy, unsigned protomask, } } + /* For backwards-compatibility with older releases where DIGEST + * used to be defined as WEAKEST, if only WEAK_DIGEST is given, + * that implies DIGEST|WEAK_DIGEST. */ + if ((protomask & (NE_AUTH_WEAK_DIGEST|NE_AUTH_DIGEST)) == NE_AUTH_WEAK_DIGEST) { + NE_DEBUG(NE_DBG_HTTPAUTH, "auth: Weak Digest support compatibility mode.\n"); + protomask |= NE_AUTH_DIGEST; + } + if ((protomask & NE_AUTH_NEGOTIATE) == NE_AUTH_NEGOTIATE) { /* Map NEGOTIATE to NTLM | GSSAPI. */ protomask |= NE_AUTH_GSSAPI | NE_AUTH_NTLM; diff --git a/src/ne_auth.h b/src/ne_auth.h index a114f7f..d241920 100644 --- a/src/ne_auth.h +++ b/src/ne_auth.h @@ -77,8 +77,13 @@ void ne_set_proxy_auth(ne_session *sess, ne_auth_creds creds, void *userdata); * password, and certain aspects of the request, so prevents passive * attackers from obtaining the credentials; active attackers can * still modify most of the request/response if using an unsecured - * channel. */ -#define NE_AUTH_DIGEST (0x0002) + * channel. Supports algorithms from RFC 2617 and RFC 7616. */ +#define NE_AUTH_DIGEST (0x0080) + +/* NE_AUTH_WEAK_DIGEST: This may be used in conjunction with + * NE_AUTH_DIGEST to enable support for the older, weaker version of + * the Digest algorithm specified in RFC 2069. */ +#define NE_AUTH_WEAK_DIGEST (0x0002) /* NE_AUTH_NEGOTIATE: Negotiate uses GSSAPI/SSPI, or NTLM, to * authenticate the user; an active attacker can modify any of the @@ -108,6 +113,8 @@ void ne_set_proxy_auth(ne_session *sess, ne_auth_creds creds, void *userdata); * this must not be used over an unsecured channel. */ #define NE_AUTH_GSSAPI_ONLY (0x0040) +/* 0x0080 used for NE_AUTH_DIGEST */ + /* The default set of supported protocols, as deemed appropriate for * the given session scheme. */ #define NE_AUTH_DEFAULT (0x1000) diff --git a/test/auth.c b/test/auth.c index 2b9fdb3..5b6ea95 100644 --- a/test/auth.c +++ b/test/auth.c @@ -399,6 +399,8 @@ static void dup_header(char *header) #define PARM_USERHASH (0x0010) /* userhash=true */ #define PARM_UHFALSE (0x0020) /* userhash=false */ #define PARM_ALTUSER (0x0040) +#define PARM_WEAK (0x0080) +#define PARM_WEAK_ONLY (0x0100) struct digest_parms { const char *realm, *nonce, *opaque, *domain; @@ -421,7 +423,8 @@ struct digest_parms { fail_ai_omit_cnonce, fail_ai_omit_digest, fail_ai_omit_nc, - fail_outside_domain + fail_outside_domain, + fail_2069_weak } failure; }; @@ -886,6 +889,14 @@ static int test_digest(struct digest_parms *parms) { ne_session *sess; void *auth_userdata = (parms->flags & PARM_ALTUSER) ? (void *)alt_username : NULL; + unsigned proto = NE_AUTH_DIGEST; + + if ((parms->flags & PARM_ALTUSER)) + proto |= NE_AUTH_UTF8; + else if ((parms->flags & PARM_WEAK)) + proto |= NE_AUTH_WEAK_DIGEST; + else if ((parms->flags & PARM_WEAK_ONLY)) + proto = NE_AUTH_WEAK_DIGEST; NE_DEBUG(NE_DBG_HTTP, ">>>> Request sequence begins " "(reqs=%d, nonce=%s, rfc=%s, stale=%d, proxy=%d).\n", @@ -900,11 +911,7 @@ static int test_digest(struct digest_parms *parms) } else { CALL(session_server(&sess, serve_digest, parms)); - if ((parms->flags & PARM_ALTUSER)) - ne_add_server_auth(sess, NE_AUTH_DEFAULT|NE_AUTH_UTF8, - auth_cb, auth_userdata); - else - ne_set_server_auth(sess, auth_cb, auth_userdata); + ne_add_server_auth(sess, proto, auth_cb, auth_userdata); } do { @@ -932,7 +939,7 @@ static int digest(void) /* staleness. */ { "WallyWorld", "this-is-a-nonce", "opaque-thingy", NULL, ALG_MD5, PARM_RFC2617 | PARM_AINFO, 3, 2, fail_not }, /* 2069 + stale */ - { "WallyWorld", "this-is-a-nonce", NULL, NULL, ALG_MD5, PARM_AINFO, 3, 2, fail_not }, + { "WallyWorld", "this-is-a-nonce", NULL, NULL, ALG_MD5, PARM_WEAK|PARM_AINFO, 3, 2, fail_not }, /* RFC 7616-style */ { "WallyWorld", "new-day-new-nonce", "new-opaque", NULL, ALG_MD5, PARM_RFC2617 | PARM_USERHASH, 1, 0, fail_not }, @@ -940,9 +947,10 @@ static int digest(void) { "WallyWorld", "just-another-nonce", "new-opaque", NULL, ALG_MD5, PARM_RFC2617 | PARM_UHFALSE, 1, 0, fail_not }, /* RFC 2069-style */ - { "WallyWorld", "lah-di-da-di-dah", NULL, NULL, ALG_MD5, 0, 1, 0, fail_not }, - { "WallyWorld", "fee-fi-fo-fum", "opaque-string", NULL, ALG_MD5, 0, 1, 0, fail_not }, - { "WallyWorld", "fee-fi-fo-fum", "opaque-string", NULL, ALG_MD5, PARM_AINFO, 1, 0, fail_not }, + { "WallyWorld", "lah-di-da-di-dah", NULL, NULL, ALG_MD5, PARM_WEAK, 1, 0, fail_not }, + { "WallyWorld", "lah-lah-lah-lah", NULL, NULL, ALG_MD5, PARM_WEAK_ONLY, 1, 0, fail_not }, + { "WallyWorld", "fee-fi-fo-fum", "opaque-string", NULL, ALG_MD5, PARM_WEAK, 1, 0, fail_not }, + { "WallyWorld", "fee-fi-fo-fum", "opaque-string", NULL, ALG_MD5, PARM_AINFO|PARM_WEAK, 1, 0, fail_not }, /* Proxy auth */ { "WallyWorld", "this-is-also-a-nonce", "opaque-string", NULL, ALG_MD5, PARM_RFC2617|PARM_PROXY, 1, 0, fail_not }, @@ -1060,6 +1068,7 @@ static int digest_failures(void) { fail_bogus_alg, "unknown algorithm" }, { fail_req0_stale, "initial Digest challenge was stale" }, { fail_req0_2069_stale, "initial Digest challenge was stale" }, + { fail_2069_weak, "weak Digest challenge not supported" }, { fail_not, NULL } }; unsigned n; @@ -1078,7 +1087,7 @@ static int digest_failures(void) parms.failure = fails[n].mode; - if (parms.failure == fail_req0_2069_stale) + if (parms.failure == fail_req0_2069_stale || parms.failure == fail_2069_weak) parms.flags &= ~PARM_RFC2617; else parms.flags |= PARM_RFC2617; @@ -1102,7 +1111,8 @@ static int digest_failures(void) ne_session_destroy(sess); if (fails[n].mode == fail_bogus_alg - || fails[n].mode == fail_req0_stale) { + || fails[n].mode == fail_req0_stale + || fails[n].mode == fail_2069_weak) { reap_server(); } else { CALL(await_server()); -- cgit v1.2.1