From patchwork Sun Apr 25 13:39:42 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 50940 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.samba.org (fn.samba.org [216.83.154.106]) by ozlabs.org (Postfix) with ESMTP id A818FB7D29 for ; Sun, 25 Apr 2010 23:39:50 +1000 (EST) Received: from fn.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id 206AEAD1FE; Sun, 25 Apr 2010 07:39:50 -0600 (MDT) X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on fn.samba.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.8 tests=AWL, BAYES_00, NO_MORE_FUNN, SPF_PASS autolearn=no version=3.2.5 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from cdptpa-omtalb.mail.rr.com (cdptpa-omtalb.mail.rr.com [75.180.132.123]) by lists.samba.org (Postfix) with ESMTP id F0E05AD049 for ; Sun, 25 Apr 2010 07:39:43 -0600 (MDT) X-Authority-Analysis: v=1.1 cv=MoTY9QBVwAhcB2GH59FwCsmLFwRIhosZyZPAEaxcLvM= c=1 sm=0 a=Vv3OjF7u-HMA:10 a=ld/erqUjW76FpBUqCqkKeA==:17 a=pGLkceISAAAA:8 a=hGzw-44bAAAA:8 a=oW8VhveGgQNFcDjC3RoA:9 a=5CDJuFBXCkFXVDUCoHQA:7 a=kTeLBxfmLLT5TjESl_fGHxfQANcA:4 a=MSl-tDqOz04A:10 a=dowx1zmaLagA:10 a=ld/erqUjW76FpBUqCqkKeA==:117 X-Cloudmark-Score: 0 X-Originating-IP: 71.70.153.3 Received: from [71.70.153.3] ([71.70.153.3:59378] helo=mail.poochiereds.net) by cdptpa-oedge04.mail.rr.com (envelope-from ) (ecelerity 2.2.2.39 r()) with ESMTP id 32/09-23257-F1644DB4; Sun, 25 Apr 2010 13:39:43 +0000 Received: by mail.poochiereds.net (Postfix, from userid 4447) id DACF758094; Sun, 25 Apr 2010 09:39:42 -0400 (EDT) From: Jeff Layton To: scott.lovenberg@gmail.com Date: Sun, 25 Apr 2010 09:39:42 -0400 Message-Id: <1272202782-25080-1-git-send-email-jlayton@samba.org> X-Mailer: git-send-email 1.6.6.1 Cc: linux-cifs-client@lists.samba.org Subject: [linux-cifs-client] [PATCH] mount.cifs: continued cleanup of open_cred_file and zero out buffer X-BeenThere: linux-cifs-client@lists.samba.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: The Linux CIFS VFS client List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: linux-cifs-client-bounces@lists.samba.org Errors-To: linux-cifs-client-bounces@lists.samba.org From: Scott Lovenberg The parsing for values has been moved to its own function and is a bit cleaner. Temporary buffers are zeroed out before being freed to ensure passwords/credentials aren't left in released memory. Signed-off-by: Scott Lovenberg Signed-off-by: Jeff Layton --- mount.cifs.c | 130 +++++++++++++++++++++++++++++++++++----------------------- 1 files changed, 78 insertions(+), 52 deletions(-) diff --git a/mount.cifs.c b/mount.cifs.c index 6725c93..3e30219 100644 --- a/mount.cifs.c +++ b/mount.cifs.c @@ -121,6 +121,14 @@ */ #define CIFS_SETUID_FLAGS (MS_NOSUID|MS_NODEV) +/* + * Values for parsing a credentials file. + */ +#define CRED_UNPARSEABLE 0 +#define CRED_USER 1 +#define CRED_PASS 2 +#define CRED_DOM 4 + /* struct for holding parsed mount info for use by privleged process */ struct parsed_mount_info { unsigned long flags; @@ -511,20 +519,43 @@ toggle_dac_capability(int writable, int enable) #endif /* HAVE_LIBCAP */ #endif /* HAVE_LIBCAP_NG */ -/* - * Null terminate string at first '\n' - */ -static void null_terminate_endl(char* source) +static void null_terminate_endl(char *source) { - char* newline = strchr(source, '\n'); + char *newline = strchr(source, '\n'); if (newline) *newline = '\0'; } - +/* + * Parse a line from the credentials file. Changes target to first + * character after '=' on 'line' and returns the value type of the line + * Returns CRED_UNPARSEABLE on failure or if either parameter is NULL. + */ +static int parse_cred_line(char *line, char **target) +{ + if (line == NULL || target == NULL) + goto parsing_err; + + /* position target at first char of value */ + *target = strchr(line, '='); + if (!*target) + goto parsing_err; + *target += 1; + + /* tell the caller which value target points to */ + if (strncasecmp("user", line, 4) == 0) + return CRED_USER; + if (strncasecmp("pass", line, 4) == 0) + return CRED_PASS; + if (strncasecmp("dom", line, 3) == 0) + return CRED_DOM; + +parsing_err: + return CRED_UNPARSEABLE; +} static int open_cred_file(char *file_name, - struct parsed_mount_info *parsed_info) + struct parsed_mount_info *parsed_info) { char *line_buf; char *temp_val; @@ -535,30 +566,29 @@ static int open_cred_file(char *file_name, i = toggle_dac_capability(0, 1); if (i) - return i; + goto return_i; i = access(file_name, R_OK); if (i) { toggle_dac_capability(0, 0); - return i; + goto return_i; } fs = fopen(file_name, "r"); if (fs == NULL) { toggle_dac_capability(0, 0); - return errno; + i = errno; + goto return_i; } i = toggle_dac_capability(0, 0); - if (i) { - fclose(fs); - return i; - } + if (i) + goto return_i; line_buf = (char *)malloc(line_buf_size); if (line_buf == NULL) { - fclose(fs); - return EX_SYSERR; + i = EX_SYSERR; + goto return_i; } /* parse line from credentials file */ @@ -570,47 +600,43 @@ static int open_cred_file(char *file_name, } null_terminate_endl(line_buf); - /* parse user */ - if (strncasecmp("user", line_buf + i, 4) == 0) { - temp_val = strchr(line_buf + i, '='); - if (temp_val) { - /* go past equals sign */ - temp_val++; - parsed_info->got_user = 1; - strlcpy(parsed_info->username, temp_val, - sizeof(parsed_info->username)); - } - } - - /* parse password */ - else if (strncasecmp("pass", line_buf + i, 4) == 0) { - temp_val = strchr(line_buf + i, '='); - if (!temp_val) - continue; - ++temp_val; + /* parse next token */ + switch (parse_cred_line(line_buf + i, &temp_val)) { + case CRED_USER: + i = parse_username(temp_val, parsed_info); + if (i) + goto return_i; + break; + case CRED_PASS: i = set_password(parsed_info, temp_val); if (i) - return i; - } - - /* parse domain */ - else if (strncasecmp("dom", line_buf + i, 3) == 0) { - temp_val = strchr(line_buf + i, '='); - if (temp_val) { - /* go past equals sign */ - temp_val++; - if (parsed_info->verboseflag) - fprintf(stderr, "\nDomain %s\n", - temp_val); - strlcpy(parsed_info->domain, temp_val, - sizeof(parsed_info->domain)); - } + goto return_i; + break; + case CRED_DOM: + if (parsed_info->verboseflag) + fprintf(stderr, "domain=%s\n", + temp_val); + strlcpy(parsed_info->domain, temp_val, + sizeof(parsed_info->domain)); + break; + case CRED_UNPARSEABLE: + if (parsed_info->verboseflag) + fprintf(stderr, + "Credential formatted incorrectly: %s", + temp_val); + break; } - } - fclose(fs); + i = 0; +return_i: + if (fs != NULL) + fclose(fs); + + /* make sure passwords are scrubbed from memory */ + if (line_buf != NULL) + memset(line_buf, 0, line_buf_size); SAFE_FREE(line_buf); - return 0; + return i; } static int