From 13298a58b39438ae9892194578b8b8f3d3b6013a Mon Sep 17 00:00:00 2001 From: Jameson Graef Rollins Date: Thu, 10 Jul 2008 16:50:05 -0400 Subject: [PATCH] Added file permission check function, and fixed bug in key writing for untranslated keys. --- debian/changelog | 4 ++- src/common | 67 ++++++++++++++++++++++++++++++++++++++--- src/monkeysphere | 13 +++++++- src/monkeysphere-server | 10 ++++++ 4 files changed, 88 insertions(+), 6 deletions(-) diff --git a/debian/changelog b/debian/changelog index 48af4d7..22f698b 100644 --- a/debian/changelog +++ b/debian/changelog @@ -11,8 +11,10 @@ monkeysphere (0.4-1) UNRELEASED; urgency=low * Add options for key generation and add-certifier functions. * Fix return codes for known_host and authorized_keys updating functions. + * Add write permission check on authorized_keys, known_hosts, and + authorized_user_ids files. - -- Jameson Graef Rollins Wed, 09 Jul 2008 19:39:44 -0400 + -- Jameson Graef Rollins Thu, 10 Jul 2008 16:47:17 -0400 monkeysphere (0.3-1) experimental; urgency=low diff --git a/src/common b/src/common index 240de38..6fc5f33 100644 --- a/src/common +++ b/src/common @@ -79,11 +79,15 @@ remove_line() { file="$1" string="$2" - # if the string is in the file and removed, return 0 + if [ -z "$file" -o -z "$string" ] ; then + return 1 + fi + + # if the string is in the file... if grep -q -F "$string" "$file" 2> /dev/null ; then + # remove the line with the string, and return 0 grep -v -F "$string" "$file" | sponge "$file" return 0 - # otherwise return 1 else return 1 @@ -114,6 +118,48 @@ test_gpg_expire() { echo "$1" | egrep -q "^[0-9][mwy]?$" } +# check that a file is properly owned, and that all it's parent +# directories are not group/other writable +check_key_file_permissions() { + local user + local path + local access + local gAccess + local oAccess + + # function to check that an octal corresponds to writability + is_write() { + [ "$1" -eq 2 -o "$1" -eq 3 -o "$1" -eq 6 -o "$1" -eq 7 ] + } + + user="$1" + path="$2" + + # return 0 is path does not exist + [ -e "$path" ] || return 0 + + owner=$(stat --format '%U' "$path") + access=$(stat --format '%a' "$path") + gAccess=$(echo "$access" | cut -c2) + oAccess=$(echo "$access" | cut -c3) + + # check owner + if [ "$owner" != "$user" -a "$owner" != 'root' ] ; then + return 1 + fi + + # check group/other writability + if is_write "$gAccess" || is_write "$oAccess" ; then + return 2 + fi + + if [ "$path" = '/' ] ; then + return 0 + else + check_key_file_permissions $(dirname "$path") + fi +} + ### CONVERSION UTILITIES # output the ssh key for a given key ID @@ -405,6 +451,10 @@ process_host_known_hosts() { keyid=$(echo "$line" | cut -d: -f2) sshKey=$(gpg2ssh "$keyid") + if [ -z "$sshKey" ] ; then + log " ! key could not be translated." + continue + fi # remove the old host key line, and note if removed remove_line "$KNOWN_HOSTS" "$sshKey" @@ -542,6 +592,10 @@ process_uid_authorized_keys() { keyid=$(echo "$line" | cut -d: -f2) sshKey=$(gpg2ssh "$keyid") + if [ -z "$sshKey" ] ; then + log " ! key could not be translated." + continue + fi # remove the old host key line remove_line "$AUTHORIZED_KEYS" "$sshKey" @@ -636,15 +690,20 @@ update_authorized_keys() { # process an authorized_user_ids file for authorized_keys process_authorized_user_ids() { local line + local nline local userIDs authorizedUserIDs="$1" log "processing authorized_user_ids file..." + nline=0 + # extract user IDs from authorized_user_ids file - for line in $(seq 1 $(meat "$authorizedUserIDs" | wc -l)) ; do - userIDs[$((line-1))]=$(cutline "$line" "$authorizedUserIDs") + IFS=$'\n' + for line in $(meat "$authorizedUserIDs") ; do + userIDs["$nline"]="$line" + nline=$((nline+1)) done update_authorized_keys "${userIDs[@]}" diff --git a/src/monkeysphere b/src/monkeysphere index a25fd6a..cfd5735 100755 --- a/src/monkeysphere +++ b/src/monkeysphere @@ -24,6 +24,9 @@ unset GREP_OPTIONS # default return code RETURN=0 +# set the file creation mask to be only owner rw +umask 077 + ######################################################################## # FUNCTIONS ######################################################################## @@ -204,6 +207,10 @@ case $COMMAND in 'update-known_hosts'|'update-known-hosts'|'k') MODE='known_hosts' + if ! check_key_file_permissions "$USER" "$KNOWN_HOSTS" ; then + failure "Improper permissions on known_hosts file." + fi + # if hosts are specified on the command line, process just # those hosts if [ "$1" ] ; then @@ -227,7 +234,11 @@ case $COMMAND in # fail if the authorized_user_ids file is empty if [ ! -s "$AUTHORIZED_USER_IDS" ] ; then - failure "$AUTHORIZED_USER_IDS is empty or does not exist." + failure "authorized_user_ids file '$AUTHORIZED_USER_IDS' is empty or does not exist." + fi + + if ! check_key_file_permissions "$USER" "$AUTHORIZED_USER_IDS" ; then + failure "Improper permissions on authorized_user_ids file." fi # process authorized_user_ids file diff --git a/src/monkeysphere-server b/src/monkeysphere-server index 9205b1d..a5497c2 100755 --- a/src/monkeysphere-server +++ b/src/monkeysphere-server @@ -141,6 +141,16 @@ update_users() { log "----- user: $uname -----" + if ! check_key_file_permissions "$uname" "$AUTHORIZED_USER_IDS" ; then + log "Improper permissions on authorized_user_ids file." + continue + fi + + if ! check_key_file_permissions "$uname" "$RAW_AUTHORIZED_KEYS" ; then + log "Improper permissions on authorized_keys file." + continue + fi + # make temporary directory TMPDIR=$(mktemp -d) -- 2.25.1