From: Vitaly Chikunov <vt@altlinux.org>
To: Mimi Zohar <zohar@linux.ibm.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>,
Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
linux-integrity@vger.kernel.org, Petr Vorel <pvorel@suse.cz>,
"Bruno E. O. Meneguele" <bmeneg@redhat.com>
Subject: Re: [PATCH v2] ima-evm-utils: Add some tests for evmctl
Date: Mon, 29 Jul 2019 07:09:57 +0300 [thread overview]
Message-ID: <20190729040956.bbprdbpas46mjcgh@altlinux.org> (raw)
In-Reply-To: <1564364944.4245.452.camel@linux.ibm.com>
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!
prev parent reply other threads:[~2019-07-29 4:10 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-27 4:41 [PATCH v2] ima-evm-utils: Add some tests for evmctl Vitaly Chikunov
2019-07-28 0:25 ` Vitaly Chikunov
2019-07-28 17:17 ` Mimi Zohar
2019-07-28 23:40 ` Vitaly Chikunov
2019-07-29 1:49 ` Mimi Zohar
2019-07-29 4:09 ` Vitaly Chikunov [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190729040956.bbprdbpas46mjcgh@altlinux.org \
--to=vt@altlinux.org \
--cc=bmeneg@redhat.com \
--cc=dmitry.kasatkin@gmail.com \
--cc=linux-integrity@vger.kernel.org \
--cc=pvorel@suse.cz \
--cc=zohar@linux.ibm.com \
--cc=zohar@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).