openssh unix dev - Mar 2017 - [PATCH] Enable specific ioctl calls for ICA crypto card (s390)

On 03-03-2017 09:54, Petr Cerny wrote:
> Damien Miller wrote:
>> On Tue, 28 Feb 2017, Eduardo Barretto wrote:
>>
>>> On 13-02-2017 13:23, Eduardo Barretto wrote:
>>> > This patch enables specific ioctl calls for ICA crypto card on
s390
>>> > platform. Without this patch, users using the IBMCA engine are
not
>>> able
>>> > to perform ssh login as the filter blocks the communication
with the
>>> > crypto card.
>>> >
>>> > Signed-off-by: Harald Freudenberger <freude at
linux.vnet.ibm.com>
>>> > Signed-off-by: Eduardo Barretto <ebarretto at
linux.vnet.ibm.com>
>>> > ---
>>> >  sandbox-seccomp-filter.c | 24 +++++++++++++++++++++---
>>> >  1 file changed, 21 insertions(+), 3 deletions(-)
>>> >
>>> > diff --git a/sandbox-seccomp-filter.c
b/sandbox-seccomp-filter.c
>>> > index 2e1ed2c..264e146 100644
>>> > --- a/sandbox-seccomp-filter.c
>>> > +++ b/sandbox-seccomp-filter.c
>>
>> [snip]
>>
>>> Hi there,
>>>
>>> Do you have any feedback on this patch?
>>
>> It's hard to evaluate it without reference to some public
documentation
>> for the crypto card and the syscalls needed to use it. Is it a standard
>> part of s390 machines or an option?
> 
> Hi,
> 
> let me step in for Eduardo so that the thread doesn't hibernate.
> 
> As far as I know it is an optional piece of hardware, that may or may
> not be present in the system. OpenSSL is typically configured to load
> the libica engine (alongside any other possibly available ones).
> 
> As for the syscalls, would
> https://sourceforge.net/p/opencryptoki/libica/ci/master/tree/src/ica_api.c
> and
>
http://lxr.free-electrons.com/source/arch/s390/include/uapi/asm/zcrypt.h#L259
> be enough?
> 
>> Does it provide substantial benefit for the crypto used in the pre-auth
>> stage of the protocol? (private key operations and DH/ECDH key
agreement)
> 
> It should speed up some these:
> https://sourceforge.net/p/opencryptoki/libica/ci/master/tree/doc/icainfo.1
Just adding some more information on Petr answer (Thanks Petr for
stepping in!):

The ioctls for the s390 crypto card support are documented. The crypto
device driver is part of the linux kernel and thus open source. It can be
found in the kernel in drivers/s390/crypto. The ioctl stuff required to
interact with the crypto device driver is as usual provided in the
kernel header file arch/s390/include/uapi/asm/zcrypt.h
In particular the defines for the ioctl magics intended to not
get filtered can be found there.

As Petr mentioned, the relevant user space shared library which provides
a comfortable api for applications is libica. Libica is open
source and currently hosted on sourceforge as part of the
opencryptoki project (URLs above).

At the moment the zcrypt device driver provides access
to crypto cards which are able to accelerate RSA functions.
We are currently working on acceleration for ECC functions
and planing to release this in the near future. So one
benefit is performance, another is relief of CPU by
offloading crypto operations to this dedicated hardware.

The key exchange may use DH, ECCDH. The authentication
may use RSA or ECC. All the kind of stuff where the crypto
hardware may assist.

Thanks,
Eduardo
> 
> Thanks
> Kind regards
>     Petr

On Fri, 3 Mar 2017, Eduardo Barretto wrote:
> Just adding some more information on Petr answer (Thanks Petr for
> stepping in!):
> 
> The ioctls for the s390 crypto card support are documented. The crypto
> device driver is part of the linux kernel and thus open source. It can be
> found in the kernel in drivers/s390/crypto. The ioctl stuff required to
> interact with the crypto device driver is as usual provided in the
> kernel header file arch/s390/include/uapi/asm/zcrypt.h
> In particular the defines for the ioctl magics intended to not
> get filtered can be found there.
ok, with the fixes for the seccomp-bpf sandbox that I just committed
the diff reduces to.

IMO this is scoped narrowly enough to go in.

-d

diff --git a/sandbox-seccomp-filter.c b/sandbox-seccomp-filter.c
index af5525ab..6ceee33f 100644
--- a/sandbox-seccomp-filter.c
+++ b/sandbox-seccomp-filter.c
@@ -223,6 +223,12 @@ static const struct sock_filter preauth_insns[] = {
 #ifdef __NR_socketcall
 	SC_ALLOW_ARG(socketcall, 0, SYS_SHUTDOWN),
 #endif
+#if defined(__NR_ioctl) && defined(__s390__)
+	/* Allow ioctls for ICA crypto card on s390 */
+	SC_ALLOW_ARG(ioctl, 1, Z90STAT_STATUS_MASK),
+	SC_ALLOW_ARG(ioctl, 1, ICARSAMODEXPO),
+	SC_ALLOW_ARG(ioctl, 1, ICARSACRT),
+#endif /* defined(__NR_ioctl) && defined(__s390__) */
 
 	/* Default deny */
 	BPF_STMT(BPF_RET+BPF_K, SECCOMP_FILTER_FAIL),

I've committed this diff. Please test and confirm that it works ok.
(If not, then I've botched the macro fixes in the previous commit)

Thanks,
Damien Miller

On Tue, 14 Mar 2017, Damien Miller wrote:
> ok, with the fixes for the seccomp-bpf sandbox that I just committed
> the diff reduces to.
> 
> IMO this is scoped narrowly enough to go in.
> 
> -d
> 
> diff --git a/sandbox-seccomp-filter.c b/sandbox-seccomp-filter.c
> index af5525ab..6ceee33f 100644
> --- a/sandbox-seccomp-filter.c
> +++ b/sandbox-seccomp-filter.c
> @@ -223,6 +223,12 @@ static const struct sock_filter preauth_insns[] = {
>  #ifdef __NR_socketcall
>  	SC_ALLOW_ARG(socketcall, 0, SYS_SHUTDOWN),
>  #endif
> +#if defined(__NR_ioctl) && defined(__s390__)
> +	/* Allow ioctls for ICA crypto card on s390 */
> +	SC_ALLOW_ARG(ioctl, 1, Z90STAT_STATUS_MASK),
> +	SC_ALLOW_ARG(ioctl, 1, ICARSAMODEXPO),
> +	SC_ALLOW_ARG(ioctl, 1, ICARSACRT),
> +#endif /* defined(__NR_ioctl) && defined(__s390__) */
>  
>  	/* Default deny */
>  	BPF_STMT(BPF_RET+BPF_K, SECCOMP_FILTER_FAIL),
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev at mindrot.org
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
>