Re: [PATCH v8 2/2] ima-evm-utils: Add sign/verify tests for evmctl

From: Mimi Zohar <zohar@linux.ibm.com>
To: Vitaly Chikunov <vt@altlinux.org>,
	Mimi Zohar <zohar@linux.vnet.ibm.com>,
	Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
	linux-integrity@vger.kernel.org
Subject: Re: [PATCH v8 2/2] ima-evm-utils: Add sign/verify tests for evmctl
Date: Wed, 01 Apr 2020 14:00:55 -0400	[thread overview]
Message-ID: <1585764055.5188.652.camel@linux.ibm.com> (raw)
In-Reply-To: <20200327042515.22315-3-vt@altlinux.org>

On Fri, 2020-03-27 at 07:25 +0300, Vitaly Chikunov wrote:

<snip>

> +# Perform sign and verify ima and evm testing
> +sign_verify() {
> +  local key=$1 alg=$2 pref="$3" opts="$4"
> +  local file=$alg.txt
> +
> +  # Set defaults:
> +  # Public key is different for v1 and v2 (where x509 cert is used).
> +  if [[ $opts =~ --rsa ]]; then
> +    KEY=test-$key.pub
> +  else
> +    KEY=test-$key.cer
> +  fi
> +  ALG=$alg
> +  PREF=$pref
> +  OPTS=$opts
> +  FILE=$file
> +
> +  TYPE=ima
> +  if expect_pass check_sign; then
> +
> +    # Normal verify with proper key should pass
> +    expect_pass check_verify
> +
> +    # Multiple files and some don't verify
> +    expect_fail check_verify FILE=/dev/null,$file

The comment and the code don't seem to be in sync.  This seems to be a
single file, for example, named "/dev/null,sha1.txt", which properly
fails.

____ START positive test: check_verify
+ evmctl -vvv ima_verify --key test-rsa1024.pub --xattr-user --rsa
sha1.txt
  hash-v1: da39a3ee5e6b4b0d3255bfef95601890afd80709
  sighash: e34807780b93cc17bdac89573df40ee4a3e984f1
  sha1.txt: verification is OK
^^^^ STOP (0) positive test: check_verify
____ START negative test: check_verify FILE=/dev/null,sha1.txt
- evmctl -vvv ima_verify --key test-rsa1024.pub --xattr-user --rsa
/dev/null,sha1.txt
evmctl ima_verify failed properly with (1)
  getxattr failed: /dev/null,sha1.txt
  errno: No such file or directory (2)
^^^^ STOP (1) negative test: check_verify FILE=/dev/null,sha1.txt

> +  fi
> +
> +  TYPE=evm
> +  # Avoid running blkid for evm tests which may require root
> +  # No generation on overlayfs:
> +  # ioctl(3, FS_IOC_GETVERSION, 0x7ffd8e0bd628) = -1 ENOTTY (Inappropriate ioctl for device)
> +  OPTS="$opts --uuid --generation 0"
> +  if expect_pass check_sign; then
> +
> +    # Normal verify with proper key
> +    expect_pass check_verify
> +
> +    # Verify with wrong key
> +    expect_fail check_verify KEY=rsa2048
> +  fi
> +
> +  # Note: Leaving TYPE=evm and file is evm signed
> +}
> +
> +# Test --keys
> +try_different_keys() {
> +  # This run after sign_verify which leaves
> +  # TYPE=evm and file is evm signed
> +
> +  # v2 signing can work with multiple keys in --key option
> +  if [[ ! $OPTS =~ --rsa ]]; then
> +
> +    # Have correct key in the key list
> +    expect_pass check_verify KEY=test-rsa2048.cer,$KEY
> +    expect_pass check_verify KEY=/dev/null,$KEY,

First test has multiple keys in the key list.  The key list with
"/dev/null" obviously fails to add the first key, so it lands up being
a single key on the list.

> +  fi
> +
> +  # Try key that is not used for signing
> +  expect_fail check_verify KEY=rsa2048
> +
> +  # Try completely wrong key files
> +  expect_fail check_verify KEY=/dev/null
> +  expect_fail check_verify KEY=/dev/zero
> +}
> +
> +try_different_sigs() {
> +  # TYPE=evm and file is evm signed
> +
> +  # Test --imasig
> +  if expect_pass check_sign OPTS="$OPTS --imasig"; then
> +
> +    # Verify both evm and ima sigs
> +    expect_pass check_verify
> +    expect_pass check_verify TYPE=ima
> +  fi
> +
> +  # Test --imahash
> +  if expect_pass check_sign OPTS="$OPTS --imahash"; then
> +
> +    expect_pass check_verify
> +
> +    # IMA hash is not verifiable by ima_verify
> +    expect_fail check_verify TYPE=ima
> +  fi
> +
> +  # Test --portable
> +  expect_pass check_sign OPTS="$OPTS --portable" PREF=0x05
> +  # Cannot be verified

True, evmctl does not support verifying portable signatures, but it
should be possible not only locally, but remotely to verify a portable
signature.  That's the whole point of having portable EVM signatures.
 The comment is a bit misleading and could say something to that
effect - "todo: add support for evmctl portable signature
verification".

Mimi