Added file permission check function, and fixed bug in key writing for
authorJameson Graef Rollins <jrollins@phys.columbia.edu>
Thu, 10 Jul 2008 20:50:05 +0000 (16:50 -0400)
committerJameson Graef Rollins <jrollins@phys.columbia.edu>
Thu, 10 Jul 2008 20:50:05 +0000 (16:50 -0400)
untranslated keys.

debian/changelog
src/common
src/monkeysphere
src/monkeysphere-server

index 48af4d760e35551b6ed8a27f8cd2934c32d84cb2..22f698bd3d17fb77c064672860538a8ee02e1a34 100644 (file)
@@ -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 <jrollins@phys.columbia.edu>  Wed, 09 Jul 2008 19:39:44 -0400
+ -- Jameson Graef Rollins <jrollins@phys.columbia.edu>  Thu, 10 Jul 2008 16:47:17 -0400
 
 monkeysphere (0.3-1) experimental; urgency=low
 
index 240de38886f5ae39568cf96cb23099946253bbe7..6fc5f3318d10dc39f4482ddf6efccea44003d7f5 100644 (file)
@@ -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[@]}"
index a25fd6a64257a780fc6628842f119218a1f198ee..cfd57357c6e9c4906ea3e14b1c8f9b43808c5af2 100755 (executable)
@@ -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
index 9205b1d315bce9b59cdfecb85df952012425d38a..a5497c29e06a36b94e827a9407dc8a4095d35184 100755 (executable)
@@ -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)