Fetching by secret container doesn't raises 404 exception

[Jorge Niedbalski]

[Impact]

Users of Ubuntu bionic running openstack clouds >= rocky
can't create octavia load balancers listeners anymore since the backport of the following patch:

https://opendev.org/openstack/octavia/commit/a501714a76e04b33dfb24c4ead9956ed4696d1df

This change was introduced as part of the following backports and
their posterior syncs into the current Bionic version.

**** IMPACTED VERSIONS NOTE ****

This issue can be triggered in standalone without any cloud-archive dependency and affects python-barbicanclient 4.6.0ubuntu1, which is the Bionic version. The issue was fixed in 4.8.1-0ubuntu1 (disco onwards).

However, this exception gets easily manifested in OpenStack deployments
that uses octavia packages from UCA + python-barbicanclient 4.6.0ubuntu1, as it provides direct interaction with the barbican client.

This means that any Ubuntu openstack cloud deployed from UCA on release >= rocky will manifest this issue when deployed on top of Bionic

 octavia-api | 3.0.0-0ubuntu3~cloud0 | rocky | all
 octavia-api | 4.0.0-0ubuntu1.1~cloud0 | stein | all
 octavia-api | 4.0.0-0ubuntu1~cloud0 | train | all

This change added a new exception handler in the code
that manages the decoding of the given PCKS12 certicate bundle when the listener is created, this handler now captures the PCKS12 decoding error and then raises it preventing
the listener creation to happen (when its invoked with i.e.: --default-tls-container="https://10.5.0.4:9312/v1/containers/68154f38-fccf-4990-b88c-86eb3cc7fe1a" ) , this was originally being hidden
under the legacy code handler as can be seen here:

https://opendev.org/openstack/octavia/commit/a501714a76e04b33dfb24c4ead9956ed4696d1df

This exception is raised because the barbicanclient doesn't know how to distinguish between a given secret and a container, therefore, when the
user specifies a container UUID the client tries to fetch a secret with that uuid (including the /containers/UUID path) and a error 400 (not the expected 404 http error) is returned.

The change proposed on the SRU makes the client aware of container and secret UUID(s) and is able to split the path to distinguish a non-secret (such as a container), in that way if a container is passed, it fails to pass the parsing validation and the right return code (404) is returned by the client.

If a error 404 gets returned, then the except Exception block gets
executed and the legacy driver code for decoding the pcks12 certicate in octavia is invoked, this legacy
driver is able to decode the container payloads and the decoding of the pcks12 certificate succeeds.

This differentiation was implemented here:

https://github.com/openstack/python-barbicanclient/commit/6651c8ffce48ce7ff08f5563a8e6212677ea0468

As an example (this worked before the latest bionic version was pushed)

openstack loadbalancer listener create --protocol-port 443 --protocol "TERMINATED_HTTPS" --name "test-listener" --default-tls-container="https://10.5.0.4:9312/v1/containers/68154f38-fccf-4990-b88c-86eb3cc7fe1a" -- lb1

With the newest package upgrade this creation will fail with the following exception:

The PKCS12 bundle is unreadable. Please check the PKCS12 bundle validity. In addition, make sure it does not require a pass phrase. Error: [('asn1 encoding routines', 'asn1_d2i_read_bio', 'not enough data')] (HTTP 400) (Request-ID: req-8e48d0b5-3f5b-4d26-9920-72b03343596a)

Further rationale on this can be found on https://storyboard.openstack.org/#!/story/2007371

[Test Case]

1) Deploy this bundle or similar (http://paste.ubuntu.com/p/cgbwKNZHbW/)

2) Create self-signed certificate, key and ca (http://paste.ubuntu.com/p/xyyxHZGDFR/)

3) Create the 3 certs at barbican

$ openstack secret store --name "test-pk-1" --secret-type "private" --payload-content-type "text/plain" --payload="$(cat ./keys/controller_key.pem)"

$ openstack secret store --name "test-ca-1" --secret-type "certificate" --payload-content-type "text/plain" --payload="$(cat ./keys/controller_ca.pem)"

$ openstack secret store --name "test-pub-1" --secret-type "certificate" --payload-content-type "text/plain" --payload="$(cat ./keys/controller_cert.pem)"

4) Create a loadbalancer
$ openstack loadbalancer create --name lb1 --vip-subnet-id private_subnet

5) Create a secrets container

$ openstack secret container create --type='certificate' --name "test-tls-1" --secret="certificate=https://10.5.0.4:9312/v1/secrets/3c9109d9-05e0-45fe-9661-087c50061c00" --secret="private_key=https://10.5.0.4:9312/v1/secrets/378e8f8c-81f5-4b5a-bffd-c0c43a41b4a8" --secret="intermediates=https://10.5.0.4:9312/v1/secrets/07a7564d-b5c6-4433-a0a9-a195e2d54c57"

6) Try to create the listener

openstack loadbalancer listener create --protocol-port 443 --protocol "TERMINATED_HTTPS" --name "test-listener" --default-tls-container="https://10.5.0.4:9312/v1/containers/68154f38-fccf-4990-b88c-86eb3cc7fe1a" -- lb1

With the newest package upgrade this creation will fail with the following exception:

The PKCS12 bundle is unreadable. Please check the PKCS12 bundle validity. In addition, make sure it does not require a pass phrase. Error: [('asn1 encoding routines', 'asn1_d2i_read_bio', 'not enough data')] (HTTP 400) (Request-ID: req-8e48d0b5-3f5b-4d26-9920-72b03343596a)

[Regression Potential]

* Creation and List/Get secrets by UUID and with different prefixes (as container secrets) and how this can affect is something to validate with the new SRU.

* Please remember that this breakage is only exposed with octavia-api from UCA >= rocky, and affects a very minor subset of users that make use of the default-tls-container option when creating the listener.
The change considers both cases for compatibility so no breakage is expected on this front.
* Also the unit and functional tests have been included in the SRU changeset in order to ensure that no functionality is broken.

[Discussion]

The following changesets needs to be backported into the bionic version 4.6.0-0ubuntu1

All of those are part of 4.8.0 onward.

** https://github.com/openstack/python-barbicanclient/commit/6651c8ffce48ce7ff08f5563a8e6212677ea0468
** https://github.com/openstack/python-barbicanclient/commit/4eec7121b39de3849b469c56d85b95520aab7bad

Corresponding reviews

https://review.opendev.org/#/c/602810/
https://review.opendev.org/#/c/628046/

[Corey Bryant (corey.bryant)]

Thanks Jorge. Uploaded to Bionic unapproved queue:
https://launchpad.net/ubuntu/bionic/+queue?queue_state=1&queue_text=python-barbicanclient

[Robie Basak (racb)]

SRU review:

Next time, please include dep3 headers such as Origin:. Given that you pinged in #ubuntu-devel to get this looked at, it would really help if you did the things that make SRU reviews easier. However this isn't a blocker. Apart from this the upload itself looks good.

> As per https://storyboard.openstack.org/#!/story/2007371 we identified that
ubuntu clouds running the version 4.6.0 (bionic) aren't raising a 404
error when a secret container is passed.

> This causes the code to not fall back into the legacy mode

I don't understand how this justifies this bug for an SRU. Please could you explain the actual user impact, so I can measure that against SRU criteria? See https://wiki.ubuntu.com/StableReleaseUpdates#Procedure "An explanation of the bug on users and justification for backporting the fix to the stable release"

> [Regression Potential]

This is missing "a discussion of how regressions are most likely to manifest, or may manifest even if it is unlikely, as a result of this change". See https://wiki.ubuntu.com/StableReleaseUpdates#Procedure

Please update the bug description, fixing [Impact] and [Regression Potential] as above, and then we can reconsider your upload. Thanks!

[Robie Basak (racb)]

Thank you for the update! The user impact is much more understandable now.

The Regression Potential section is still missing "a discussion of how regressions are most likely to manifest, or may manifest even if it is unlikely, as a result of this change". Please can you update that also?

> Users of Ubuntu bionic running openstack clouds >= rocky
can't create octavia load balancers listeners anymore since the backport of the following patch...

Which package update in Ubuntu caused this regression? Or was the update in the cloud archive only, which case shouldn't this backport also be going into the cloud archive and not into the main Ubuntu archive?

[Jorge Niedbalski (niedbalski)]

@racb, thank you very much for the review. I've updated description, versions and the regression potential of the SRU template.

[Robie Basak (racb)]

Thanks! I think this is a rather special case. Since the Ubuntu archive isn't itself affected by this bug, it's not obvious to me that an SRU to the Ubuntu archive is justified, instead of expecting this to be fixed in UCA only. I've made a note to discuss this with the SRU team.

[Robie Basak (racb)]

> I can't identify any regression potential for the fix...

I don't understand why this makes it impossible to identify the area of functionality being changed to assist with targeted testing.

[Jorge Niedbalski (niedbalski)]

Hello @racb @coreycb,

I've updated the description, testing and others sections. Is there anything left to be done at my side to continue with the SRU?

Thank you.

[Dan Streetman (ddstreet)]

since this is in the Bionic unapproved upload queue already, i'm removing ubuntu-sponsors. I'm leaving sts-sponsors subscribed to help nudge the upload through until it reaches bionic-updates.

[Łukasz Zemczak (sil2100)]

Thank you for preparing this SRU! I think I'm fine with the SRU contents-wise, but I'd like to first have some clarification on the situation here. So as I understand it correctly, users running bionic only (without UCA enabled) also experience this issue now, yes?

[Jorge Niedbalski (niedbalski)]

@sil2100 hello lukasz

I've added a **IMPACTED VERSIONS NOTE **** note in the description. Any user
running Bionic may hit this issue if running the library in standalone
and hitting the same endpoints. However, this is unlikely to be manifested
by any user, unless it is deployed with octavia (which is in the cloud-archive). This component (octavia-api) makes extensive use of the barbicanclient API and therefore any clouds >= rocky
deployed on top of Bionic will manifest the issue.

I hope this clarifies the situation further and if not, please let me know
to provide you any further details.

[Łukasz Zemczak (sil2100)]

Hello Jorge, or anyone else affected,

Accepted python-barbicanclient into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/python-barbicanclient/4.6.0-0ubuntu1.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Jorge Niedbalski (niedbalski)]

Hello,

I've verified that the current -proposed package fixes the issue for us, for the
given use case.

Using the following deployment bundle on a Bionic + Rocky cloud
http://paste.ubuntu.com/p/jnVdVvQg7k/

Without the patch, the problem is reproduced as expressed on the case description:

ubuntu@niedbalski-bastion:~/stsstack-bundles/openstack/00268110$ openstack secret container create --type='certificate' --name "test-tls-1" --secret="certificate=https://10.5.0.11:9312/v1/secrets/7aa7727d-f39b-45f8-9310-f5c595ad4feb" --secret="private_key=https://10.5.0.11:9312/v1/secrets/189736d1-51d8-4cbe-9638-ceadcbb664ac" --secret="intermediates=https://10.5.0.11:9312/v1/secrets/70e2cf9c-8110-4d25-a1e3-f7b6f3950e64"

ubuntu@niedbalski-bastion:~/stsstack-bundles/openstack/00268110$ openstack loadbalancer listener create --protocol-port 443 --protocol "TERMINATED_HTTPS" --name "test-listener" --default-tls-container="https://10.5.0.11:9312/v1/containers/b548ab63-474d-4a94-b121-4eae8193fcc1" -- lb1
The PKCS12 bundle is unreadable. Please check the PKCS12 bundle validity. In addition, make sure it does not require a pass phrase. Error: [('asn1 encoding routines', 'asn1_d2i_read_bio', 'not enough data')] (HTTP 400) (Request-ID: req-c79fbcb1-06d8-47e4-9754-8066596ba262)

With the patch applied in the following version:

root@juju-be44b9-barbican-10:/home/ubuntu# dpkg -l |grep barbican
ii python3-barbicanclient 4.6.0-0ubuntu1.1 all OpenStack Key Management API client - Python 3.x

| https://10.5.0.11:9312/v1/containers/bd67d6f4-3a82-4a86-9679-c97a66ceeb19 | None | 2020-05-12T21:37:32+00:00 | ACTIVE | certificate | certificate=https://10.5.0.11:9312/v1/secrets/26ed5706-5f0a-4f9f-b226-e8595031515e | None |
| | | | | | private_key=https://10.5.0.11:9312/v1/secrets/9a3bd926-6ba9-46be-8168-6b5e79e09b36 | |
+---------------------------------------------------------------------------+----------------+---------------------------+--------+-------------+--------------------------------------------------------------------------------------+-----------+

The issue isn't longer reproducible and listeners can be created.

ubuntu@niedbalski-bastion:~/stsstack-bundles/openstack/00268110$ openstack loadbalancer listener create --protocol-port 443 --protocol "TERMINATED_HTTPS" --name "test-listener-2" --default-tls-container="https://10.5.0.11:9312/v1/containers/bd67d6f4-3a82-4a86-9679-c97a66ceeb19" -- lb2
+-----------------------------+---------------------------------------------------------------------------+
| Field | Value |
+-----------------------------+---------------------------------------------------------------------------+
| admin_state_up | True |
| connection_limit | -1 |
| created_at ...

[Corey Bryant (corey.bryant)]

Hello Jorge, or anyone else affected,

Accepted python-barbicanclient into queens-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:queens-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-queens-needed to verification-queens-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-queens-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Launchpad Janitor (janitor)]

This bug was fixed in the package python-barbicanclient - 4.6.0-0ubuntu1.1

---------------
python-barbicanclient (4.6.0-0ubuntu1.1) bionic; urgency=medium

  [ Corey Bryant ]
  * d/gbp.conf: Create stable/queens branch.

  [ Jorge Niedbalski ]
  * d/p/0001-Allow-fetching-by-UUID-and-respect-interface.patch,
  * d/p/0002-Secret-payload-should-also-be-fetched-by-UUID.patch:
    Adds patches to properly support fetching by UUID (LP: #1867676).

 -- Jorge Niedbalski <email address hidden> Tue, 17 Mar 2020 11:13:24 -0300

[Brian Murray (brian-murray)]

The verification of the Stable Release Update for python-barbicanclient has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Jorge Niedbalski (niedbalski)]

Hello Corey,

This package is available on B, E, F, G. I don't think is strictly required to be backported into the queens cloud archive for other than having this package in the correct pocket (UCA).

As I expressed in the bug description, The combination that triggers this issue is octavia + barbican when creating listeners, but octavia only exists >= Rocky. So I doubt that this could be hit in any other situation other than a specific user using the standalone library of barbicanclient, that would be a VERY rare use case.

The particular issue reported here was reported on Bionic (LTS) >= Rocky clouds (where the UCA package was missed), and the bionic package in universe was missing the fix.

I think its OK if you want to move this very same missing package version into the UCA, but I think is safe to assume that the verification done for Bionic would apply for anyone brave enough for deploying a version of Queens barbican + Rocky Octavia.

I hope this clarifies the situation.

[Corey Bryant (corey.bryant)]

The verification of the Stable Release Update for python-barbicanclient has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Corey Bryant (corey.bryant)]

This bug was fixed in the package python-barbicanclient - 4.6.0-0ubuntu1.1~cloud0
---------------

 python-barbicanclient (4.6.0-0ubuntu1.1~cloud0) xenial-queens; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 python-barbicanclient (4.6.0-0ubuntu1.1) bionic; urgency=medium
 .
   [ Corey Bryant ]
   * d/gbp.conf: Create stable/queens branch.
 .
   [ Jorge Niedbalski ]
   * d/p/0001-Allow-fetching-by-UUID-and-respect-interface.patch,
   * d/p/0002-Secret-payload-should-also-be-fetched-by-UUID.patch:
     Adds patches to properly support fetching by UUID (LP: #1867676).