[RFE] Add a field to accept the default verify_ca path

Bug #2040236 reported by ZhouHao
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Ironic
Triaged
Wishlist
ZhouHao

Bug Description

We have created an enhancement(https://github.com/openshift/enhancements/pull/1395) in openshift-enhancements that allows users to provide a local CA certificate for IPI installation or subsequent node management, ensuring secure communication with the bare metal BMC using a local CA issued certificate or using a self-signed certificate.

This is a proposal for the specific implementation of the enhancement in Ironic. The goal of this proposal is to add an option for accepting a default verify_ca path. This path will then be used to access the certificate for verification during the communication between Ironic and the nodes.

Below are our implementation details.

We can break down the implementation into two main steps:

1. Adding a new option default_verify_ca_path for each driver

Adding a default_verify_ca_path option (same like kernel_append_params) to each driver's conf to accept the specified path.

2. Retrieving the path before node verification

Before performing the node verification, retrieve the certificate path, and pass it to verify_ca for validation. This implementation vary based on different vendors.

Tags: needs-spec rfe
ZhouHao (zhouhao3)
Changed in ironic:
assignee: nobody → ZhouHao (zhouhao3)
ZhouHao (zhouhao3)
description: updated
Revision history for this message
Jay Faulkner (jason-oldos) wrote :

We discussed this RFE at the weekly Ironic meeting and are not ready to approve it at this time.

There are a few items we need clarity on:

- What is the use case for needing this value on a node level override?
- If this is scoped as a [conductor] level config, will it apply conductor-wide?

During the meeting, further security concerns came up
- How is the user expected to get the certificate onto the conductor securely?

For these reasons, we feel it's best you provide a spec. https://opendev.org/openstack/ironic-specs Please provide additional detail as well on use cases.

Thanks!

-Jay

tags: added: needs-spec
Revision history for this message
ZhouHao (zhouhao3) wrote :

Thanks for your reply. We will carefully consider your opinions and create a corresponding spec sheet after we have a complete design.

Revision history for this message
Dmitry Tantsur (divius) wrote :

I'm somewhat surprised by the concerns here.

> What is the use case for needing this value on a node level override?

Passing the CA through a configuration option is actually the only usable way to do it: passing it through the Node requires somehow knowing local paths on the conductor and being able to upload files there.

> How is the user expected to get the certificate onto the conductor securely?

A *user* neither can nor should (see above). CA certificates are deployment-wide, I would not expect a sane operator to use different local CA's for different bare-metal machines. So the literal answer to the question is: via container options, puppet, ansible or whatever way is used to install Ironic.

Changed in ironic:
status: New → Triaged
importance: Undecided → Wishlist
Revision history for this message
Jay Faulkner (jason-oldos) wrote :

So going to clarify concerns here:

1:

The prose says we want a conductor config override, but the example code implements a node-level override as well. We are only OK with implementing a conductor config override, but doing a node-level override is not a good idea because it's an API-exposed configuration without an API-exposed manner for that CA to go into place.

2:

The proposal wants to add [conductor]/default_verify_ca_path, but example code is for iRMC. If we are adding this config at [conductor], we must add support at a higher level than iRMC driver.

Revision history for this message
Dmitry Tantsur (divius) wrote :

Thanks Jay! I've removed the code samples from the RFE since they're not helpful here (we will review code once the proposal is accepted).

Maybe we just need to make sure every driver has an option like this [redfish]verify_ca, [irmc]verify_ca, etc. This is verbose, but that's what we do with kernel parameters. The Metal3's ironic-image could set all of them (again, the same way it handles kernel_append_params).

description: updated
ZhouHao (zhouhao3)
description: updated
Revision history for this message
ZhouHao (zhouhao3) wrote :

Thanks Dmitry and Jay! I changed this RFE based on your discussion.
I put the default_verify_ca_path option in each driver, not in the conductor. Please see the RFE for details.

I'm not sure if I still need to create a spec for just such a change?

If there are no other questions, I plan to put this RFE on the agenda for the next community meeting.

ZhouHao (zhouhao3)
description: updated
description: updated
ZhouHao (zhouhao3)
description: updated
description: updated
ZhouHao (zhouhao3)
description: updated
Revision history for this message
ZhouHao (zhouhao3) wrote :

I have updated the demo of the specific implementation of iRMC in RFE and deleted a misunderstood comment.

description: updated
ZhouHao (zhouhao3)
description: updated
ZhouHao (zhouhao3)
description: updated
Revision history for this message
Jay Faulkner (jason-oldos) wrote :

Having to fill out a spec shouldn't be seen as a negative thing -- I often do it even for small features just to make sure I've thought about all the possible things that might need to change.

This is a case of a feature which could have unintended consequences, and so it's important for it to go through that spec review. If you can put something up, we'll prioritize getting feedback and getting it approved quickly.

Revision history for this message
ZhouHao (zhouhao3) wrote :

Apologies for not mentioning it here earlier, but I have already submitted the relevant spec to https://review.opendev.org/c/openstack/ironic-specs/+/912050. Please take a moment to review it. Thank you!

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to ironic-specs (master)

Reviewed: https://review.opendev.org/c/openstack/ironic-specs/+/912050
Committed: https://opendev.org/openstack/ironic-specs/commit/21a3aa613a241441f0062b018ed97c46559c57c6
Submitter: "Zuul (22348)"
Branch: master

commit 21a3aa613a241441f0062b018ed97c46559c57c6
Author: Zhou Hao <email address hidden>
Date: Tue Mar 12 17:39:12 2024 +0800

    Add field to accept the default verify_ca path

    Signed-off-by: Zhou Hao <email address hidden>
    Change-Id: I89f846d6b53e84de6cc371724476a3963fc37d02
    Related-Bug: #2040236

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.