Skip to content

Conversation

@psafont
Copy link
Member

@psafont psafont commented Apr 11, 2025

Adds IPv6 addresses to host certificates when their management interfaces are configured to use Dual stack.

I've tested manually on IPv4-only hosts, and the certificates are generated in the same way as before.
I've run the smoke and verification tests (Suite Run 215863), all of them passed

@last-genius you probably want to test these changes, run openssl x509 -text -in /etc/xensource/xapi-ssl.pem on a dual host, and see that it contains the IPv6 and IPv4 addresses.

@psafont psafont requested a review from last-genius April 11, 2025 10:30
@stormi
Copy link
Contributor

stormi commented Apr 11, 2025

We have tests here which can install two hosts in IPv6 and then join them in a pool. Would this be a sufficient check?

(However running them is not really straightforward at the moment, so a manual test might be better for this PR. For future regression testing with IPv6, my question remains)

@psafont
Copy link
Member Author

psafont commented Apr 11, 2025

We have tests here which can install two hosts in IPv6 and then join them in a pool. Would this be a sufficient check?

I think so, yes. Do the test also inspect certificates?

@psafont psafont changed the title IPv6 IPs in host certificates IPv6 IPs in host certificates for dual-stack management interfaces Apr 11, 2025
Copy link
Contributor

@lindig lindig left a comment

Choose a reason for hiding this comment

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

Extended and clever use of Option.fold

@stormi
Copy link
Contributor

stormi commented Apr 11, 2025

I think so, yes. Do the test also inspect certificates?

I don't think so. Also we don't test dual stack yet 😬 (seen the title update)

@last-genius
Copy link
Contributor

I think so, yes. Do the test also inspect certificates?

I don't think so. Also we don't test dual stack yet 😬 (seen the title update)

I will test it manually then on Monday

@last-genius
Copy link
Contributor

xapi on a dualstack host without this PR:

Certificate:
    Data:....
    Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN=10.1.19.21
        Validity...
        Subject: CN=10.1.19.21
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (2048 bit)
                Modulus:  ...
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            X509v3 Subject Alternative Name:
                DNS:xcp-ng-asvds, IP Address:10.1.19.21

with this PR:

Certificate:
    Data: ...
    Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN=xcp-ng-asvds
        Validity ...
        Subject: CN=xcp-ng-asvds
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (2048 bit)
                Modulus: ....
                Exponent: ...
        X509v3 extensions:
            X509v3 Subject Alternative Name:
                DNS:xcp-ng-asvds, IP Address:10.1.19.21, IP Address:2A01:240:AB08:5:19:0:0:21

@psafont
Copy link
Member Author

psafont commented Apr 14, 2025

I didn't expect the issuer and the subject to change. I'm not sure what we want to do here, at some level it makes sense to use the DNS name if possible, the IPs are still available in the SAN anyway.

I checked the code, and the hsotname should be preferred since 2021, in 7ca6ea4

@psafont psafont force-pushed the private/paus/ipv6 branch from 00f37d3 to b2cfaa1 Compare April 14, 2025 15:14
This will be important to have a dual-stack mode

Signed-off-by: Pau Ruiz Safont <[email protected]>
@psafont psafont force-pushed the private/paus/ipv6 branch from b2cfaa1 to 121be30 Compare April 14, 2025 15:22
psafont added 2 commits April 14, 2025 16:25
This is done for host certificates only

Signed-off-by: Pau Ruiz Safont <[email protected]>
@psafont psafont force-pushed the private/paus/ipv6 branch from 121be30 to d94a4bf Compare April 14, 2025 15:25
@psafont psafont enabled auto-merge April 14, 2025 15:25
@psafont psafont added this pull request to the merge queue Apr 14, 2025
Merged via the queue into xapi-project:master with commit e64b4bc Apr 14, 2025
17 checks passed
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.

4 participants