search

gridcf / gct

Grid Community Toolkit
Apache License 2.0
48 stars 30 forks source link

Include GSI-OpenSSH 8.8p1 w/HPN 15.2 from Fedora 37 #212

Closed fscheiner closed 1 year ago

fscheiner commented 1 year ago

based on https://kojipkgs.fedoraproject.org//packages/gsi-openssh/8.8p1/4.fc37/src/gsi-openssh-8.8p1-4.fc37.src.rpm

fscheiner commented 1 year ago

Yet untested. Waiting for CI build result.

fscheiner commented 1 year ago

@msalle, @ellert

UPDATE:

We have build errors for *7, *8 and *9 RHEL compatible OSes.

The problem for *9 is easy to fix by adding our fips_mode_replacement.h also to kexgen.c and ssh-ed25519.c, I assume.

The problems for both 7 and 8 are due to rsa_generate_private_key() in sshkey.c using EVP_PKEY_CTX_[...]() functions from OpenSSL 3.x not available in the older OpenSSL versions of 7 and 8. Not sure what to do here. We could for example use the older version:

https://github.com/gridcf/gct/blob/master/gsi_openssh/source/sshkey.c#L1692..L1723

...when we don't have OpenSSL 3.0. The interface stayed the same between GSI-OpenSSH 8.7 and 8.8 for this function (BTW, the new version comes from this patch).

What do you think about that?

msalle commented 1 year ago

@msalle, @ellert

UPDATE:

We have build errors for *7, *8 and *9 RHEL compatible OSes.

The problem for *9 is easy to fix by adding our fips_mode_replacement.h also to kexgen.c and ssh-ed25519.c, I assume.

The problems for both 7 and 8 are due to rsa_generate_private_key() in sshkey.c using EVP_PKEY_CTX_[...]() functions from OpenSSL 3.x not available in the older OpenSSL versions of 7 and 8. Not sure what to do here. We could for example use the older version:

https://github.com/gridcf/gct/blob/master/gsi_openssh/source/sshkey.c#L1692..L1723

...when we don't have OpenSSL 3.0. The interface stayed the same between GSI-OpenSSH 8.7 and 8.8 for this function (BTW, the new version comes from this patch).

What do you think about that?

Sounds definitely like the best thing to do. And then using an #if to check for the right OpenSSL version of course

fscheiner commented 1 year ago

@msalle The needed changes have been made and it now builds on all build targets, see the following links for the implementation:

msalle commented 1 year ago

@msalle The needed changes have been made and it now builds on all build targets, see the following links for the implementation:

* [kexgen.c](https://github.com/gridcf/gct/blob/f60262da4d171415c8482cdaeec4025a61ce8cab/gsi_openssh/source/kexgen.c#L36)

* [ssh-ed25519.c](https://github.com/gridcf/gct/blob/f60262da4d171415c8482cdaeec4025a61ce8cab/gsi_openssh/source/ssh-ed25519.c#L29)

* [sshkey.c: rsa_generate_private_key()](https://github.com/gridcf/gct/blob/f60262da4d171415c8482cdaeec4025a61ce8cab/gsi_openssh/source/sshkey.c#L1741..L1830)

* [sshkey.c: ecdsa_generate_private_key()](https://github.com/gridcf/gct/blob/f60262da4d171415c8482cdaeec4025a61ce8cab/gsi_openssh/source/sshkey.c#L1905..L1973)

These 4 seem fine to me

fscheiner commented 1 year ago

With a disclaimer that I haven't checked all the files I think it should be ok to merge. Have you already tried an actual real-life test, i.e. setup its server and client?

Good point. I think initially I wanted to see if everything builds fine before doing any functional testing. So it was actually a little too early to ask for another review, although it works fine incl. user change (after installing a PAM configuration file for gsisshd (i.e. cp /etc/pam.d/sshd /etc/pam.d/gsisshd)), when built and tested on openSUSE Leap 15.4 on x86_64 now:

johndoe@gridftp-5:~/git-projects/fscheiner/gsissh-tests> ~/bin/test-gss-kex-for-gsi-openssh.bash
Usage: /home/johndoe/bin/test-gss-kex-for-gsi-openssh.bash HOST_FQDN REMOTE_USER [PREFIX]

johndoe@gridftp-5:~/git-projects/fscheiner/gsissh-tests> sudo ~/bin/test-gss-kex-for-gsi-openssh.bash gridftp-5.machine-hall.org johndoe2
gsisshd: OpenSSH_8.8p1c-GSI GSI-hpn15v2, OpenSSL 1.1.1l  24 Aug 2021 SUSE release 150400.7.25.1
gsissh: OpenSSH_8.8p1c-GSI GSI-hpn15v2, OpenSSL 1.1.1l  24 Aug 2021 SUSE release 150400.7.25.1

Wait 3 seconds for startup of gsisshd ...

gss-gex-sha1- OK
gss-group1-sha1- OK
gss-group14-sha256- OK
gss-nistp256-sha256- OK
gss-curve25519-sha256- OK
gss-group16-sha512- OK

test-gss-kex-for-gsi-openssh.bash

So I think we're good to go.

In principle the test-suite should have tested that effectively of course.

Not sure if this also has test cases for the GSI functionality in GSI-OpenSSH.

msalle commented 1 year ago

With a disclaimer that I haven't checked all the files I think it should be ok to merge. Have you already tried an actual real-life test, i.e. setup its server and client?

Good point. I think initially I wanted to see if everything builds fine before doing any functional testing. So it was actually a little too early to ask for another review, although it works fine incl. user change (after installing a PAM configuration file for gsisshd (i.e. cp /etc/pam.d/sshd /etc/pam.d/gsisshd)),

shouldn't that be part of the installation?

[...]

Not sure if this also has test cases for the GSI functionality in GSI-OpenSSH.

Doesn't look like it. In principle possible to pull up a test CA with host and client certs and then configure some gsi-callout to do an appropriate mapping, but that would take quite some effort I guess (I don't have it currently unfortunately).

fscheiner commented 1 year ago

shouldn't that be part of the installation?

When looking at the corresponding RPM spec file, this file gets only installed during RPM installation, so it's not part of make install IIC.

Not sure if this also has test cases for the GSI functionality in GSI-OpenSSH.

Doesn't look like it. In principle possible to pull up a test CA with host and client certs and then configure some gsi-callout to do an appropriate mapping, but that would take quite some effort I guess (I don't have it currently unfortunately).

I think manually testing the GSI functionality with my script does the job for now.

msalle commented 1 year ago

shouldn't that be part of the installation?

When looking at the corresponding RPM spec file, this file gets only installed during RPM installation, so it's not part of make install IIC.

Ah right, makes sense. Actually line above btw: line 479, plus also line 565 to get it in the RPM without overwriting it.

Not sure if this also has test cases for the GSI functionality in GSI-OpenSSH.

Doesn't look like it. In principle possible to pull up a test CA with host and client certs and then configure some gsi-callout to do an appropriate mapping, but that would take quite some effort I guess (I don't have it currently unfortunately).

I think manually testing the GSI functionality with my script does the job for now.

I agree!

fscheiner commented 1 year ago

When looking at the corresponding RPM spec file, this file gets only installed during RPM installation, so it's not part of make install IIC.

Ah right, makes sense. Actually line above btw: line 479

Whoops, indeed.

Merging this now.