Conversation
provisioners.proto
Outdated
| message SSHPOPProvisioner {} | ||
|
|
||
| message SCEPDecrypter { | ||
| config.KMS kms = 1; |
There was a problem hiding this comment.
If possible, we should not include this.
There was a problem hiding this comment.
Have made some changes locally, and got a basic version working with just the decrypterKey configuration, so we should be able to do without.
It would require the decrypterKey to contain the full configuration for the KMS and to be passed in using the Options.URI in all cases and for all options. We may have to check/update some KMSs to have this logic, but we can do so when users need it, IMO.
provisioners.proto
Outdated
| bytes decrypterCertificate = 2; | ||
| string decrypterKey = 3; |
There was a problem hiding this comment.
Both should be the same type, but we have mixed values between the certificates and keys in the Configuration type, and the ones in the provisioners.
There was a problem hiding this comment.
To verify: decrypterKey should thus become bytes too? Then try base64 decoding to see if there's a PEM formatted key in there, and if so, use that as the (softkms) key? If base64 decoding fails, try interpreting it as a key URI?
There was a problem hiding this comment.
In my opinion, because the provisioners can be configured remotely, we should send store the key itself, a PEM, or the URI that represents it. For a PEM, bytes are "easier"; for the URI, a string is easier. We can consider having two fields if it makes this easier. We can use oneof or Any, but these are more difficult than just adding two incompatible fields that will be verified on the server side.
provisioners.proto
Outdated
| config.KMS kms = 1; | ||
| bytes decrypterCertificate = 2; | ||
| string decrypterKey = 3; | ||
| string decrypterKeyPassword = 4; |
There was a problem hiding this comment.
Bytes per Configuration.password.
There was a problem hiding this comment.
Do we need a decrypterKeyPasswordFile too? Or try interpreting decrypterKeyPassword as a path to a file? I think the latter could be confusing and unexpected behavior.
There was a problem hiding this comment.
For a file it should be a flag --decrypter-password-file
There was a problem hiding this comment.
For now I'll only support a password stored as bytes. If a password file is used (locally), the contents have to be read first and then stored in the provisioner.
This reverts the change to move `KMS` into its own file.
For SCEP we need a new webhook to notify a system that a certificate was issued successfully or failed. The `NOTIFYING` webhook type can be used for this, but is also somewhat generically named, so that it can be used for other use cases too.
Add `NOTIFYING` webhook type
Reorder existing properties and change key properties to have one for both a PEM as well as an URI.
…tep/linkedca into herman/scep-provisioner-decrypter
2 participants
MovedKMSto a separate file to be able to reuse it within the SCEP provisioner. From what I can tell, this should not affect existing serialized (and persisted) authority configurations with KMS objects; the generated protobuf code only has internal changes. Does that sound sensible?