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

Pluggable CSR signing framework #113

Merged
merged 3 commits into from
Feb 18, 2021

Conversation

jessepeterson
Copy link
Member

@jessepeterson jessepeterson commented Sep 14, 2020

tl;dr Changes internal API for how signing of certificates works in the scep server.

This PR changes the core organization of the SCEP server service in support of #112. The service itself (in server/service.go) is much simplified due to removing all CA functionality. In lieu of CA implementation details being in the server service we opted for a way to modularlize certificate signing (and any other duties that need to inspect and take action on the CSR). Thus the scepserver.CSRSigner interface:

// CSRSigner is a handler for CSR signing by the CA/RA
//
// SignCSR should take the CSR in the CSRReqMessage and return a
// Certificate signed by the CA.
type CSRSigner interface {
	SignCSR(*scep.CSRReqMessage) (*x509.Certificate, error)
}

Due to the interface and associated adapter functions this also makes it possible to chain together these CSRSigners to modularize most of the functionality we need — not so dissimilar from how http.HandlerFunc works. For example because scep.CSRReqMessage contains the parsed and raw CSR as well as the challenge password, the SCEP challenge password checking can just be a middleware over the actual CA implementation.

The other (minor, compared to the above) change is a separation of "service" certificates from the CA certificates. While for most users these are the same certs and keys (i.e. the SCEP protocol exchanges just use the same keypair as the CA) some users may want to use the SCEP service with another CA that has its own keypair, or as an RA (Registration Authority — a "proxy" of sorts). This change allows that.

@jessepeterson
Copy link
Member Author

jessepeterson commented Sep 14, 2020

Some specific notes and concerns that I have:

  • The structure of placing the middleware into the sub-packages leads to dependency recursion. Currently it only pops up for one of the tests, the actual server runs fine. Perhaps moving the CSRSignerFunc defintions and middleware into its own middleware package? Otherwise maybe couple it to the scepserver package again by including the middleware layers for all the sub-packages in there (sort of how the previous version did it)
    • This was fixed by making the test its own _test package. I still think having unit tests in the packages and then maybe having a dedicated test package for larger, more encompassing integration tests would be cleaner.
  • Now With a modular framework for implementing sign-time logic could perhaps eliminate the CSR verifier interface (but keeping the feature) and just make it another CSRSigner middleware
    • I think for keeping this as is is fine.
  • Not 100% sure about the name CSRSignerFunc and pals.
  • For some reason testing with -race (as it is in the Makefile now) causes all kinds of trouble with BoltDB I think. I haven't had time to look into it. But this is the case without these changes, too.
  • Because this is such a core change it'll need lots and lots of testing. I haven't even looked at MicroMDM integration yet.

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