search

ThalesGroup / crypto11

Implement crypto.Signer and crypto.Decrypter for HSM-protected keys via PKCS#11
MIT License
219 stars 86 forks source link

panic when MaxSessions is 1 #60

Closed cbroglie closed 5 years ago

cbroglie commented 5 years ago

If MaxSessions is set to 1, the following panic occurs:

panic: invalid/out of range capacity

goroutine 1 [running]:
github.com/thales-e-security/pool.NewResourcePool(0xc00629c030, 0x0, 0x0, 0x0, 0x0, 0x0)
    /Users/cbroglie/.../vendor/github.com/thales-e-security/pool/resource_pool.go:87 +0x481
github.com/ThalesIgnite/crypto11.Configure(0xc0000caba0, 0x0, 0x0, 0x0)
    /Users/cbroglie/.../vendor/github.com/ThalesIgnite/crypto11/crypto11.go:327 +0x4bd

This is due to:

// We will use one session to keep state alive, so the pool gets maxSessions - 1
instance.pool = pool.NewResourcePool(instance.resourcePoolFactoryFunc, maxSessions-1, maxSessions-1, 0, 0)
dmjones commented 5 years ago

Thanks for flagging this.

A straight-forward fix for this is to document that the actual number of sessions opened is one greater than MaxSessions. Then we can drop the -1 from the offending code line.

@cbroglie Do you have any use cases where only 1 session is available? If so, we may need to more thoroughly redesign this part of the library to handle that case.

cbroglie commented 5 years ago

No, we haven't come across any HSMs which support exactly 1 session. Simply documenting the behavior would be fine for us.

dmjones commented 5 years ago

Documentation updated. The library will also return an error rather than panicking now. Release 1.2.1 contains this fix.