Add CreateDecrypter (RSA OAEP) support to Google Cloud KMS

[hslatman]

No description provided.

[maraino]

Let's not support any type of DecrypterOpts

[maraino]

Let's add a typed switch here, similar to rsa.Decrypt(). The difference would be that if opts == nil the default algorithm would be RSA-OAEP instead of PKCS #1 v1.5. We should explicitly say that in the comment.

[hslatman]

I tried that, but the code doesn't look cleaner then. The reason for that is that there can be typed nils, but also untyped nils. Both have to be handled to be able to verify the input.

The goal of the code is to show a more descriptive error message in case decryption fails if a user tries to use a parameter that we know will for sure result in an error. For example, when a label is used, the error is very generic: Error: error decrypting input: cloudKMS AsymmetricDecrypt failed: rpc error: code = InvalidArgument desc = Decryption failed.. Same is true if someone tries to use this with PKCS1 options. So we're not actually passing the options to GCP, because there's no way to do so. We're merely checking for them, and return an error for things that GCP doesn't support.

We could in fact improve the check even more by capturing the key type during preload, storing that, and verifying that the request decryption parameters match the keys properties.

[maraino]

You can do something like the following:

func validateOAEPOptions(o *rsa.OAEPOptions) error {
	if len(o.Label) > 0 {
		return errors.New("cloudKMS does not support RSA-OAEP label")
	}
	switch o.Hash {
	case crypto.Hash(0), crypto.SHA1, crypto.SHA256, crypto.SHA512:
		return nil
	default:
		return fmt.Errorf("cloudKMS does not support hash algorithm %q with RSA-OAEP", ropts.Hash)
	}
}

func (d *Decrypter) Decrypt(rand io.Reader, ciphertext []byte, opts crypto.DecrypterOpts) ([]byte, error) {
	if opts == nil {
		opts = &rsa.OAEPOptions{}
	}

	switch o := opts.(type) {
	case *rsa.OAEPOptions:
		if err := validateOAEPOptions(o); err != nil {
			return nil, err
		}
	case *rsa.PKCS1v15DecryptOptions:
		return nil, errors.New("cloudKMS does not support PKCS #1 v1.5 decryption")
	default:
		return nil, errors.New("invalid options for Decrypt")
	}
	// ...
}

You can also do nothing in the case *rsa.OAEPOptions and do the validation after the switch.

The algorithm really depends on the key itself, so it makes sense to allow crypto.Hash(0) too.

[hslatman]

Changed it in c726662. Note the additional o == nil case. It's required in a fairly specific usage pattern.

[maraino]

you can do this in validateOAEPOptions.

[maraino]

lgtm