install_with_python: Secure Python download with sha256 checks.
Martin Schwenke
martin at meltin.net
Tue Jun 23 18:49:00 MDT 2015
On Tue, 23 Jun 2015 22:45:40 +0200, Christian Ambach <ambi at samba.org>
wrote:
> Am 19.06.15 01:13, schrieb Adrian Cochrane:
> > O.K. Here's an update to my patch which removes the tempfile specifying
> > the hash to check against. Shouldn't really matter as the containing
> > directory is removed anyways. And while I was at it I added md5 support.
>
> > + md5sum --status -c checksums.md5
> Isnt't there a || exit 1 missing here to abort the processing when the
> check fails?
>
> >
> > + sha256sum --status -c checksums.sha256 || exit 1
> > + rm checksums.sha256
>
> If the check fails, the file will not be removed.
> But looking at the surrounding code, the same problem is true for nearly
> everything else that happens in do_install_python() as
> the check_something || exit 1 pattern is used consistently.
> But exit 1 makes the shell exit immediately and the final cleanup
> > rm -rf python_install
> in line 45 will only be reached if everything worked fine.
>
> I am not sure how to realize it in a shell script, but using the same
> technique that is used in Samba's portions written in C would make this
> look much cleaner (and less polluted by all of the exit statements).
> Something in the line of
> =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
> if command -v sha256sum
> then
> echo "xxx Python-2.6.5.tar" > checksums.sha256
> sha256sum --status -c checksums.sha256 || goto error
> else
> echo "c83c... Python-2.6.5.tar" > checksums.md5
> md5sum --status -c checksums.md5 || goto error
> fi
>
> :error
>
> :out
> cd ..
> rm -rf python_install
>
> =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
>
> What do you think?
One way of making the code cleaner might be to use "set -e" so that the
script exits on the first command failure and to use "trap" for the
cleanup. Here's an example:
##############################
#!/bin/sh
mycleanup ()
{
echo "Doing cleanup"
}
trap mycleanup 0
false
##############################
You just have to be careful in the script so that any expected command
"failures" are caught. For example:
thing_that_might_fail_but_is_ok || true
There shouldn't be too many of those... ;-)
You might also want the cleanup to run on other signals (e.g. INT
TERM).
peace & happiness,
martin
More information about the samba-technical
mailing list