Skip to content

Add CertsSelector option #147

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

Merged
merged 3 commits into from
Mar 6, 2021

Conversation

omorsi
Copy link
Contributor

@omorsi omorsi commented Mar 5, 2021

As mentioned in #145, some CAs don't have KeyUsageKeyEncipherment key usage set in their certificates. This PR introduces CertsSelector which should make it easier for clients that want to enforce a custom criteria on how to filter certificates.

@omorsi
Copy link
Contributor Author

omorsi commented Mar 5, 2021

@jessepeterson WDYT?

@omorsi
Copy link
Contributor Author

omorsi commented Mar 5, 2021

@pavolmarko FYI!

}

// NopCertsSelector is a CertsSelector that does not do anything.
type NopCertsSelector struct{}
Copy link
Member

@jessepeterson jessepeterson Mar 5, 2021

Choose a reason for hiding this comment

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

Just a nit, but I kinda like the HandlerFunc pattern over the empty structs. I.e.:

type CertsSelectorFunc func([]*x509.Certificate) []*x509.Certificate

func (f *CertsSelectorFunc) SelectCerts(certs []*x509.Certificate) []*x509.Certificate {
    return f(certs)
}

And then NopSelector becomes:

func NopCertsSelector() CertsSelectorFunc {
    return func(certs []*x509.Certificate) []*x509.Certificate {
        return certs
    }
}

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done
I am still super new to Go, so you may find me adapting best practices of other languages. Thanks for teaching me the Go way of doing things :-)

import "crypto/x509"

// CertsSelectorFunc is a type of function that filters certificates.
type CertsSelectorFunc func([]*x509.Certificate) []*x509.Certificate
Copy link
Member

Choose a reason for hiding this comment

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

Oh, sorry! Didn't mean to get rid of the CertsSelector interface. Just that CertsSelectorFunc satisfies the CertsSelector interface. So folks can provide the implementation that they want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice one!!
Done!

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