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

Improve p12 passphrase generation #654

Closed
csirac2 opened this issue Aug 26, 2017 · 6 comments · Fixed by #697
Closed

Improve p12 passphrase generation #654

csirac2 opened this issue Aug 26, 2017 · 6 comments · Fixed by #697
Assignees

Comments

@csirac2
Copy link
Contributor

csirac2 commented Aug 26, 2017

See

easyrsa_p12_export_password: "{{ p12_export_password|default((ansible_date_time.iso8601_basic|sha1|to_uuid).split('-')[0]) }}"

Under the hood, ansible uses python's uuid5 function https://github.com/ansible/ansible/blob/7e71bdd3a0fd38b089464c884154e2152507d61f/lib/ansible/plugins/filter/core.py#L308

Which is implemented without adding any additional entropy of its own, unlike uuid4():
https://hg.python.org/cpython/file/2.7/Lib/uuid.py#l586

Which appears to result in deterministic output given the same input:

import uuid
>>> uuid.UUID(bytes='deadbeefdeadbeef', version=5)
UUID('64656164-6265-5566-a465-616462656566')
>>> uuid.UUID(bytes='deadbeefdeadbeef', version=5)
UUID('64656164-6265-5566-a465-616462656566')
>>> uuid.UUID(bytes='deadbeefdeadbeef', version=5)
UUID('64656164-6265-5566-a465-616462656566')

I could be wrong, but I worry that this reduces the entropy in p12 secrets to the time at which they were created

csirac2 added a commit to csirac2/algo that referenced this issue Aug 27, 2017
We're already doing it this way for CA_password, and ansible's to_uuid is problematic as it uses uuid v5 under the hood (trailofbits#654)
@csirac2 csirac2 changed the title Is the way we generate pk12 secrets deterministic? Improve pk12 passphrase generation Aug 27, 2017
@csirac2
Copy link
Contributor Author

csirac2 commented Aug 27, 2017

The main problem with #655 is that it retains the 32-bit passwords, which my i7 laptop with https://github.com/crackpkcs12/crackpkcs12 seems to get 7,000/sec with (about a week of walltime for 2 cores).

@csirac2 csirac2 changed the title Improve pk12 passphrase generation Improve p12 passphrase generation Aug 27, 2017
dguido pushed a commit that referenced this issue Aug 29, 2017
We're already doing it this way for CA_password, and ansible's to_uuid is problematic as it uses uuid v5 under the hood (#654)
csirac2 added a commit to csirac2/algo that referenced this issue Sep 3, 2017
This buys us an extra 16bits of password guessing entropy by expanding the characterset from hex to [a-zA-Z0-9_@]
jackivanov pushed a commit that referenced this issue Sep 29, 2017
This buys us an extra 16bits of password guessing entropy by expanding the characterset from hex to [a-zA-Z0-9_@]
njooma pushed a commit to njooma/algo that referenced this issue Oct 1, 2017
)

This buys us an extra 16bits of password guessing entropy by expanding the characterset from hex to [a-zA-Z0-9_@]
@jackivanov
Copy link
Collaborator

Taking to account that we store the p12 password in plaintext, does this issue make any sense?

@csirac2
Copy link
Contributor Author

csirac2 commented Oct 18, 2017

Am I the only one that doesn't use mobileconfigs? :D I traditionally rely on .p12 certs not being trivially crackable, but perhaps I'm weird.

@csirac2
Copy link
Contributor Author

csirac2 commented Oct 18, 2017

FWIW this issue is already resolved via #655 and #657.

@csirac2 csirac2 closed this as completed Oct 18, 2017
@jackivanov
Copy link
Collaborator

Yes, but we still need to get rid of plaintext passwords

@jackivanov jackivanov reopened this Oct 18, 2017
@csirac2
Copy link
Contributor Author

csirac2 commented Oct 18, 2017

Understood - is it worth addressing that in a separate issue?

eyecat pushed a commit to eyecat/algo that referenced this issue Oct 23, 2018
…ts#655)

We're already doing it this way for CA_password, and ansible's to_uuid is problematic as it uses uuid v5 under the hood (trailofbits#654)
eyecat pushed a commit to eyecat/algo that referenced this issue Oct 23, 2018
)

This buys us an extra 16bits of password guessing entropy by expanding the characterset from hex to [a-zA-Z0-9_@]
faf0 pushed a commit to faf0/algo that referenced this issue Dec 13, 2018
…ts#655)

We're already doing it this way for CA_password, and ansible's to_uuid is problematic as it uses uuid v5 under the hood (trailofbits#654)
faf0 pushed a commit to faf0/algo that referenced this issue Dec 13, 2018
)

This buys us an extra 16bits of password guessing entropy by expanding the characterset from hex to [a-zA-Z0-9_@]
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 a pull request may close this issue.

2 participants