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

piv: PIN caching doesn't seem to work #42

Closed
FiloSottile opened this issue Apr 26, 2020 · 8 comments · Fixed by #44
Closed

piv: PIN caching doesn't seem to work #42

FiloSottile opened this issue Apr 26, 2020 · 8 comments · Fixed by #44

Comments

@FiloSottile
Copy link

Hey, thank you so much for landing these changes so fast, without me even asking!

I tried v1.4.0 with yubikey-agent, and it doesn't quite work =(

  1. It requires a touch before asking for the PIN, presumably because it issues a sign request first, then when it fails it prompts for the PIN.
  2. This happens at every sign operation, so PIN caching does not seem to actually work, even if I'm pretty sure I set this up with a "once" PIN policy.

No idea what's going on with (2), but for (1) I think you can learn about the policy from the attestation certificate, and behave accordingly.

@ericchiang
Copy link
Collaborator

Uggg... yeah (1) sounds right. Let me think about that. For (2) your YubiKey would actually have to be rejecting the request and require authentication.

Thanks for the report!

@FiloSottile
Copy link
Author

I double checked the code that refreshes the *YubiKey is not firing, for (2), and it looks like it's not, so no idea what might be going on. Happy to provide traces or anything that helps.

This was the setup, and it's a YubiKey 4 with PIV version 4.3.6.

yubico-piv-tool -k -s 9a -a generate -o public.pem --pin-policy=once --touch-policy=always
yubico-piv-tool -a verify-pin -a selfsign-certificate -s 9a -S "/CN=SSH key/" -i public.pem -o cert.pem
yubico-piv-tool -a import-certificate -s 9a -i cert.pem -k

@ericchiang
Copy link
Collaborator

Using management key 010203040506070801020304050607080102030405060708 and PIN 123456, the following program worked for me:

package main

import (
        "crypto"
        "crypto/rand"
        "crypto/sha256"
        "log"

        "github.com/go-piv/piv-go/piv"
)

func main() {
        cards, err := piv.Cards()
        if err != nil {
                log.Fatalf("cards: %v", err)
        }
        if len(cards) != 1 {
                log.Fatalf("expected on card got %d", len(cards))
        }
        yk, err := piv.Open(cards[0])
        if err != nil {
                log.Fatalf("open card: %v", err)
        }
        a := piv.KeyAuth{
                PINPrompt: func() (string, error) {
                        log.Println("pin prompt called")
                        return piv.DefaultPIN, nil
                },
        }
        cert, err := yk.Attest(piv.SlotAuthentication)
        if err != nil {
                log.Fatalf("getting attestation cert for slot: %v", err)
        }
        priv, err := yk.PrivateKey(piv.SlotAuthentication, cert.PublicKey, a)
        if err != nil {
                log.Fatalf("private key: %v", err)
        }
        s, ok := priv.(crypto.Signer)
        if !ok {
                log.Fatalf("private key didn't implement crypto.Signer")
        }
        data := sha256.Sum256([]byte("hello"))
        if _, err := s.Sign(rand.Reader, data[:], crypto.SHA256); err != nil {
                log.Fatalf("signing: %v", err)
        }
}
% yubico-piv-tool -k -s 9a -a generate -o public.pem --pin-policy=once --touch-policy=always
Enter management key: 
Successfully generated a new private key.
% yubico-piv-tool -a verify-pin -a selfsign-certificate -s 9a -S "/CN=SSH key/" -i public.pem -o cert.pem
Enter PIN: 
Successfully verified PIN.
Successfully generated a new self signed certificate.
% yubico-piv-tool -a import-certificate -s 9a -i cert.pem -k
Enter management key: 
Successfully imported a new certificate.
% go run hi.go
2020/04/25 19:07:02 pin prompt called

I do have to touch the key twice, but the signing works fine.

With regards to the overall PIN prompt, I'm lookin at https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-73-4.pdf to see if there's a way to determine when a PIN is needed or not.

Another idea is to let the caller keep the state. E.g. let you provide something like the following to KeyAuth:

type PINPrompter interface {
    // PIN returns a PIN for a signing operation. This should be cached by the prompter
    // for repeated calls.
    PIN() (string, error)
    // PINInvalid is called by the signer when the returned PIN is deemed invalid. This likely
    // means the prompter should re-prompt for a PIN.
    PINInvalid() error
}

Or you could write the PIN to disk with the threat model that malicious code still can't get a signing operation without tricking you into touching your security key.

@FiloSottile
Copy link
Author

I do have to touch the key twice, but the signing works fine.

Indeed, even adding a second PrivateKey and Sign call only invokes PINPrompt once. I'll try to figure out what's going on on my side, ignore this part of the report for now.

Or you could write the PIN to disk with the threat model that malicious code still can't get a signing operation without tricking you into touching your security key.

I'd like to avoid that, part of why I am building yubikey-agent is that pivy-agent disregards the PIN policy and just keeps the PIN in memory.

I think you can tell the policy from the certificate. If that's the case, you can keep per-slot state about whether the PIN has been provided or not. The annoying part is that you now need a round of Attest, but the good news is that since you are holding a lock on the YubiKey, nothing external can change it from under your feet. Since most applications will call Attest before Sign, you can probably get away with caching Attest results, and calling it from Sign.

(Empirically, PKCS#11 somehow manages, I think, so we could also look into how that works.)

@ericchiang
Copy link
Collaborator

Mind giving #44 a try?

@FiloSottile
Copy link
Author

v1.4.1-0.20200426030018-8f5f73161d1d (from #44) fixes (1) 👍✨ Thank you!

Now trying to make a reproducer for (2), I'll open a new issue if it turns out to be an upstream issue.

@FiloSottile
Copy link
Author

Figured it out: calling Serial on my older YubiKey was switching application, which presumably locks the session. I have it all working as intended now! Might be worth documenting it in the Serial docs.

I also tried Retries for a health check, but that fails when the session is unlocked. Ended up using AttestationCertificate, but maybe it's worth adding a Ping method? Could even return the PIN lock status.

Finally, I tested simultaneous use of other applets. FIDO2 works in parallel without even dropping the PIN! gpg-agent though can't see the key at all while a PIV session is running, maybe because they use the same macOS daemon =( I'll switch to age+PIV soon anyway, but that will be even worse, because it's even less likely the two PIV sessions can be made to work at the same time. Any ideas? I'd be ok with the PIN getting dropped when another application uses the PIV (or PGP) applets, so a way of detecting the other one would be enough. At the moment the second Open just hangs until the other program exits (or calls Close I presume).

BTW, it would be nice for Open and other operations that might block to be cancellable, somehow, or at least to have a timeout.

@ericchiang
Copy link
Collaborator

Opened #45, #46, and #47

To get PIN caching to work, I had to have the YubiKey struct hold onto a PS/SC transaction #39, so theoretically no other process should be able to access the PIV applet until you call (*YubiKey).Close(). Will test some more later to see if there's a way around this.

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 a pull request may close this issue.

2 participants