From patchwork Mon Oct 28 13:55:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 3916 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:589a:b0:5b9:581e:f939 with SMTP id h26csp1513885max; Mon, 28 Oct 2024 06:55:30 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCWdezBAqA3VE+/AcS6jJ+MbXyDA+yrMN6Ge1GXGdkGe6jdxnWOh0kTYb5glbZWaAdbhMfZQcNV4fME=@openvpn.net X-Google-Smtp-Source: AGHT+IGZ4YIua8s36kLpyQXzl4THPREin9lvyjlXWLG+EKczfrdxxYhykcdsbo17XEhcW4I7Eq1e X-Received: by 2002:a05:6830:4422:b0:718:c99:de12 with SMTP id 46e09a7af769-71858896406mr8463517a34.3.1730123730495; Mon, 28 Oct 2024 06:55:30 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1730123730; cv=none; d=google.com; s=arc-20240605; b=PjKC8tLC8M9lgo4OsanjYofsJXWna+7Gt1YL54G9gN+OvlES4BohzeF5VMrvV7qjG0 1JBKUPGolre2c32FP6qehhcDMz/C8gqxDDSEL1yS+PIPubQEau3ql1eCyqeuH5N1IH8i IbCqbvATZ8Qu0gD4T1ci28AbFkEwYefTD9jCL354Rgj+1FNIvMdHkOgena1XSM2gWbbh 1sUVf0LRdZ2gE5NnqWH8wtCuWu2mUFghlJMYCCNp8RyAmYzctH3dcmnYt/B+QCUxP3nl /wLj3UmzTPqf/rvaygVGFbdcMIfRHuj6kYluutHlLVjfPcTkH2wMfli6mf02ibML78qt /UDw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:references:in-reply-to:message-id:date:to:from :dkim-signature:dkim-signature; bh=F1dr/q59vNZ7i0nsh1s4Tt9XcTeJTJOdXoYA7yZ5foU=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=jRmgFNqK9AKYc3B54Bqal8B5Dn+XoCHTHOkAgcKLpX3DfYleKlrOiV4HKOjtFgOB+8 vyQgOR0Llk8NRlW+jN0D7YEknSayUd1jwzjJdD09Q2uMwx0vXrFPhxo6Ml+iJLV6VTcP deEWQMY3pR+gSuPv+oqvPugfeXrzT85FqtWIGMKtKYe3DU7eK8sxeo6twNA0JdQgnGfC hIYyCFF1719BEKe92MpkDX+oZ+T6mcpDQixKpW8WCXSRNdoQwfEwpbjX5LAE62Fo+bAd QHRHpS3JqRX3NKlNfdtD3NDdBTNHkcaSLwxklZpyCzV2+Lftb2y3I/QwYisftPJLu1J+ u4vA==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=EVi8fC8r; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=fDPZL+wD; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=muc.de Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 46e09a7af769-7186187a7ddsi3569555a34.220.2024.10.28.06.55.30 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 28 Oct 2024 06:55:30 -0700 (PDT) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=EVi8fC8r; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=fDPZL+wD; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=muc.de Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1t5QDK-0001vs-Uw; Mon, 28 Oct 2024 13:55:22 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1t5QDJ-0001vi-7M for openvpn-devel@lists.sourceforge.net; Mon, 28 Oct 2024 13:55:21 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-ID:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=yGJVpdJQRnEzmOaQ7tBbqg3m4sTEEOYtkZgTCzPM9Hw=; b=EVi8fC8r/Ml8puespt8JdDHzO5 1If26wHkmbWqAHAjLLgy3om9/iLk3n3WQhTpb+/oUWOw7ZAglXi1JBrXOGESxmZkU1G0tAEROnKOb sGt4lN0Bu57Mwg9BB8ktLa3keH0sGk/Nm9D04D8msyktU05UECYQm69+vC37V0FZkmKQ=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-ID: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=yGJVpdJQRnEzmOaQ7tBbqg3m4sTEEOYtkZgTCzPM9Hw=; b=fDPZL+wDDyLBGbWx1zdynWfjmB 4EMZLFTFVL+ktuu1LehktxfTLjZVtQBURbQ1guumBs9KB1iB/v4xn/6etO9+9epumt8my40t4m7Gr hdWLMLB+vUIVTlEm/WzQqccro8VYX+5TAQMHen4FG41+zED4+8yQts1S8buuPkj9WYuA=; Received: from dhcp-174.greenie.muc.de ([193.149.48.174] helo=blue.greenie.muc.de) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1t5QDG-0006q4-MD for openvpn-devel@lists.sourceforge.net; Mon, 28 Oct 2024 13:55:20 +0000 Received: from blue.greenie.muc.de (localhost [127.0.0.1]) by blue.greenie.muc.de (8.17.1.9/8.17.1.9) with ESMTP id 49SDt6mn028719 for ; Mon, 28 Oct 2024 14:55:06 +0100 Received: (from gert@localhost) by blue.greenie.muc.de (8.17.1.9/8.17.1.9/Submit) id 49SDt6px028718 for openvpn-devel@lists.sourceforge.net; Mon, 28 Oct 2024 14:55:06 +0100 From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Mon, 28 Oct 2024 14:55:04 +0100 Message-ID: <20241028135505.28651-1-gert@greenie.muc.de> X-Mailer: git-send-email 2.45.2 In-Reply-To: References: MIME-Version: 1.0 X-Spam-Score: -0.0 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-2.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: From: Arne Schwabe When OpenVPN is compiled without PKCS11 support USER_PASS_LEN is 128 bytes. If we encounter a username larger than this length, we would only read the 2 bytes length header of the username/password. W [...] Content analysis details: (-0.0 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.0 SPF_PASS SPF: sender matches SPF record X-Headers-End: 1t5QDG-0006q4-MD Subject: [Openvpn-devel] [PATCH v2] Refuse clients if username or password is longer than USER_PASS_LEN X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1814166220786482443?= X-GMAIL-MSGID: =?utf-8?q?1814166220786482443?= From: Arne Schwabe When OpenVPN is compiled without PKCS11 support USER_PASS_LEN is 128 bytes. If we encounter a username larger than this length, we would only read the 2 bytes length header of the username/password. We did then also NOT skip the username or password field meaning that we would continue reading the rest of the packet at the wrong offset and get garbage results like not having peerinfo and then rejecting a client because of no common cipher or missing data v2 support. This will tell the client that username/password is too regardless of whether password/username authentication is used. This way we do not leak if username/password authentication is active. To reproduce this issue have the server compiled with a USER_PASS_LEN set to 128 (e.g. without pkcs11 or manually adjusting the define) and have the client with a larger USER_PASS_LEN to actually be able to send the larger password. The server must also be set to use only certificate authentication while the client must use certificates and auth-user-pass because otherwise the user/pass verification will reject the empty credentials. Using the openvpn3 test client with overlong username/password also works. Change-Id: I60f02c919767eb8f1b95253689a8233f5f68621d Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/787 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index e2be614..8040e7b 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1835,20 +1835,33 @@ return true; } -static bool +/** + * Read a string that is encoded as a 2 byte header with the length from the + * buffer \c buf. Will return the non-negative value if reading was successful. + * The returned value will include the trailing 0 byte. + * + * If the message is over the capacity or could not be read + * it will return the negative length that was in the + * header and try to skip the string. If the string cannot be skipped, the + * buf will stay at the current position or position + 2 + */ +static int read_string(struct buffer *buf, char *str, const unsigned int capacity) { const int len = buf_read_u16(buf); if (len < 1 || len > (int)capacity) { - return false; + buf_advance(buf, len); + + /* will also return 0 for a no string being present */ + return -len; } if (!buf_read(buf, str, len)) { - return false; + return -len; } str[len-1] = '\0'; - return true; + return len; } static char * @@ -2218,8 +2231,6 @@ { struct key_state *ks = &session->key[KS_PRIMARY]; /* primary key */ - bool username_status, password_status; - struct gc_arena gc = gc_new(); char *options; struct user_pass *up = NULL; @@ -2253,7 +2264,7 @@ } /* get options */ - if (!read_string(buf, options, TLS_OPTIONS_LEN)) + if (read_string(buf, options, TLS_OPTIONS_LEN) < 0) { msg(D_TLS_ERRORS, "TLS Error: Failed to read required OCC options string"); goto error; @@ -2266,8 +2277,8 @@ * peer_info data which follows behind */ ALLOC_OBJ_CLEAR_GC(up, struct user_pass, &gc); - username_status = read_string(buf, up->username, USER_PASS_LEN); - password_status = read_string(buf, up->password, USER_PASS_LEN); + int username_len = read_string(buf, up->username, USER_PASS_LEN); + int password_len = read_string(buf, up->password, USER_PASS_LEN); /* get peer info from control channel */ free(multi->peer_info); @@ -2290,10 +2301,21 @@ multi->remote_ciphername = string_alloc("none", NULL); } - if (tls_session_user_pass_enabled(session)) + if (username_len < 0 || password_len < 0) + { + msg(D_TLS_ERRORS, "TLS Error: Username (%d) or password (%d) too long", + abs(username_len), abs(password_len)); + auth_set_client_reason(multi, "Username or password is too long. " + "Maximum length is 128 bytes"); + + /* treat the same as failed username/password and do not error + * out (goto error) to sent an AUTH_FAILED back to the client */ + ks->authenticated = KS_AUTH_FALSE; + } + else if (tls_session_user_pass_enabled(session)) { /* Perform username/password authentication */ - if (!username_status || !password_status) + if (!username_len || !password_len) { CLEAR(*up); if (!(session->opt->ssl_flags & SSLF_AUTH_USER_PASS_OPTIONAL))