test: unit: tcti-libtpms: fix test failed at 32-bit platforms.

[grandpaul]

When running the test on 32-bit platforms, we should not convert pointers to integers. expect_value() will convert the cmd to an integer value thus we should use expect_memory() instead.

[grandpaul]

This bug was reported in Debian. The link: https://bugs.debian.org/1040521

[joholl]

Thanks @grandpaul. However, this does not address the issue completely.

I expected expect_value() to be able to compare pointers on all platforms, which clearly is not the case. expect_memory() compares the buffers - which works. However, I actually want to compare pointers.

Also, there are much more calls like that in here:

expect_value(TPMLIB_Process, cmd, cmd);

I hope I have some more time for this tomorrow.

Hasty judgement

[joholl]

Ok, I was able to reproduce the problem.

Docker file

This is based on ubuntu-18.04.docker, but 32 bit. I exchanged the FROM line and commented the irrelevant stuff out (got some errors).

#line 1 "ubuntu-18.04.docker.m4"
# Ubuntu 18.04 docker file
#
FROM i386/ubuntu

LABEL org.opencontainers.image.source https://github.com/tpm2-software/tpm2-software-container

ENV DEBIAN_FRONTEND=noninteractive
RUN apt-get update && \
  apt-get install -y \
  autoconf-archive \
  curl \
  libcmocka0 \
  libcmocka-dev \
  net-tools \
  build-essential \
  git \
  pkg-config \
  gcc \
  g++ \
  m4 \
  libtool \
  automake \
  libgcrypt20-dev \
  libssl-dev \
  autoconf \
  gnulib \
  wget \
  doxygen \
  libdbus-1-dev \
  libglib2.0-dev \
  clang-9 \
  clang-tools-9 \
  pandoc \
  lcov \
  libcurl4-openssl-dev \
  dbus-x11 \
  vim-common \
  libsqlite3-dev \
  iproute2 \
  libtasn1-6-dev \
  socat \
  libseccomp-dev \
  expect \
  gawk \
  libjson-c-dev \
  libengine-pkcs11-openssl \
  default-jre \
  default-jdk \
  sqlite3 \
  libnss3-tools \
  libyaml-dev \
  uuid-dev \
  opensc \
  gnutls-bin \
  rustc \
  acl \
  libjson-glib-dev \
  libusb-1.0-0-dev \
  libftdi-dev

RUN update-alternatives --install /usr/bin/clang clang /usr/bin/clang-9 100
RUN update-alternatives --install /usr/bin/scan-build scan-build /usr/bin/scan-build-9 100

#line 1 "modules/autoconf.m4"
ARG autoconf_archive=autoconf-archive-2018.03.13
RUN cd /tmp \
  && wget --quiet --show-progress --progress=dot:giga "http://mirror.kumi.systems/gnu/autoconf-archive/$autoconf_archive.tar.xz" \
  && tar -xf $autoconf_archive.tar.xz \
  && rm $autoconf_archive.tar.xz \
  && cd $autoconf_archive \
  && ./configure --prefix=/usr \
  && make -j $(nproc) && make install \
  && rm -fr /tmp/$autoconf_archive.tar.xz /tmp/$autoconf_archive

#line 64 "ubuntu-18.04.docker.m4"

#line 1 "modules/ibmtpm1637.m4"
ARG ibmtpm_name=ibmtpm1637
RUN cd /tmp \
  && wget $WGET_EXTRA_FLAGS -L "https://downloads.sourceforge.net/project/ibmswtpm2/$ibmtpm_name.tar.gz" \
  && sha256sum $ibmtpm_name.tar.gz | grep ^dd3a4c3f7724243bc9ebcd5c39bbf87b82c696d1c1241cb8e5883534f6e2e327 \
  && mkdir -p $ibmtpm_name \
  && tar xv --no-same-owner -f $ibmtpm_name.tar.gz -C $ibmtpm_name \
  && rm $ibmtpm_name.tar.gz \
  && cd $ibmtpm_name/src \
  && sed -i 's/-DTPM_NUVOTON/-DTPM_NUVOTON $(CFLAGS)/' makefile \
  && CFLAGS="-DNV_MEMORY_SIZE=32768 -DMIN_EVICT_OBJECTS=7" make -j$(nproc) \
  && cp tpm_server /usr/local/bin \
  && rm -fr /tmp/$ibmtpm_name
#line 65 "ubuntu-18.04.docker.m4"

#line 1 "modules/python3.7.2.m4"
#ARG pyver="3.7.2"
#RUN cd /tmp \
#	&& wget --quiet --show-progress --progress=dot:giga "https://github.com/python/cpython/archive/v${pyver}.tar.gz" \
#	&& tar -xf v${pyver}.tar.gz \
#	&& cd cpython-${pyver}/ \
#	&& ./configure \
#	&& make -j$(nproc) \
#	&& make altinstall \
#	&& rm -fr /tmp/v${pyver}.tar.gz /tmp/cpython-${pyver}

#RUN update-alternatives --install "/usr/bin/python3" "python3" "$(which python3.7)" 100
#RUN update-alternatives --install "/usr/bin/pip3" "pip3" "$(which pip3.7)" 100
#line 66 "ubuntu-18.04.docker.m4"

#line 1 "modules/pip3.m4"
#
# upgrade pip first so packages are not reinstalled using a version other than what may have been specified
#
#RUN python3 -m pip install --upgrade pip
## install everything in one shot so we don't get a newer version of a package we specified. Ie if a module has dep on cryptogtraphy
## and we install it in different phases pip will upgrade cryptography
#RUN pkgs="cryptography==$PYCRYPTO_VERSION pyyaml cpp-coveralls pyasn1 pyasn1_modules python-pkcs11 \
#          bcrypt==$PYBCRYPT_VERSION setuptools"; \
#    pkgs=$(echo "$pkgs" | sed -E 's/==\s+/ /g'); \
#    python3 -m pip install $pkgs
##line 67 "ubuntu-18.04.docker.m4"


#line 1 "modules/swtpm.m4"
RUN git -C /tmp clone --depth=1 https://github.com/stefanberger/libtpms.git \
  && cd /tmp/libtpms \
  && ./autogen.sh --prefix=/usr $LIBTPMS_AUTOGEN_EXTRA --with-openssl --with-tpm2 \
  && make -j$(nproc) \
  && make install \
  && rm -fr /tmp/libtpms \
  && git -C /tmp clone --depth=1 https://github.com/stefanberger/swtpm.git \
  && cd /tmp/swtpm \
  && ./autogen.sh --prefix=/usr \
  && make -j$(nproc) $SWTPM_MAKE_EXTRA \
  && make install \
  && rm -fr /tmp/swtpm
#line 69 "ubuntu-18.04.docker.m4"

#line 1 "modules/uthash.m4"
ARG uthash="2.1.0"
RUN cd /tmp \
  && wget $WGET_EXTRA_FLAGS -L  "https://github.com/troydhanson/uthash/archive/v${uthash}.tar.gz" \
  && tar -xf v${uthash}.tar.gz \
  && cp uthash-${uthash}/src/*.h /usr/include/ \
  && rm -fr /tmp/v${uthash}.tar.gz /tmp/uthash-${uthash}
#line 70 "ubuntu-18.04.docker.m4"

#line 1 "modules/junit.m4"
#ARG jver="4.13"
#ARG hver="2.2"
#RUN mkdir -p /java \
#	&& wget $WGET_EXTRA_FLAGS -L -O /java/junit.jar "https://search.maven.org/remotecontent?filepath=junit/junit/4.13/junit-${jver}.jar" \
#	&& wget $WGET_EXTRA_FLAGS -L -O /java/hamcrest.jar "https://search.maven.org/remotecontent?filepath=org/hamcrest/hamcrest/${hver}/hamcrest-${hver}.jar"
#
#ENV CLASSPATH=/java/hamcrest.jar:/java/junit.jar
##line 71 "ubuntu-18.04.docker.m4"

WORKDIR /

Build:

docker build --platform linux/i386 -t tpm2-tss32 -f ubuntu-18.04.docker .                                                                      

Run:

docker run --cap-add=SYS_PTRACE --env-file .ci/docker.env -v "$PWD:/workspace/tpm2-tss" "tpm2-tss32" sh -c "/workspace/tpm2-tss/.ci/docker.run"

(I also had to comment out test/unit/esys-vendor in Makefile-test.am. Seems to be another bug I currently don't have the time for.)

With the following changes, the test runs green. I am not sure how to properly compare pointers using cmocka assert macros, instead... which is what we actually want to do.

Potential Fix

diff --git a/test/unit/tcti-libtpms.c b/test/unit/tcti-libtpms.c
index 823cc0d76cb9..3402bb2000e2 100644
--- a/test/unit/tcti-libtpms.c
+++ b/test/unit/tcti-libtpms.c
@@ -899,7 +899,7 @@ tcti_libtpms_locality_success_test(void **state)
   rc = Tss2_Tcti_SetLocality(ctx, 4);
   assert_int_equal(rc, TSS2_RC_SUCCESS);

-    expect_value(TPMLIB_Process, cmd, cmd);
+    expect_memory(TPMLIB_Process, cmd, cmd, sizeof(cmd));
   expect_value(TPMLIB_Process, cmd_len, sizeof(cmd));
   expect_value(TPMLIB_Process, locality, 4); /* expect locality 4 */
   will_return(TPMLIB_Process, rsp);
@@ -919,7 +919,7 @@ tcti_libtpms_transmit_success_test(void **state)
   unsigned char cmd[] = {0x80, 0x01, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x01, 0x44, 0x00, 0x00};
   unsigned char rsp[] = {0x80, 0x01, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x00};

-    expect_value(TPMLIB_Process, cmd, cmd);
+    expect_memory(TPMLIB_Process, cmd, cmd, sizeof(cmd));
   expect_value(TPMLIB_Process, cmd_len, sizeof(cmd));
   expect_value(TPMLIB_Process, locality, 0);
   will_return(TPMLIB_Process, rsp);
@@ -994,7 +994,7 @@ tcti_libtpms_remap_state_success_test(void **state)
   unsigned char cmd[] = {0x80, 0x01, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x01, 0x44, 0x00, 0x00};
   unsigned char rsp[] = {0x80, 0x01, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x00};

-    expect_value(TPMLIB_Process, cmd, cmd);
+    expect_memory(TPMLIB_Process, cmd, cmd, sizeof(cmd));
   expect_value(TPMLIB_Process, cmd_len, sizeof(cmd));
   expect_value(TPMLIB_Process, locality, 0);
   will_return(TPMLIB_Process, rsp);
@@ -1051,7 +1051,7 @@ tcti_libtpms_remap_state_mremap_fail_test(void **state)
   unsigned char cmd[] = {0x80, 0x01, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x01, 0x44, 0x00, 0x00};
   unsigned char rsp[] = {0x80, 0x01, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x00};

-    expect_value(TPMLIB_Process, cmd, cmd);
+    expect_memory(TPMLIB_Process, cmd, cmd, sizeof(cmd));
   expect_value(TPMLIB_Process, cmd_len, sizeof(cmd));
   expect_value(TPMLIB_Process, locality, 0);
   will_return(TPMLIB_Process, rsp);
@@ -1094,7 +1094,7 @@ tcti_libtpms_remap_state_posix_fallocate_fail_test(void **state)
   unsigned char cmd[] = {0x80, 0x01, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x01, 0x44, 0x00, 0x00};
   unsigned char rsp[] = {0x80, 0x01, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x00};

-    expect_value(TPMLIB_Process, cmd, cmd);
+    expect_memory(TPMLIB_Process, cmd, cmd, sizeof(cmd));
   expect_value(TPMLIB_Process, cmd_len, sizeof(cmd));
   expect_value(TPMLIB_Process, locality, 0);
   will_return(TPMLIB_Process, rsp);
@@ -1156,7 +1156,7 @@ tcti_libtpms_no_statefile_success_test(void **state)
   unsigned char rsp_out[sizeof(rsp)];
   size_t rsp_len_out = sizeof(rsp);

-    expect_value(TPMLIB_Process, cmd, cmd);
+    expect_memory(TPMLIB_Process, cmd, cmd, sizeof(cmd));
   expect_value(TPMLIB_Process, cmd_len, sizeof(cmd));
   expect_value(TPMLIB_Process, locality, 0);
   will_return(TPMLIB_Process, rsp);
@@ -1210,7 +1210,7 @@ tcti_libtpms_two_states_no_statefiles_success_test(void **state)
   tcti_common[1] = tcti_common_context_cast(ctxs[1]);

   /* ===== transmit on instance 0 ===== */
-    expect_value(TPMLIB_Process, cmd, cmd_aa);
+    expect_memory(TPMLIB_Process, cmd, cmd_aa, sizeof(cmd_aa));
   expect_value(TPMLIB_Process, cmd_len, sizeof(cmd_aa));
   expect_value(TPMLIB_Process, locality, 0);
   will_return(TPMLIB_Process, rsp_aa);
@@ -1231,7 +1231,7 @@ tcti_libtpms_two_states_no_statefiles_success_test(void **state)
   assert_int_equal(tcti_libtpms[0]->state_len, 0);

   /* ===== transmit on instance 1 ===== */
-    expect_value(TPMLIB_Process, cmd, cmd_bb);
+    expect_memory(TPMLIB_Process, cmd, cmd_bb, sizeof(cmd_bb));
   expect_value(TPMLIB_Process, cmd_len, sizeof(cmd_bb));
   expect_value(TPMLIB_Process, locality, 0);
   will_return(TPMLIB_Process, rsp_bb);
@@ -1308,7 +1308,7 @@ tcti_libtpms_two_states_success_test(void **state)
   tcti_common[1] = tcti_common_context_cast(ctxs[1]);

   /* ===== transmit on instance 0 ===== */
-    expect_value(TPMLIB_Process, cmd, cmd_aa);
+    expect_memory(TPMLIB_Process, cmd, cmd_aa, sizeof(cmd_aa));
   expect_value(TPMLIB_Process, cmd_len, sizeof(cmd_aa));
   expect_value(TPMLIB_Process, locality, 0);
   will_return(TPMLIB_Process, rsp_aa);
@@ -1336,7 +1336,7 @@ tcti_libtpms_two_states_success_test(void **state)
   assert_memory_equal(tcti_libtpms[0]->state_mmap, S1_STATE, S1_STATE_LEN);

   /* ===== transmit on instance 1 ===== */
-    expect_value(TPMLIB_Process, cmd, cmd_bb);
+    expect_memory(TPMLIB_Process, cmd, cmd_bb, sizeof(cmd_bb));
   expect_value(TPMLIB_Process, cmd_len, sizeof(cmd_bb));
   expect_value(TPMLIB_Process, locality, 0);
   will_return(TPMLIB_Process, rsp_bb);

[codecov]

Codecov Report

Merging #2649 (dae6bf5) into master (1a713d5) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2649      +/-   ##
==========================================
- Coverage   82.55%   82.54%   -0.01%     
==========================================
  Files         366      368       +2     
  Lines       42477    42938     +461     
==========================================
+ Hits        35067    35445     +378     
- Misses       7410     7493      +83     

see 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[joholl]

Not perfect, but better than buggy.

Thanks @grandpaul!