Yedidyah Bar David has posted comments on this change. Change subject: pki: enforce lock file permissions same as ca private key ......................................................................
Patch Set 4: (2 comments) http://gerrit.ovirt.org/#/c/25629/4/packaging/bin/pki-enroll-request.sh File packaging/bin/pki-enroll-request.sh: Line 101: # create lock file if not already exists Line 102: # make sure it is world readable so we can Line 103: # lock file by any user. Line 104: if ! [ -f "${LOCKFILE}" ]; then Line 105: touch "${LOCKFILE}.tmp" || die "Cannot create lockfile '${LOCKFILE}.tmp'" > this is not atomic/reentrant. > not important if not regular file as long as permissions are ok. But I remind you that my own problem wasn't with DOS by locking the file. It was a symlink attack causing us to change permissions to some other file. Line 106: chown --reference="${LOCKFILE_REF}" "${LOCKFILE}.tmp" || die "Cannot set ownership of lockfile '${LOCKFILE}.tmp'" Line 107: chmod --reference="${LOCKFILE_REF}" "${LOCKFILE}.tmp" || die "Cannot set permissions of lockfile '${LOCKFILE}.tmp'" Line 108: mv "${LOCKFILE}.tmp" "${LOCKFILE}" || die "Cannot create lockfile '${LOCKFILE}'" Line 109: fi http://gerrit.ovirt.org/#/c/25629/4/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/pki/upgrade.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/pki/upgrade.py: Line 42: # private key. Line 43: # Remove lock file, it will be created Line 44: # at next attempt. Line 45: # Line 46: if os.path.exists(osetupcons.FileLocations.OVIRT_ENGINE_PKI_LOCKFILE): > it is removed, why is this important? os.path.exists(symlink to a non-existant target) returns False. So you do nothing. Not sure what you intended to do in this case. Line 47: os.unlink(osetupcons.FileLocations.OVIRT_ENGINE_PKI_LOCKFILE) Line 48: Line 49: # Line 50: # Since 3.0 we relayed on directory -- To view, visit http://gerrit.ovirt.org/25629 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I89d1bee3c7fff1bae2ee555d556e35171bef612c Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Sandro Bonazzola <[email protected]> Gerrit-Reviewer: Yedidyah Bar David <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
