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

Adding WrapKey to p11/session #95

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

dkiser
Copy link

@dkiser dkiser commented Jan 27, 2019

Initial stab at #94

@emmanuel
Copy link
Contributor

emmanuel commented Feb 15, 2019

This is a drive-by, so feel free to disregard.

Wouldn't this be a nicer API if implemented in terms of p11.PrivateKey and p11.PublicKey? I.e.,

func (priv PrivateKey) Wrap(mechanism pkcs11.Mechanism, wrapper PublicKey) {
	s := priv.session
	s.Lock()
	defer s.Unlock()
	return s.ctx.WrapKey(s.handle,
		[]*pkcs11.Mechanism{&mechanism},
		wrapper.objectHandle,
		priv.objectHandle)
}

Admittedly, this only addresses the use-case of wrapping a private key using a public key. I was going to open a separate PR to add a SecretKey type to complement to existing PrivateKey and PublicKey in p11 (i.e., an object with class pkcs11.CKA_SECRET_KEY). That SecretKey could have its own implementation of WrapKey Wrap(), but also accepting a PublicKey as the arg.

That idea also only addresses the use-case of wrapping with a PublicKey, which may be too limited, but I personally would prefer a more 'narrow' API in the p11 package than handling raw pkcs11.ObjectHandles (or p11.Objects), which could be easily mis-used.

What do you think?

@miekg
Copy link
Owner

miekg commented Feb 21, 2019

I agree with @emmanuel

@dkiser
Copy link
Author

dkiser commented Feb 21, 2019

Sounds good!

@@ -40,6 +40,8 @@ type Session interface {
GenerateKeyPair(request GenerateKeyPairRequest) (*KeyPair, error)
// GenerateRandom returns random bytes generated by the token.
GenerateRandom(length int) ([]byte, error)
// Wrap key returns ciphertext bytes from wrapping a key with another key
WrapKey(request WrapKeyRequest) ([]byte, error)
Copy link
Owner

Choose a reason for hiding this comment

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

This session interface starts looking like a dumping ground for methods, can you find a better way than adding another method here?

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.

3 participants