Skip to content

Enable PKCS11 Support#331

Merged
manojampalam merged 2 commits into
PowerShell:latestw_allfrom
NoMoreFood:enable_pkcs11
May 21, 2019
Merged

Enable PKCS11 Support#331
manojampalam merged 2 commits into
PowerShell:latestw_allfrom
NoMoreFood:enable_pkcs11

Conversation

@NoMoreFood

@NoMoreFood NoMoreFood commented Jul 15, 2018

Copy link
Copy Markdown
  • Enable use of PKCS11 library files by adjusting master configuration file.
  • Modified dlsym() to return a void pointer instead of an int which is consistent with POSIX. The previous return type caused an issue with 32-bit builds with PKCS11 enabled.

@NoMoreFood NoMoreFood changed the title - Enabled Compiler PKCS#11 Support Enable PKCS#11 Support Jul 18, 2018
@cloudhan

Copy link
Copy Markdown

Any chance will this PR be merged? I need PKCS#11 support to ssh into the server which only support smartcard authentication.

@manojampalam

Copy link
Copy Markdown

@cloudhan can you enlighten me on how your scenario works? Do you have a PKCS provider on Windows? What did you mean by "server only support smartcard authentication" ?

@cloudhan

cloudhan commented Oct 2, 2018

Copy link
Copy Markdown

@manojampalam My company provide me with an USBKey (aka, the smartcard, similar to Yubikey) to login to the dev server. To my understanding, it basically is ssh public key auth. I can retrieve a public key from the USBKey and register it to the server. To login, I need the private key, but there is no way to retrieve the private key literal (by design). So, I either use the MS-CAPI or PKCS#11 for login authentication . However, OpenSSH ship with Windows 10 does not support. Currently I'm using the PuTTY-CAC suggested by OpenSC. But its font rendering is pretty awful.

If this pr can be merged, it can slightly improve my workflow in the future. (as well as a much border audience, as you can see from the short list of OpenSC)

@NoMoreFood

Copy link
Copy Markdown
Author

@manojampalam The way that PKCS support works in OpenSSH is that you pass in a vendor-provided DLL (or .SO file on Linux) that enables the capability to interface with the hardware token. The exported functions in the DLL must match the PKCS11 specification in order to be used by OpenSSH. PuTTY-CAC (of which I am the maintainer) is an add-on to PuTTY to provide this functionality by either using the same PKCS DLL or by using the CAPI subsystem on Windows (which uses the Windows minidriver for the token). Admittedly, I do not control the font rendering part of the code --- that's part of the upstream PuTTY code.

@NoMoreFood

Copy link
Copy Markdown
Author

@manojampalam Thoughts on moving forward with this?

@jcotton42

Copy link
Copy Markdown

This is also something I'd like to see

@NoMoreFood

Copy link
Copy Markdown
Author

@bingbing8 I adjusted my commit for simplicity by defining the PKCS11 compiler flag in the central configuration file and it looks like the appveyor tests are giving me "The specified module 'pester' was not loaded because no valid module file was found in any module directory." I doubt it's related to my change. Any idea what might be going on here?

@bingbing8

Copy link
Copy Markdown

@NoMoreFood , if you check the set up log, pester is not installed successfully. https://ci.appveyor.com/api/buildjobs/u0fl8r630g54pf00/artifacts/TestSetupLog.txt

@NoMoreFood

Copy link
Copy Markdown
Author

@bingbing8 Understood. Shouldn't that be something that the test setup does automatically or is there something additional I should be doing in my commits to install pester?

@bingbing8

bingbing8 commented Nov 29, 2018

Copy link
Copy Markdown

@NoMoreFood , the tests download and install it. The failure could due to temporary chocolatey server issue, network issue. or the particular version does not exist any more. Previously we install pester 3.4.6 due to a pester bug at that time. Please refer https://github.com/PowerShell/openssh-portable/blob/latestw_all/contrib/win32/openssh/OpenSSHTestHelper.psm1#L366. looks like the version still exists https://chocolatey.org/packages/pester/3.4.6.
you can either push a changes, or close/reopen the PR to trigger another test run if it is temporary chocolatey server or network issue.

@bingbing8 bingbing8 closed this Nov 29, 2018
@bingbing8 bingbing8 reopened this Nov 29, 2018
@bingbing8

Copy link
Copy Markdown

@NoMoreFood , I trigger the build again by close/reopen the PR, all checks passed.

@NoMoreFood NoMoreFood changed the title Enable PKCS#11 Support Enable PKCS11 Support Nov 30, 2018
@yan4321 yan4321 mentioned this pull request Dec 18, 2018
@mattmccull

Copy link
Copy Markdown

@manojampalam Do we have a estimate as to when this will be merged? This is something I'm interested in.

@spencercw

Copy link
Copy Markdown

This PR exposes an existing issue with the dlerror() implementation which causes a crash if the PKCS#11 DLL cannot be loaded. See #379 for the fix.

Comment thread contrib/win32/win32compat/inc/dlfcn.h Outdated
int dlclose(HMODULE handle);

FARPROC dlsym(HMODULE handle, const char *symbol); No newline at end of file
void * __stdcall dlsym(HMODULE handle, const char *symbol); No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does this need to be stdcall? It would probably make more sense to leave this with the default calling convention like the other functions in this file.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You're right. We do not. Commit adjusted.

@NoMoreFood NoMoreFood force-pushed the enable_pkcs11 branch 2 times, most recently from e914bd4 to 330acad Compare March 6, 2019 04:41
- Enable use of PKCS11 library files by adjusting central configuration file.
- Modified dlsym() to return a void pointer instead of an int which is consistent with POSIX. The previous return type caused an issue with 32-bit builds with PKCS11 enabled.
@jmfrank63

Copy link
Copy Markdown

Is this pull request going to be accepted? Would be a great addition.

@keliansb

Copy link
Copy Markdown

Please accept this PR, I'd like this to be implemented too.

@manojampalam manojampalam merged commit 44ba548 into PowerShell:latestw_all May 21, 2019
@NoMoreFood NoMoreFood deleted the enable_pkcs11 branch May 22, 2019 11:14
hazelduvall pushed a commit to hazelduvall/openssh-portable that referenced this pull request May 6, 2026
- Enable use of PKCS11 library files by adjusting central configuration file.
- Modified dlsym() to return a void pointer instead of an int which is consistent with POSIX. The previous return type caused an issue with 32-bit builds with PKCS11 enabled.
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.

9 participants