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

Test certificate generator doesn't create a certificate chain #622

Merged
merged 4 commits into from
Jul 24, 2021

Conversation

christopher-henderson
Copy link
Member

@christopher-henderson christopher-henderson commented Jul 17, 2021

This PR addresses #620 wherein @robplee identified that certs were incorrectly passed their own private key as the signing key, resulting in a "chain" of self signed certificates.

I also went ahead and knocked out some other issues such as the incorrectly setting BasicConstraintsValid = false for signing certificates and assigned broad validity dates.

@@ -334,3 +339,9 @@ var nextSerial = func() func() *big.Int {
// }
// return serial
// }

type Certificate struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be entirely personal preference/the conventions I'm used to but when I was looking for where this was defined I was expecting it at the top under the imports rather than at the bottom.

Happy to concede this point if I'm the only person who cares.

Copy link
Member Author

Choose a reason for hiding this comment

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

So my approach to this is based on a mix of locality and how "interesting" a definition is.

For example, if a packages structs are the main show for external consumers then I will order it as

struct A def
impl A def
  ...
  ...
  ...
struct B def
impl B def
  ...
  ...
  ...

If a type is very specific to a local use, then I feel fine keep its definition local to that use (even if its smack dab in the middle of the file)

func 
func 
func
struct def
func that uses the above def as a one-off
func
func

Now, to me, this struct def falls under the "just not very interesting" category. It's a way to shuttle around a parsed certificate alongside with its original key. It's more of an oddity then a feature of interest (at least to me).

@robplee
Copy link
Contributor

robplee commented Jul 19, 2021

Apart from my minor formatting request it looks good to me.

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.

2 participants