build: Add configure flags for controlling tpm logging #1996
Conversation
|
What about Does it have run-time dependencies? |
|
|
||
| AC_ARG_ENABLE([tpm], | ||
| [AS_HELP_STRING([--enable-tpm], | ||
| [enable logging to TPM, use 'auto' to enable it if required development files are found, default: 'yes'])], |
Right. For now we just continue if we fail to log to tpm, so it still should work for users without TPM.
I guess that there is some daemon which handles writing stuff into TPM. See https://github.com/coreos/rkt/blob/master/pkg/tpm/tpm.go#L25 |
|
I don't know if we should merge this now because of #1816 (comment) |
krnowak
force-pushed
the
krnowak/enable-tpm-by-default
branch
from
2776960
to
acd80fc
Compare
| [AS_VAR_IF([HAVE_TPM],[no], | ||
| [AC_MSG_ERROR([*** TPM is enabled, but could not find required development files])]) | ||
| TPM_TAGS=tpm], | ||
| [AC_MSG_ERROR([*** Invalid value passed to --enable-tpm, should be either 'yes', 'no' or 'auto'])]) |
There was a problem hiding this comment.
Can ./configure say whether TPM is enabled or not? (AC_MSG_RESULT at the end)
There was a problem hiding this comment.
Ah, good point. Will do.
krnowak
force-pushed
the
krnowak/enable-tpm-by-default
branch
from
acd80fc
to
3c89fc7
Compare
|
Should |
|
|
||
| ## Security | ||
|
|
||
| There is only one security-related flag - to enable TPM for logging. |
There was a problem hiding this comment.
why the dash? maybe a semi-colon would be better?
|
Moving milestone for now, until we know the outcome of google/go-tspi#3 |
|
I don't care about this getting into 1.0 so I've bumped the milestone. I do, however, want a fix for #1816 to happen. |
|
Sorry, I was a little hasty and misunderstood. I think we can land this without being dependent on google/go-tspi#3 (rather, we could have #1816 in place). Moving back to 1.0.. |
krnowak
force-pushed
the
krnowak/enable-tpm-by-default
branch
from
3c89fc7
to
d68d4c3
Compare
|
Updated. The unanswered question for now is whether rkt binary in new releases should depend on trousers. |
|
This needs to be updated following the merge of this commit: 5e8ac50 |
krnowak
force-pushed
the
krnowak/enable-tpm-by-default
branch
from
d68d4c3
to
b278151
Compare
|
Updated. The unanswered question for now is whether rkt binary in new releases should depend on trousers. |
|
I think we should build without for now for the release on GitHub. But make it very clear how to enable it, and do so in the CoreOS build. |
|
|
||
| With this release, `rkt` RPM/dpkg packages should have the following updates: | ||
|
|
||
| - Pass `no` to the new configure flag `--enable-tpm`, if `rkt` should not use TPM. |
There was a problem hiding this comment.
Does it mean --enable-tpm=no? Or --disable-tpm? It is not so clear to me what "pass no" mean.
There was a problem hiding this comment.
Both forms are fine. But I'll be more explicit about it then.
|
Either here or in a following PR:
|
krnowak
force-pushed
the
krnowak/enable-tpm-by-default
branch
from
b278151
to
76eeff2
Compare
|
Bah, didn't notice the checkbox about TPM in |
|
LGTM and Semaphore is green. |
It is enabled by default to follow the secure-by-default principle. CI platforms are unlikely to have the headers installed, so we enable TPM conditionally.
krnowak
force-pushed
the
krnowak/enable-tpm-by-default
branch
from
76eeff2
to
89a9ce7
Compare
|
Updated with printing the features. The features also also printed in configure instead of "TPM enabled" now. |
krnowak
force-pushed
the
krnowak/enable-tpm-by-default
branch
from
89a9ce7
to
100105b
Compare
|
LGTM. For some reason, the green Semaphore logo has not been updated on this PR but Semaphore has completed with success: https://semaphoreci.com/coreos/rkt/branches/pull-request-1996/builds/8 |
build: Add configure flags for controlling tpm logging
It is enabled by default to follow the secure-by-default principle.
CI platforms are unlikely to have the headers installed, so we enable
TPM conditionally.
Fixes #1815.