Certificate issuance #16
Conversation
This commit adds the initial CA skeleton for doing order issuance. Pebble generates a root & intermediate keypair/certificate at startup. The intermediate is used for certificate signing purposes and the root issues the intermediate and is otherwise just there to mimick production. In the future we should introduce additional intermediates & chain options to use the full specification. Couple other changes: * Fixed the embedded challenges in authorizations to use a pointer so that the embedded contents are updated when the challenge is completed. * Replaced a few WFE `Printf`'s with log statements. * Updated the VA to log whether an HTTP validation was a success or a failure. * Added a pointer from Authorizations to the Order they belong to * Added enforcement of Order expiry for authz updates. * Moved the MemoryStore out of the `wfe` package. This was primarily to let the CA store the root & intermediate certificates in the DB. This allows using the `/certZ/` endpoint to retrieve the root & intermediate.
ca/ca.go
Outdated
| ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, | ||
| BasicConstraintsValid: true, | ||
| IsCA: true, | ||
| MaxPathLenZero: true, |
There was a problem hiding this comment.
Let's remove this. It should be false for the root, and could be true for the intermediate, but doesn't need to be. So defaulting to false in both cases is fine,
ca/ca.go
Outdated
| // Make an intermediate private key | ||
| ik, err := makeKey() | ||
| if err != nil { | ||
| panic(fmt.Sprintf( |
There was a problem hiding this comment.
Better to return an error and have the caller panic, since each of these panics.
core/types.go
Outdated
| } | ||
|
|
||
| // Return the chain, leaf cert first | ||
| return bytes.Join(chain, []byte{}) |
There was a problem hiding this comment.
The []byte{} here can be nil.
core/types.go
Outdated
| for { | ||
| // if the issuer is nil, or the issuer's issuer is nil then it isn't a leaf | ||
| // or an intermediate. Don't put it in the chain | ||
| if issuer == nil || issuer.Issuer == nil { |
There was a problem hiding this comment.
Why do we need to check both? Wouldn't issuer == nil be sufficient?
There was a problem hiding this comment.
Oops! Spoke too soon. I remember why I did this: without checking issuer.Issuer == nil then the root issuer is included in the chain as well as the leaf & intermediate. Removing the check results in a chain of three certs when it should be two.
db/memorystore.go
Outdated
| func (m *MemoryStore) GetCertificateByID(id string) *core.Certificate { | ||
| m.RLock() | ||
| defer m.RUnlock() | ||
| if cert, present := m.certificatesByID[id]; present { |
There was a problem hiding this comment.
This function can just be turned into return m.certificatesByID[id], since that will return nil when not present.
There was a problem hiding this comment.
Good catch - this applies equally to the other memory store GetX functions. I'll simplify all of them with this round of review feedback.
| @@ -27,6 +32,7 @@ type Authorization struct { | |||
| ID string | |||
| URL string | |||
| ExpiresDate time.Time | |||
| Order *Order | |||
There was a problem hiding this comment.
I think you meant this to be a list of orders? But AFAICT this isn't used in this PR so should wait.
There was a problem hiding this comment.
It's used by the WFE's updateChallenge function where a POST to an Authz checks the Order the authz is associated with to determine if it is expired. If it is then POSTing the Authz isn't allowed.
Should it be a []*Order field? Right now pebble creates authorizations uniquely per order. You can't have an authz shared across orders (at least right now).
There was a problem hiding this comment.
I was confused by some diff collapsing and thought this was a field on Registration. This is fine for now. Thanks!
This PR adds the initial CA skeleton for doing order issuance.
Pebble generates a root & intermediate keypair/certificate at startup. The intermediate is used for certificate signing purposes and the root issues the intermediate and is otherwise just there to mimick production. In the future we should introduce additional intermediates & chain options to use the full specification.
Couple other changes:
Printf's with log statements.wfepackage. This was primarily to let the CA store the root & intermediate certificates in the DB. This allows using the/certZ/endpoint to retrieve the root & intermediate.