Yedidyah Bar David has posted comments on this change. Change subject: pki: enforce lock file permissions same as ca private key ......................................................................
Patch Set 4: (3 comments) http://gerrit.ovirt.org/#/c/25629/4//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-03-11 19:32:18 +0200 Line 4: Commit: Alon Bar-Lev <[email protected]> Line 5: CommitDate: 2014-03-11 21:02:25 +0200 Line 6: Line 7: pki: enforce lock file permissions same as ca private key I'd add: and being a regular file Line 8: Line 9: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1075209 Line 10: Change-Id: I89d1bee3c7fff1bae2ee555d556e35171bef612c 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'" What if ${LOCKFILE}.tmp exists? What if it's a symlink? I do not see the point in using .tmp . Perhaps: touch $LOCKFILE || die [ -L $LOCKFILE ] && die "symlink" # Or: [ -f $LOCKFILE ] || die "not a regular file" # Then chmod etc 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): What if it's a symlink? 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
