Mimi,
On Sun, Jul 28, 2019 at 09:49:04PM -0400, Mimi Zohar wrote:
> On Mon, 2019-07-29 at 02:40 +0300, Vitaly Chikunov wrote:
> > > The functions "pos" and "neg" are written very concisely, but they are
> > > part of a common set of functions, which are the crux of the tests
> > > scripts. I'm really hesitant about having common functions that
> > > execute any command passed to it, without any form of verification.
> >
> > What verification and why?
>
> Even though the tests are not running as root, it's still executing
> "$@", whatever that might be. For ima_hash.test, the first argument
> is "check".
I believe this will not add any security at all. As these are purely
internal functions. If somebody can call pos/neg they are already able
to call just anything they want. This is like protecting awk from
running something somehow not verified.
> > > > +_enable_gost_engine() {
> > > > + # Do not enable if it's already working (enabled by user)
> > > > + if ! openssl md_gost12_256 /dev/null >/dev/null 2>&1 \
> > > > + && openssl engine gost >/dev/null 2>&1; then
> > > > + ENGINE=gost
> > > > + fi
> > > > +}
> > >
> > > With gost provided as an Openssl engine, is it possible to be able to
> > > execute the first command without the gost engine enabled? With
> > > commit 782224f33cd7 ("ima-evm-utils: Rework openssl init"),
> >
> > I don't understand question. What is 'first command'? `openssl
> > md_gost12_256` will not work if gost-engine is not configured somehow.
>
> Exactly. "openssl md_gost12_256 /dev/null" (returns 0, but is
> negated) will succeed only if the engine is enabled. The "openssl
> engine gost" test will never fail.
It can return non 0 and check looks correct. For example:
debian:~$ openssl md_gost12_256 /dev/null
Invalid command 'md_gost12_256'; type "help" for a list.
debian:~$ echo $?
1
If there is gost-engine in the system but it's not enabled via config - it will
be enabled for tests via `--engine gost' option.
> > > I'm now wondering if the "--engine e' option is still needed?
> >
> > It's needed. Why you thinking it doesn't? Commit 782224f33cd7 will not
> > load gost (or any other) engine on its own.
>
> If the gost engine is enabled in openssl.cnf then we don't need to set
> "ENGINE=gost". I'm obviously missing something here.
There is two ways to load engine in openssl. I think it is mentioned in
the commit messages when this functionality was added to evmctl.
1. One way is via careful editing of openssl config.
2. Another is simply via --engine option.
Both ways are supported by openssl tools. (For example curl supports
--engine option.) But for tests I thought it would be simpler to use
--engine option instead of generating openssl.cnf.
If user have already configured, for example system wide, openssl.cnf to
load gost-engine this test will not add any option. I think this is
flexible.
> > (check* was supposed to be top-level tests. I will change this in v3.)
> >
> > > > + local alg=$1 pref=$2 hash=$3 file=$4
> > > > +
> > > > + FOR=$alg DEL=$file
> > >
> > > Why not use ALG=$alg and FILE=$file as the global variable names?
> >
> > check was called once for every algo. Are you proposing to change
> > call like
>
> Although "FOR" is capitalized, I was reading it as "for". It took me
> a while to realize that "FOR" and "DEL" are global variables being
> used in "_evmctl_run". Anything you can do to make it easier to read
> would be appreciated. Just adding comments would help.
OK
> While further testing, "_require evmctl openssl" should also make sure
> that getfattr is installed.
Thanks!
