Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some work on swtpm_cert #206

Merged
merged 6 commits into from
Jan 30, 2020
Merged

Some work on swtpm_cert #206

merged 6 commits into from
Jan 30, 2020

Conversation

stefanberger
Copy link
Owner

@stefanberger stefanberger commented Jan 26, 2020

This series of patches allows passing of passwords to swtpm_cert using environment variables, converts the parsing of command line options to use getopt_long_only and allow certificate serial numbers with larger values (up to 64bit).

@coveralls
Copy link

coveralls commented Jan 26, 2020

Pull Request Test Coverage Report for Build 1761

  • 80 of 105 (76.19%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.02%) to 72.869%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/swtpm_cert/ek-cert.c 80 105 76.19%
Files with Coverage Reduction New Missed Lines %
src/swtpm_cert/ek-cert.c 1 74.77%
Totals Coverage Status
Change from base Build 1742: 0.02%
Covered Lines: 3497
Relevant Lines: 4799

💛 - Coveralls

@elmarco
Copy link
Contributor

elmarco commented Jan 27, 2020

Passing secrets via environment variable isn't ideal. We should use fd, like existing pwdfd

@stefanberger
Copy link
Owner Author

stefanberger commented Jan 27, 2020

Passing secrets via environment variable isn't ideal. We should use fd, like existing pwdfd

I primarily did this to accommodate the invocation via shell scripts , but I agree, we should have this as well. Another option to try for the shell script would be to pass it via file and then use this construct here:

function test() { echo $@; }
test <(echo -n "test")

Output: /dev/fd/63 (NetBSD, Linux, Cygwin, Apple, OpenBSD
Output: /tmp//sh-np.CVMkwU (named pipe): Dragonfly, FreeBSD, 

Besides this, I think it may be good to avoid introducing too many new command line options. So passing it via something like this here may be good too:

--signkey-password: <password>
--signkey-password: fd:<fd>
--signkey-password: file:<filename>
--signkey-password: password:<password>

@elmarco
Copy link
Contributor

elmarco commented Jan 27, 2020

--signkey-password: <password>
--signkey-password: fd:<fd>
--signkey-password: file:<filename>
--signkey-password: password:<password>

Looks ok, although not consistent with swtpm command line options. Although unlikely, this may also break existing users, perhaps add --signkey-pass/pwd ?

@stefanberger stefanberger changed the base branch from enable_spaces_in_paths to master January 27, 2020 15:49
@stefanberger
Copy link
Owner Author

@elmarco Thanks for the suggestion. Pushed an updated.

@stefanberger stefanberger force-pushed the swtpm_cert_work branch 8 times, most recently from fcb749c to 15fb3d5 Compare January 27, 2020 18:57
@stefanberger
Copy link
Owner Author

Also added support for --print-capabilities to swtpm_cert.

Allow passing signing key and parent key via files and file descriptors
and environment variables. Adapt a test case to exercise this new
functionality.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Use the swtpm_cert --signkey-pwd and --parentkey-pwd to pass key passwords
using files rather than using the command line options.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Add support for the --print-capabilities option to display newly
added capabilities. Adpat the man page and related test case.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Convert the code to use getopt_long_only for parsing the options.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Accept serial number that use up to 64bits.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
@elmarco
Copy link
Contributor

elmarco commented Jan 30, 2020

looks ok to me

@stefanberger stefanberger merged commit 28f1209 into master Jan 30, 2020
@stefanberger stefanberger deleted the swtpm_cert_work branch February 7, 2020 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants