Skip to content

Conversation

@eparis
Copy link
Member

@eparis eparis commented May 31, 2019

The ReadArmoredKeyRing() function from Golang's OpenPGP library only
supports reading from a single armored block. Instead of concatenating
the two armored keys together, the keys need to be dearmored,
concatenated, and then armored. This will allow the OpenPGP library to
read both keys into the keyring.

The resulting armored block looks as follows:

$ gpg --list-packets
:public key packet:
	version 4, algo 1, created 1256212795, expires 0
	pkey[0]: [4096 bits]
	pkey[1]: [17 bits]
	keyid: 199E2F91FD431D51
:user ID packet: "Red Hat, Inc. (release key 2) <security@redhat.com>"
:signature packet: algo 1, keyid 199E2F91FD431D51
	version 4, created 1256212795, md5len 0, sigclass 0x13
	digest algo 2, begin of digest 6c e9
	hashed subpkt 2 len 4 (sig created 2009-10-22)
	hashed subpkt 27 len 1 (key flags: 03)
	hashed subpkt 11 len 5 (pref-sym-algos: 9 8 7 3 2)
	hashed subpkt 21 len 3 (pref-hash-algos: 2 8 3)
	hashed subpkt 22 len 3 (pref-zip-algos: 2 3 1)
	hashed subpkt 30 len 1 (features: 01)
	hashed subpkt 23 len 1 (keyserver preferences: 80)
	subpkt 16 len 8 (issuer key ID 199E2F91FD431D51)
	data: [4095 bits]
:public key packet:
	version 4, algo 1, created 1235485488, expires 0
	pkey[0]: [4096 bits]
	pkey[1]: [17 bits]
	keyid: 938A80CAF21541EB
:user ID packet: "Red Hat, Inc. (beta key 2) <security@redhat.com>"
:signature packet: algo 1, keyid 938A80CAF21541EB
	version 4, created 1246901223, md5len 0, sigclass 0x13
	digest algo 2, begin of digest ff 6d
	hashed subpkt 2 len 4 (sig created 2009-07-06)
	hashed subpkt 27 len 1 (key flags: 03)
	hashed subpkt 11 len 5 (pref-sym-algos: 9 8 7 3 2)
	hashed subpkt 21 len 3 (pref-hash-algos: 2 8 3)
	hashed subpkt 22 len 3 (pref-zip-algos: 2 3 1)
	hashed subpkt 30 len 1 (features: 01)
	hashed subpkt 23 len 1 (keyserver preferences: 80)
	subpkt 16 len 8 (issuer key ID 938A80CAF21541EB)
	data: [4096 bits]

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 31, 2019
@jwforres
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 31, 2019
@eparis eparis changed the title Both keys Concatenate keys into a single armored block May 31, 2019
The `ReadArmoredKeyRing()` function from Golang's OpenPGP library only
supports reading from a single armored block. Instead of concatenating
the two armored keys together, the keys need to be dearmored,
concatenated, and then armored. This will allow the OpenPGP library to
read both keys into the keyring.
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label May 31, 2019
@openshift-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@smarterclayton
Copy link
Contributor

/approve

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eparis, jwforres, smarterclayton

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 31, 2019
@smarterclayton
Copy link
Contributor

/hold

Until the armored keys thing is resolved.

An alternative (which I haven’t tried) to all of these changes is to change the parse order from armored -> unarmored to unarmored -> armored during CVO verify, which should work with the original keys.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 31, 2019
@wking
Copy link
Member

wking commented May 31, 2019

An alternative (which I haven’t tried) to all of these changes is to change the parse order from armored -> unarmored...

Go's unarmor support does not let you reliably unarmor multiple blocks from a single reader, although we could possibly hack together a workaround in the CVO. But the single-armored block here works with existing CVO code (openshift/cluster-version-operator#196), so I prefer this approach.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented May 31, 2019

@eparis: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws 259218e link /test e2e-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@eparis eparis merged commit 704114b into openshift:release-4.1 May 31, 2019
@eparis
Copy link
Member Author

eparis commented Jun 18, 2019

This was a mashup of
#11
#15

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. 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.

6 participants