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

Check account and cert keys for strength #156

Merged
merged 8 commits into from
May 11, 2015
Merged

Check account and cert keys for strength #156

merged 8 commits into from
May 11, 2015

Conversation

jsha
Copy link
Contributor

@jsha jsha commented May 6, 2015

Fixes #103
Fixes #116

@coveralls
Copy link

Coverage Status

Coverage increased (+1.01%) to 47.96% when pulling 0882e34 on goodkey into b1e3c37 on master.

rsaKey, ok := key.(*rsa.PublicKey)
if !ok {
if jwk, ok := key.(jose.JsonWebKey); ok && jwk.Rsa != nil {
rsaKey = jwk.Rsa
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to go ahead and call a new method like GoodKeyRsa() here and move the below out, and make a stub for GoodKeyEcc() that returns err.

@jcjones
Copy link
Contributor

jcjones commented May 8, 2015

I'm good with this merging as is if you open Issues for the two comments above, or resolve them before merging.

Overall, looks good to me!

@jsha jsha force-pushed the goodkey branch 2 times, most recently from ea457f7 to 0882e34 Compare May 9, 2015 18:37
@coveralls
Copy link

Coverage Status

Coverage increased (+0.43%) to 49.24% when pulling 0882e34 on goodkey into 8acae62 on master.

jsha added 2 commits May 9, 2015 11:48
Conflicts:
	ca/certificate-authority.go
	core/good_key.go
	core/good_key_test.go
@jsha
Copy link
Contributor Author

jsha commented May 9, 2015

Done.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.98%) to 49.78% when pulling 915956c on goodkey into 8acae62 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.05%) to 49.86% when pulling 915956c on goodkey into 8acae62 on master.

jcjones added a commit that referenced this pull request May 11, 2015
Check account and cert keys for strength
@jcjones jcjones merged commit 470125d into master May 11, 2015
@jcjones
Copy link
Contributor

jcjones commented May 11, 2015

All looks good; merging.

@jcjones jcjones deleted the goodkey branch May 11, 2015 22:14
@jcjones jcjones added this to the Security milestone May 11, 2015
@jsha jsha restored the goodkey branch May 29, 2015 19:05
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.

Require minimum key strength for account keys Check for weak keys
3 participants