Skip to content

Conversation

@smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Mar 15, 2019

As part of our release process we build container images for installer
that are added to the release image (which has a cryptographic
relationship to the images it contains, giving strong integrity). A
consumer should be able to download and install a locked installer
binary that uses the payload. However, we would prefer to not
rebuild the binary outside of the image, but instead have:

  1. a source for the binary from the payload
  2. the binary be locked to the payload it comes from

This commit allows a build system above the payload to extract the
installer binary for linux from the image (other platforms later)
and perform a replacement on the binary itself, patching:

_RELEASE_IMAGE_LOCATION_XXXXXXXXXXX

with

quay.io/openshift/ocp-release:v4.0\x00

without requiring a recompilation of the binary. The internal code
checks the constant and verifies bounds (panicking if necessary)
and then returns the updated constant. This allows a simpler
replacement process to customize a binary for both external use
and offline use that is locked to a payload.

Chose this over a few other options:

  1. rebuilding outside the payload (loses reproducibility)
  2. not locking payload (loses simplicity for test and users)

Steps after this:

  1. oc can stream back either this binary or a tar from a payload with the proper locking
  2. release-controller can serve that
  3. ART creates the tar as the last step in the promotion flow and sends it to the mirror

/assign @wking

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 15, 2019
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 15, 2019
@smarterclayton smarterclayton force-pushed the allow_installer_binary_to_have_replacement branch from e29667a to 5886dea Compare March 15, 2019 18:49
@wking
Copy link
Member

wking commented Mar 15, 2019

I've pushed d07804b to a branch in my repo dropping the prefix variable (because I don't see a need to distinguish between changed and unchanged) and adding hack/pin-release-image.sh to somewhat reliably make the substitution. Feel free to pick and choose interesting things to squash in from that commit.

As part of our release process we build container images for installer
that are added to the release image (which has a cryptographic
relationship to the images it contains, giving strong integrity). A
consumer should be able to download and install a locked installer
binary that uses the payload. However, we would prefer to not
rebuild the binary outside of the image, but instead have:

1. a source for the binary from the payload
2. the binary be locked to the payload it comes from

This commit allows a build system above the payload to extract the
installer binary for linux from the image (other platforms later)
and perform a replacement on the binary itself, patching:

```
_RELEASE_IMAGE_LOCATION_XXXXXXXXX...
```

with

```
quay.io/openshift/ocp-release:v4.0\x00
```

without requiring a recompilation of the binary. The internal code
checks the constant and verifies bounds (panicking if necessary)
and then returns the updated constant. This allows a simpler
replacement process to customize a binary for both external use
and offline use that is locked to a payload.
@smarterclayton smarterclayton force-pushed the allow_installer_binary_to_have_replacement branch from 5886dea to a31e12f Compare March 25, 2019 17:07
@smarterclayton
Copy link
Contributor Author

I updated with the logging changes. I hope my comments in d07804b were clear - we need the prefix to detect mutation, we don't want the constant the code replaces to have to change by version (i.e. the code that mutates the binary shouldn't change unless we grow the allowed pull spec length in the future).

@smarterclayton smarterclayton changed the title WIP: release: Allow release image to be directly substituted into binary release: Allow release image to be directly substituted into binary Mar 25, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 25, 2019
@wking
Copy link
Member

wking commented Mar 25, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 25, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [smarterclayton,wking]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit f592cd5 into openshift:master Mar 25, 2019
wking added a commit to wking/openshift-installer that referenced this pull request Apr 1, 2019
Using the new approach provided by a31e12f (release: Allow release
image to be directly substituted into binary, 2019-03-15, openshift#1422).  For
example:

  $ hack/build.sh
  $ cp bin/openshift-install bin/openshift-install-orig
  $ hack/pin-release-image.sh bin/openshift-install quay.io/openshift-release-dev/ocp-release:4.0.0-0.8
  $ diff -u <(hexdump -vC bin/openshift-install-orig) <(hexdump -vC bin/openshift-install)
  --- /dev/fd/63			2019-03-25 22:30:43.534719090 -0700
  +++ /dev/fd/62			2019-03-25 22:30:43.534719090 -0700
  @@ -4911608,10 +4911608,10 @@
   04af1f70  20 74 68 65 20 73 61 6d  65 20 49 6e 69 74 69 61  | the same Initia|
   04af1f80  6c 69 7a 65 72 43 6f 6e  66 69 67 75 72 61 74 69  |lizerConfigurati|
   04af1f90  6f 6e 20 69 73 20 70 72  65 73 65 72 76 65 64 2e  |on is preserved.|
  -04af1fa0  00 5f 52 45 4c 45 41 53  45 5f 49 4d 41 47 45 5f  |._RELEASE_IMAGE_|
  -04af1fb0  4c 4f 43 41 54 49 4f 4e  5f 00 58 58 58 58 58 58  |LOCATION_.XXXXXX|
  -04af1fc0  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
  -04af1fd0  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
  +04af1fa0  71 75 61 79 2e 69 6f 2f  6f 70 65 6e 73 68 69 66  |quay.io/openshif|
  +04af1fb0  74 2d 72 65 6c 65 61 73  65 2d 64 65 76 2f 6f 63  |t-release-dev/oc|
  +04af1fc0  70 2d 72 65 6c 65 61 73  65 3a 34 2e 30 2e 30 2d  |p-release:4.0.0-|
  +04af1fd0  30 2e 38 00 58 58 58 58  58 58 58 58 58 58 58 58  |0.8.XXXXXXXXXXXX|
   04af1fe0  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
   04af1ff0  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
   04af2000  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
  @@ -4911628,7 +4911628,7 @@
   04af20b0  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
   04af20c0  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
   04af20d0  58 58 58 58 58 58 58 58  58 58 58 58 58 58 58 58  |XXXXXXXXXXXXXXXX|
  -04af20e0  58 58 58 58 58 58 00 53  65 63 72 65 74 4e 61 6d  |XXXXXX.SecretNam|
  +04af20e0  58 58 58 58 58 58 58 53  65 63 72 65 74 4e 61 6d  |XXXXXXXSecretNam|
   04af20f0  65 20 69 73 20 74 68 65  20 6e 61 6d 65 20 6f 66  |e is the name of|
   04af2100  20 74 68 65 20 73 65 63  72 65 74 20 75 73 65 64  | the secret used|
   04af2110  20 74 6f 20 74 65 72 6d  69 6e 61 74 65 20 53 53  | to terminate SS|
wking added a commit to wking/openshift-installer that referenced this pull request Apr 2, 2019
Catching up with a31e12f (release: Allow release image to be
directly substituted into binary, 2019-03-15, openshift#1422).
wking added a commit to wking/openshift-installer that referenced this pull request Apr 2, 2019
Catching up with a31e12f (release: Allow release image to be
directly substituted into binary, 2019-03-15, openshift#1422).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants