[PATCH] Re: PKCS#11 support in OpenSSH 4.3p2

[Alon Bar-Lev]

On Sunday 24 September 2006 02:41, Andrew Bartlett wrote:
> > One reason is that I am not the author of this code.
> > Another reason is that my OpenSSH developers did not comment on
> > this patch, and since they did not, I don't know what to pack
> > yet.
>
> That's one approach.  Another is to clean things up really well,
> and target the changes well, so that the developer feels that they
> simply can't object to the patch :-).

From my experience, it does not go this way.
There are a lot of issues to discuss before merging.

> I use ssh-agent without a gui all the time.  Why can't ssh-add
> prompt for a pin just like it prompts for passphrases?

Because the ssh-agent should challenge ssh back for passphrase when 
card session expired, or card is removed/inserted.
Current protocol does not support his.
This is one issue to be discussed...

> > There is no reason to remove anything. The patch will work with
> > or without X.509 as expected.
>
> Yeah, I think i wrote that comment before I finished my patch
> hacking session.  My apologies.
>
> Still, the less you mention X509 in terms of the SSH end, and
> present this in terms of simply smartcards, the less red flags the
> OpenSSH developers need to consider.  Fight the X.509 battle
> another day :-)

If this patch is merged, then the X.509 support will be provided by 
the X.509 patch.

> > Not exactly.
> > Roumen Petrov explained to me that there is somekind of limit in
> > the negotiation stage. So the user should specify if he wishes
> > downgrade.
>
> What a pity.  This should be looked into very carefully, as it
> would drastically limit the usefulness of X.509 certs.

Current implementation is the most usable one. I had a long discussion 
regarding that. And the last version is offering a good alternative.

> But if (as on Fedora Core 6, and RHEL5 betas) pam_pkcs11 is
> functional, can we make use of it?  They seem to have it down to
> 'tick a box' for smartcard login...

pam_pkcs11 a bad implementation, please stop refering to it, unless 
you review its code, and find it acceptable.

The pkcs11-helper library is used in OpenVPN (merged), OpenSSH 
(patch), QCA (merged), GnuPG (external daemon), I also have 
xsupplicant and some more.

I've written the pkcs11-helper after I've found that there is no valid 
PKCS#11 usage among open-source projects.

pkcs11-helper works with many cards, includeing OpenSC, Aladdin, 
Athena, Siemens, openCryptoki, Rainbow, ARX, Datakey.

If you wish, pam_pkcs11 can use pkcs11-helper in order to provide a 
better service.

> > I think that all smartcard related code (opensc and javacard) be
> > considered to be removed after a standard PKCS#11 implementation
> > is added.
>
> That's a rather large step, and in any case, the old UI will need
> to be preserved.

As I said, the ssh-agent protocol needs also be revised.

> Looking at the ssh-agent code, your pkcs_11 mode shares no options
> in common with the other smartcard code.  If that code is to be
> replaced with yours, then users with scripts etc will break.  If
> the smartcard code is not replaced, manpages get bigger to list
> both, and users become confused 'which smartcard do I have?'.

Current smartcard support is invalid, because of this I written this 
new one.
It adds all certificates into the agent, it does not support removal 
and insert of cards, it does not support session timeout, multi 
providers and more.

> > True...
> > Well... I kind of hope that a cleaner exit will be applied in the
> > future into ssh-agent.
>
> Why?  ssh-agent, like many other programs, needs to deal gracefully
> with abnormal termination.  What happens if that _terminate isn't
> called? (Because the process was killed in a nasty way?).

Nothing happens.
But I like the signal code informs the main to perform a clean exit, 
and not exit from the signal handler.

> I'm saying that pkcs11_helper.c and basicly everything outside
> pkinit.c feel like they belong elsewhere.  It is just a gut feeling
> that 'surely the system should provide this'.

system? which system?
I can make the pkcs11-helper a standalone library... But I find it 
much easier to merge without external dependencies.

> Current Portable CVS.

Well...
You make it difficult for me to review this patch...
But I got the mainline comment.

> > But as I said... the PKCS#11 should be default on, the
> > modification of the pre-compiler constant will be decided when
> > merging occurs, Why did you add all these string.h, errno.h
> > #include directive? Did you have some problem? Nobody reported
> > such... yet.
>
> Yes, I needed these headers to compile on Fedora Core 5.

Strange...
I will modify next version.

Best Regards,
Alon Bar-Lev.