-
Notifications
You must be signed in to change notification settings - Fork 88
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
Improve session handling #3
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,9 +24,10 @@ package crypto11 | |
import ( | ||
"crypto" | ||
"crypto/dsa" | ||
pkcs11 "github.com/miekg/pkcs11" | ||
"io" | ||
"math/big" | ||
|
||
pkcs11 "github.com/miekg/pkcs11" | ||
) | ||
|
||
// PKCS11PrivateKeyDSA contains a reference to a loaded PKCS#11 DSA private key object. | ||
|
@@ -35,14 +36,14 @@ type PKCS11PrivateKeyDSA struct { | |
} | ||
|
||
// Export the public key corresponding to a private DSA key. | ||
func exportDSAPublicKey(session pkcs11.SessionHandle, pubHandle pkcs11.ObjectHandle) (crypto.PublicKey, error) { | ||
func exportDSAPublicKey(session *PKCS11Session, pubHandle pkcs11.ObjectHandle) (crypto.PublicKey, error) { | ||
template := []*pkcs11.Attribute{ | ||
pkcs11.NewAttribute(pkcs11.CKA_PRIME, nil), | ||
pkcs11.NewAttribute(pkcs11.CKA_SUBPRIME, nil), | ||
pkcs11.NewAttribute(pkcs11.CKA_BASE, nil), | ||
pkcs11.NewAttribute(pkcs11.CKA_VALUE, nil), | ||
} | ||
exported, err := libHandle.GetAttributeValue(session, pubHandle, template) | ||
exported, err := libHandle.GetAttributeValue(session.Handle, pubHandle, template) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
@@ -71,10 +72,10 @@ func GenerateDSAKeyPair(params *dsa.Parameters) (*PKCS11PrivateKeyDSA, error) { | |
func GenerateDSAKeyPairOnSlot(slot uint, id []byte, label []byte, params *dsa.Parameters) (*PKCS11PrivateKeyDSA, error) { | ||
var k *PKCS11PrivateKeyDSA | ||
var err error | ||
if err = setupSessions(slot, 0); err != nil { | ||
if err = setupSessions(slot); err != nil { | ||
return nil, err | ||
} | ||
err = withSession(slot, func(session pkcs11.SessionHandle) error { | ||
err = withSession(slot, func(session *PKCS11Session) error { | ||
k, err = GenerateDSAKeyPairOnSession(session, slot, id, label, params) | ||
return err | ||
}) | ||
|
@@ -84,7 +85,7 @@ func GenerateDSAKeyPairOnSlot(slot uint, id []byte, label []byte, params *dsa.Pa | |
// GenerateDSAKeyPairOnSession creates a DSA private key using a specified session | ||
// | ||
// Either or both label and/or id can be nil, in which case random values will be generated. | ||
func GenerateDSAKeyPairOnSession(session pkcs11.SessionHandle, slot uint, id []byte, label []byte, params *dsa.Parameters) (*PKCS11PrivateKeyDSA, error) { | ||
func GenerateDSAKeyPairOnSession(session *PKCS11Session, slot uint, id []byte, label []byte, params *dsa.Parameters) (*PKCS11PrivateKeyDSA, error) { | ||
var err error | ||
var pub crypto.PublicKey | ||
|
||
|
@@ -124,7 +125,7 @@ func GenerateDSAKeyPairOnSession(session pkcs11.SessionHandle, slot uint, id []b | |
pkcs11.NewAttribute(pkcs11.CKA_ID, id), | ||
} | ||
mech := []*pkcs11.Mechanism{pkcs11.NewMechanism(pkcs11.CKM_DSA_KEY_PAIR_GEN, nil)} | ||
pubHandle, privHandle, err := libHandle.GenerateKeyPair(session, | ||
pubHandle, privHandle, err := libHandle.GenerateKeyPair(session.Handle, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a logic behind passing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of the functions in crypto11 should operate on |
||
mech, | ||
publicKeyTemplate, | ||
privateKeyTemplate) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,9 +28,10 @@ import ( | |
"crypto/elliptic" | ||
"encoding/asn1" | ||
"errors" | ||
pkcs11 "github.com/miekg/pkcs11" | ||
"io" | ||
"math/big" | ||
|
||
pkcs11 "github.com/miekg/pkcs11" | ||
) | ||
|
||
// ErrUnsupportedEllipticCurve is returned when an elliptic curve | ||
|
@@ -196,15 +197,15 @@ func unmarshalEcPoint(b []byte, c elliptic.Curve) (x *big.Int, y *big.Int, err e | |
} | ||
|
||
// Export the public key corresponding to a private ECDSA key. | ||
func exportECDSAPublicKey(session pkcs11.SessionHandle, pubHandle pkcs11.ObjectHandle) (crypto.PublicKey, error) { | ||
func exportECDSAPublicKey(session *PKCS11Session, pubHandle pkcs11.ObjectHandle) (crypto.PublicKey, error) { | ||
var err error | ||
var attributes []*pkcs11.Attribute | ||
var pub ecdsa.PublicKey | ||
template := []*pkcs11.Attribute{ | ||
pkcs11.NewAttribute(pkcs11.CKA_ECDSA_PARAMS, nil), | ||
pkcs11.NewAttribute(pkcs11.CKA_EC_POINT, nil), | ||
} | ||
if attributes, err = libHandle.GetAttributeValue(session, pubHandle, template); err != nil { | ||
if attributes, err = libHandle.GetAttributeValue(session.Handle, pubHandle, template); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding to the previous comment, I think having two different "handles" (one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's the nature of pkcs11 - there are many handles. I'm just maintaining the existing conventions, see // PKCS11Object contains a reference to a loaded PKCS#11 object.
type PKCS11Object struct {
// The PKCS#11 object handle.
Handle pkcs11.ObjectHandle
// The PKCS#11 slot number.
//
// This is used internally to find a session handle that can
// access this object.
Slot uint
} |
||
return nil, err | ||
} | ||
if pub.Curve, err = unmarshalEcParams(attributes[0].Value); err != nil { | ||
|
@@ -235,10 +236,10 @@ func GenerateECDSAKeyPair(c elliptic.Curve) (*PKCS11PrivateKeyECDSA, error) { | |
func GenerateECDSAKeyPairOnSlot(slot uint, id []byte, label []byte, c elliptic.Curve) (*PKCS11PrivateKeyECDSA, error) { | ||
var k *PKCS11PrivateKeyECDSA | ||
var err error | ||
if err = setupSessions(slot, 0); err != nil { | ||
if err = setupSessions(slot); err != nil { | ||
return nil, err | ||
} | ||
err = withSession(slot, func(session pkcs11.SessionHandle) error { | ||
err = withSession(slot, func(session *PKCS11Session) error { | ||
k, err = GenerateECDSAKeyPairOnSession(session, slot, id, label, c) | ||
return err | ||
}) | ||
|
@@ -251,7 +252,7 @@ func GenerateECDSAKeyPairOnSlot(slot uint, id []byte, label []byte, c elliptic.C | |
// | ||
// Only a limited set of named elliptic curves are supported. The | ||
// underlying PKCS#11 implementation may impose further restrictions. | ||
func GenerateECDSAKeyPairOnSession(session pkcs11.SessionHandle, slot uint, id []byte, label []byte, c elliptic.Curve) (*PKCS11PrivateKeyECDSA, error) { | ||
func GenerateECDSAKeyPairOnSession(session *PKCS11Session, slot uint, id []byte, label []byte, c elliptic.Curve) (*PKCS11PrivateKeyECDSA, error) { | ||
var err error | ||
var parameters []byte | ||
var pub crypto.PublicKey | ||
|
@@ -290,7 +291,7 @@ func GenerateECDSAKeyPairOnSession(session pkcs11.SessionHandle, slot uint, id [ | |
pkcs11.NewAttribute(pkcs11.CKA_ID, id), | ||
} | ||
mech := []*pkcs11.Mechanism{pkcs11.NewMechanism(pkcs11.CKM_ECDSA_KEY_PAIR_GEN, nil)} | ||
pubHandle, privHandle, err := libHandle.GenerateKeyPair(session, | ||
pubHandle, privHandle, err := libHandle.GenerateKeyPair(session.Handle, | ||
mech, | ||
publicKeyTemplate, | ||
privateKeyTemplate) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use
log.Fatalf
instead sincelog
is already imported.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm merely returning an error here, not trying to make the application exit (which is what
log.Fatal
will do). Handling the error is up to the calling application.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, but if you look above, this same function is logging errors in addition to returning errors. I think it's a good idea given that this function handles hardware. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current behavior (i.e.without this PR) is a bit inconsistent; I've created #4 as a placeholder for doing a cleanup at some later date.