Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TLS keys containing other pem blocks proceeding the private key are considered invalid #1702

Closed
mattalberts opened this issue Oct 15, 2019 · 6 comments · Fixed by #1707
Closed
Assignees

Comments

@mattalberts
Copy link

What steps did you take and what happened:
Created a secret whose EC PRIVATE KEY contains EC PARAMETERS. The function validatePrivateKey locates the EC PARAMETERS pem.Block, attempts to validate it as a private key, and rejects the secret as valid.

What did you expect to happen:
I expected the secret to be accepted as valid :)

Anything else you would like to add:
The private key pem struct is something like this

-----BEGIN EC PARAMETERS-----
-----END EC PARAMETERS-----
-----BEGIN EC PRIVATE KEY-----
-----END EC PRIVATE KEY-----

I did scan, but didn't find a similar issue.

Environment:

  • Contour version: v0.15.1 and master
  • Kubernetes version: (use kubectl version): N/A
  • Kubernetes installer & version: N/A
  • Cloud provider or hardware configuration: N/A
  • OS (e.g. from /etc/os-release): N/A
@mattalberts
Copy link
Author

I already have a patch, just working on a test :)

@jpeach
Copy link
Contributor

jpeach commented Oct 15, 2019

Does Envoy accept and use the additional PEM objects?

mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 15, 2019
Walk each PEM block to identify the PRIVATE KEY

closes projectcontour#1702
@mattalberts
Copy link
Author

mattalberts commented Oct 15, 2019

@jpeach The certificates I've been using with contour versions v0.5.0 to v0.14.2 have contained EC PARAMETERS and were valid for both contour and envoy (e.g. I've been terminating TLS with them for ~2 years). I found this while upgrading to v0.15.1.

Technically, there is a similar flaw with the certificate handling (to less detriment). Currently, a secret will be accepted if the first cert in the PEM block is valid

mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 15, 2019
Walk each PEM block to identify the PRIVATE KEY

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 15, 2019
Walk each PEM block to identify the PRIVATE KEY

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 15, 2019
Walk each PEM block to identify the PRIVATE KEY

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 15, 2019
During certificate validation, walk the PEM and validate each
appropriate block.

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
@youngnick youngnick self-assigned this Oct 15, 2019
@mattalberts
Copy link
Author

@youngnick I think #1707 makes more sense than #1704 (i was trying to keep this very small hoping for a back port .. but the other addresses both the cert and the private key).

@davecheney
Copy link
Contributor

davecheney commented Oct 16, 2019 via email

@youngnick
Copy link
Member

@mattalberts I agree, I'm going to check out #1707 properly now and, assuming it's all good (which a cursory inspection suggests), then I'll close out #1704.

mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 16, 2019
During certificate validation, walk the PEM and validate each
appropriate block.

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 16, 2019
During certificate validation, walk the PEM and validate each
appropriate block.

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 16, 2019
During certificate validation, walk the PEM and validate each
appropriate block.

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 16, 2019
During certificate validation, walk the PEM and validate each
appropriate block.

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 17, 2019
During certificate validation, walk the PEM and validate each
appropriate block.

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 17, 2019
During certificate validation, walk the PEM and validate each
appropriate block.

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 17, 2019
During certificate validation, walk the PEM and validate each
appropriate block.

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 17, 2019
During certificate validation, walk the PEM and validate each
appropriate block.

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 17, 2019
During certificate validation, walk the PEM and validate each
appropriate block.

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 17, 2019
During certificate validation, walk the PEM and validate each
appropriate block.

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
mattalberts pushed a commit to mattalberts/contour that referenced this issue Oct 17, 2019
During certificate validation, walk the PEM and validate each
appropriate block.

closes projectcontour#1702

Signed-off-by: Matt Alberts <[email protected]>
davecheney pushed a commit that referenced this issue Oct 18, 2019
During certificate validation, walk the PEM and validate each
appropriate block.

closes #1702

Signed-off-by: Matt Alberts <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants