Differential D23906

add password as a config option in Appx Package signing
ClosedPublic
Actions

Authored by brute4s99 on Sep 12 2019, 2:01 PM.

Details

Reviewers
vonreth
Summary

for AppxPackage Signing from within Craft.

some certificates might be secured by password. This patch extends Craft to support password-protected certificates

Diff Detail

Repository
R138 Craft
Branch
arcpatch-D23906
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 20897
Build 20915: arc lint + arc unit
brute4s99 requested review of this revision.Sep 12 2019, 2:01 PM
brute4s99 created this revision.
brute4s99 added a comment.Sep 18 2019, 11:30 PM

I for one use this :)

brute4s99 added a comment.EditedOct 27 2019, 10:43 PM

@vonreth mayI land this? :)

vonreth requested changes to this revision.Oct 28 2019, 1:29 PM

I won't accept this change.
Saving a password in config file offers some risk.
Also if its a self signed cert for testing, just create one without pw.

This revision now requires changes to proceed.Oct 28 2019, 1:29 PM
brute4s99 added a comment.Oct 28 2019, 2:23 PM

ah, right. Cleartext password thing didn't cross my mind.

brute4s99 updated this revision to Diff 71061.Dec 7 2019, 6:23 PM

I have re-implemented the patch, so the CLI asks for password at every sign() call.

vonreth requested changes to this revision.Dec 8 2019, 12:49 PM
vonreth added inline comments.
bin/utils.py
887

Maybe you could create a general implementation for promts like this in https://github.com/KDE/craft/blob/master/bin/Utils/CraftChoicePrompt.py (be sure to directly fail and not block in ci mode. ci mode would need another implementation)

It sould probably also create a notification to make aware of the promt.

This revision now requires changes to proceed.Dec 8 2019, 12:49 PM
brute4s99 updated this revision to Diff 71506.EditedDec 14 2019, 12:45 PM

What's new?

  • You can now add

Protected = True

in the CraftSettings.ini's Signing section to ask for password prompt if your signing certificate is password-protected.

Other than this, CraftChoicePrompt/py has now support for promptForPassword so password inputs can be done anywhere within Craft.

TBD

as you can see here ^, password is visible in the next logged command. How do we get rid of the password from that?

  • in-line comments
brute4s99 updated this revision to Diff 71507.Dec 14 2019, 12:47 PM
brute4s99 marked an inline comment as done.Dec 14 2019, 12:49 PM
brute4s99 added inline comments.
bin/Utils/CraftChoicePrompt.py
98

What is expected of Craft to do here?

102

Should we have this?

bin/utils.py
888

this shows the password in the compiled command (as shown in the comment with the diff)

Any ideas on how should we hide it?

vonreth added inline comments.Dec 15 2019, 12:53 PM
bin/Utils/CraftChoicePrompt.py
98

Well I'm not sure.. I hate the idea of managing it.
I guess the best would be to pass it with a special --option flag, that way the admin could use a secret provider from their ci platform. so if its not set just fail 🤷‍♀️

102

In a debug log probably.

bin/utils.py
888

You can pass kwargs["logCommand"] = False to the system call. then it will only show in verbose mode.

brute4s99 updated this revision to Diff 71624.Dec 15 2019, 8:05 PM

hid the logged command for signing when password protection is used for signing.

brute4s99 marked 4 inline comments as done.Dec 15 2019, 8:06 PM
brute4s99 added inline comments.
bin/Utils/CraftChoicePrompt.py
98

actually, I meant to ask how to 'fail' here. Could you help me with a short code snippet or something?

brute4s99 added inline comments.Dec 15 2019, 8:08 PM
bin/utils.py
889

I tried doing

kwargs["logCommand"] = False

here but it didn't work. I saw a similar thing in Craft.py, where you initialized an empty dict first and then started filling it, so I followed.

vonreth added inline comments.Dec 17 2019, 9:03 PM
bin/Utils/CraftChoicePrompt.py
98

raise Exception

bin/utils.py
889

I guess you need to always define it else you pass kwargs when its not defined.

kurrently its only defined if certProected is defined.

Or what exactly is the issue here.

brute4s99 added inline comments.Dec 17 2019, 9:07 PM
bin/utils.py
889

no issue, just something i noticed.

brute4s99 updated this revision to Diff 73206.Jan 10 2020, 3:15 PM

added exception for CI mode packaging

vonreth accepted this revision.Jan 10 2020, 5:31 PM
This revision is now accepted and ready to land.Jan 10 2020, 5:31 PM

Revision Contents
Changeset List

PathPackages
Mbin/Utils/CraftChoicePrompt.py (13 lines)
Mbin/utils.py (8 lines)

Diff 73206

View Options

bin/Utils/CraftChoicePrompt.py

Loading...
View Options

bin/utils.py

Loading...