Magnum

Insecure certificate requests

Bug #1663757 reported by Adrian Otto
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Magnum
Fix Released
High
yatin
Magnum queens-3
Pike
Fix Released
Undecided
Unassigned
Queens
Fix Released
Undecided
Unassigned

Bug Description

In October, our team merged a patch to solve a problem with certificate transfers between cluster nodes and the Magnum API.

https://review.openstack.org/383493

The problem for this resolution is that it causes Bandit tests to fail, as explained here:

http://lists.openstack.org/pipermail/openstack-dev/2017-February/111931.html Make certs insecure in magnum

Please re-implement a solution that uses TLS connections by default, but optionally allows operators to relax certificate verification or use an insecure HTTP transport if their deployments require those workarounds.

Revision history for this message
Adrian Otto (aotto) wrote :

Adding the CA cert to each cluster node at create time would allow for proper operation. We could allow this to be configured in the cluster driver(s).

Vijendar Komalla (vijendar-komalla)
Changed in magnum:
assignee: nobody → Vijendar Komalla (vijendar-komalla)
OpenStack Infra (hudson-openstack)
Changed in magnum:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to magnum (master)

Fix proposed to branch: master
Review: https://review.openstack.org/447685

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.openstack.org/447686

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.openstack.org/447687

Revision history for this message
Spyros Trigazis (strigazi) wrote :

@vijendar

Remember to add reno and update the docs

Revision history for this message
Spyros Trigazis (strigazi) wrote :

In the same bug we could remove the python scripts from swarm and do it like in kubernetes with bash only.

Vijendar, What do you think?

Revision history for this message
Vijendar Komalla (vijendar-komalla) wrote :

sure. we can do the bash. I will work on it.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.openstack.org/450292

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on magnum (master)

Change abandoned by Vijendar Komalla (<email address hidden>) on branch: master
Review: https://review.openstack.org/450292
Reason: duplicate

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Change abandoned by Vijendar Komalla (<email address hidden>) on branch: master
Review: https://review.openstack.org/447686

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to magnum (master)

Fix proposed to branch: master
Review: https://review.openstack.org/450310

Revision history for this message
Adrian Otto (aotto) wrote :

Spyros made this remark in comment #6:

> In the same bug we could remove the python scripts from swarm and do it like in kubernetes with bash only.

I think we should consider the opposite.

The problem with using bash scripts rather than python is that we don't end up with unit tests to cover that code. I would rather see our shell scripts ported to python scripts with suitable testing added to them. Simply put: if python is present, we should prefer it (with tests) over shell scripts (without tests).

Spyros, was your suggestion made in an effort to have consistent implementation between drivers, or was there some other motivation you had in mind?

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on magnum (master)

Change abandoned by Vijendar Komalla (<email address hidden>) on branch: master
Review: https://review.openstack.org/447143
Reason: merged into single patch https://review.openstack.org/#/c/447687/

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Change abandoned by Vijendar Komalla (<email address hidden>) on branch: master
Review: https://review.openstack.org/447685
Reason: merged into single patch https://review.openstack.org/#/c/447687/

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Change abandoned by Vijendar Komalla (<email address hidden>) on branch: master
Review: https://review.openstack.org/450310
Reason: merged into single patch https://review.openstack.org/#/c/447687/

Spyros Trigazis (strigazi)
Changed in magnum:
assignee: Vijendar Komalla (vijendar-komalla) → Kirsten G. (oikiki)
Revision history for this message
Spyros Trigazis (strigazi) wrote :

Let's fix this bug in the following way:

pick-up patch [1] which adds a new field in the cluster template and CHANGE it.

Instead of adding a new field we should add a configuration parameter in
magnum.conf and pass it to the heat-templates similarly to patch [1].

Create a new fine in [2] named drivers and add a configuration parameter there,
named verify_ca which will default to True.

[1] https://review.openstack.org/#/c/447687
[2] http://git.openstack.org/cgit/openstack/magnum/tree/magnum/conf

Changed in magnum:
milestone: none → ocata-3
milestone: ocata-3 → queens-3
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to magnum (master)

Reviewed: https://review.openstack.org/447687
Committed: https://git.openstack.org/cgit/openstack/magnum/commit/?id=b07b6f34d5b85b57ff0aafc57cb5a268d34aff13
Submitter: Zuul
Branch: master

commit b07b6f34d5b85b57ff0aafc57cb5a268d34aff13
Author: Kirsten G <email address hidden>
Date: Wed Oct 25 01:27:40 2017 -0700

    Add verify_ca configuration parameter

    Added configuration parameter, verify_ca, to magnum.conf with default
    value of True. This parameter is passed to the heat templates to
    indicate whether the cluster nodes validate the Certificate Authority
    when making requests to the OpenStack APIs (Keystone, Magnum, Heat).
    This configuration parameter can be set to False to disable CA
    validation.

    Co-Authored-By: Vijendar Komalla <email address hidden>

    Change-Id: Iab02cb1338b811dac0c147378dbd0e63c83f0413
    Partial-Bug: #1663757

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to magnum (stable/pike)

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/529166

Revision history for this message
Spyros Trigazis (strigazi) wrote :

Kirsten can you fix this error:
https://review.openstack.org/#/c/529166/1/magnum/drivers/common/templates/kubernetes/fragments/wc-notify-master.sh

verify_ca is not translated to '-k', it passes True

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to magnum (master)

Fix proposed to branch: master
Review: https://review.openstack.org/529852

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to magnum (stable/pike)

Reviewed: https://review.openstack.org/529166
Committed: https://git.openstack.org/cgit/openstack/magnum/commit/?id=1f4a13e9e88aa2844431db7147c7df939994c271
Submitter: Zuul
Branch: stable/pike

commit 1f4a13e9e88aa2844431db7147c7df939994c271
Author: Kirsten G <email address hidden>
Date: Wed Oct 25 01:27:40 2017 -0700

    Add verify_ca configuration parameter

    Added configuration parameter, verify_ca, to magnum.conf with default
    value of True. This parameter is passed to the heat templates to
    indicate whether the cluster nodes validate the Certificate Authority
    when making requests to the OpenStack APIs (Keystone, Magnum, Heat).
    This configuration parameter can be set to False to disable CA
    validation.

    Co-Authored-By: Vijendar Komalla <email address hidden>

    Change-Id: Iab02cb1338b811dac0c147378dbd0e63c83f0413
    Partial-Bug: #1663757

tags: added: in-stable-pike
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to magnum (master)

Reviewed: https://review.openstack.org/529852
Committed: https://git.openstack.org/cgit/openstack/magnum/commit/?id=ba1e7b82fbd4c3b64d7d8d47f2036e97d18b5392
Submitter: Zuul
Branch: master

commit ba1e7b82fbd4c3b64d7d8d47f2036e97d18b5392
Author: Kirsten G <email address hidden>
Date: Fri Dec 22 08:20:18 2017 -0800

    Add missing translation for verify_ca

    verify_ca should be translated to ""/"-k" before use.

    Change-Id: I2c51b3cb26ad9a703d5ecfe5090be87ab59dfee6
    Partial-Bug: #1663757

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to magnum (master)

Fix proposed to branch: master
Review: https://review.openstack.org/531601

Changed in magnum:
assignee: Kirsten G. (oikiki) → yatin (yatinkarel)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to magnum (stable/pike)

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/531602

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to magnum (master)

Reviewed: https://review.openstack.org/531601
Committed: https://git.openstack.org/cgit/openstack/magnum/commit/?id=192dc8b1fb9d4f6b3512f4bdbbf6ff101f5049fa
Submitter: Zuul
Branch: master

commit 192dc8b1fb9d4f6b3512f4bdbbf6ff101f5049fa
Author: yatin <email address hidden>
Date: Sun Jan 7 19:50:38 2018 +0530

    [k8s] Add missing verify_ca in minion_wc_notify

    Change-Id: I1db23b88097fae77377cce5c56e176e9296f76a2
    Partial-Bug: #1663757

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to magnum (stable/pike)

Reviewed: https://review.openstack.org/531602
Committed: https://git.openstack.org/cgit/openstack/magnum/commit/?id=d0f08306150f2f14a5951d0916538471fb028752
Submitter: Zuul
Branch: stable/pike

commit d0f08306150f2f14a5951d0916538471fb028752
Author: Kirsten G <email address hidden>
Date: Fri Dec 22 08:20:18 2017 -0800

    Add missing translation for verify_ca

    verify_ca should be translated to ""/"-k" before use.

    Change-Id: I2c51b3cb26ad9a703d5ecfe5090be87ab59dfee6
    Partial-Bug: #1663757
    (cherry picked from commit ba1e7b82fbd4c3b64d7d8d47f2036e97d18b5392)

    [k8s] Add missing verify_ca in minion_wc_notify

    Change-Id: I1db23b88097fae77377cce5c56e176e9296f76a2
    Partial-Bug: #1663757
    (cherry picked from commit 1ff85eb9702382fc527f1db6a0f99ea33e8c62a8)

Spyros Trigazis (strigazi)
Changed in magnum:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

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