Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Remove mandatory x509v3 Subject/Authority Key Identifiers in certificate(s) (#3181)#3211

Merged
dangogh merged 1 commit intomasterfrom
unknown repository
Jan 18, 2019
Merged

Remove mandatory x509v3 Subject/Authority Key Identifiers in certificate(s) (#3181)#3211
dangogh merged 1 commit intomasterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jan 12, 2019

Remove mandatory check for x509v3 Subject/Authority Key Identifiers in imported certificate(s)

What does this PR do?

Fixes #3181

Which TC components are affected by this PR?

  • Documentation
  • Grove
  • Traffic Analytics
  • Traffic Monitor
  • Traffic Ops
  • Traffic Ops ORT
  • Traffic Portal
  • Traffic Router
  • Traffic Stats
  • Traffic Vault
  • Other _________

What is the best way to verify this PR?

  1. Start up a Traffic Ops environment with at least one HTTP_and_HTTPS delivery service.
  2. Import a self-signed CA and a certificate that is signed by this CA. Make sure that the certificate does NOT contain the X509v3 Subject/Authority Identifer Key(s) extensions.
  3. After importing the certs, view the certificate either via the API or in the DS sslkeys page of the Traffic Portal. The certificate field should contain a valid certificate.

Check all that apply

  • This PR includes tests
  • This PR includes documentation updates
  • This PR includes an update to CHANGELOG.md
  • This PR includes all required license headers
  • This PR includes a database migration (ensure that migration sequence is correct)
  • This PR fixes a serious security flaw. Read more: www.apache.org/security

@asfgit
Copy link
Copy Markdown
Contributor

asfgit commented Jan 12, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3043/
Test PASSed.

@asfgit
Copy link
Copy Markdown
Contributor

asfgit commented Jan 12, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3044/
Test PASSed.

@rawlinp rawlinp added bug something isn't working as intended Traffic Ops related to Traffic Ops SSL support for/problems with SSL features labels Jan 14, 2019
@mitchell852 mitchell852 added the WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on) label Jan 16, 2019
@mitchell852 mitchell852 changed the title Remove mandatory x509v3 Subject/Authority Key Identifiers in certificate(s) (#3181) WIP: Remove mandatory x509v3 Subject/Authority Key Identifiers in certificate(s) (#3181) Jan 16, 2019
@jhg03a
Copy link
Copy Markdown
Contributor

jhg03a commented Jan 18, 2019

This PR works for me

@ghost
Copy link
Copy Markdown
Author

ghost commented Jan 18, 2019

I've reviewed the existing unit test for this endpoint and it doesn't need to change IMO. I've also tested the following X509 certificate import scenarios on a test instance and all have been successful:

  • X509v1 Single Self-Signed Certificate: CN only
  • X509v1 Root CA Self Signed + Server Certificate Signed by Root CA
  • X509v1 Root CA + Intermediate CA + Server Certificate Signed by Intermediate CA
  • X509v3 Root CA Self Signed + Server Certificate (Test Both with and w-out SKI/AKI extensions)
  • X509v3 Root CA Self Signed + Two intermediate CAs + Server Certificate (Test Both with and w-out SKI/AKI extensions)
  • Imporrted various real certificates from a real prod system. All verify okay.

IMO this PR is ready to be merged.

@mitchell852 mitchell852 removed the WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on) label Jan 18, 2019
@mitchell852 mitchell852 changed the title WIP: Remove mandatory x509v3 Subject/Authority Key Identifiers in certificate(s) (#3181) Remove mandatory x509v3 Subject/Authority Key Identifiers in certificate(s) (#3181) Jan 18, 2019
@dangogh dangogh self-assigned this Jan 18, 2019
@dangogh dangogh merged commit 3e44782 into apache:master Jan 18, 2019
mitchell852 pushed a commit that referenced this pull request Jan 23, 2019
…AKI (#3255)

Test has been updated to reflect that rootCA is included in certificate chain output.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug something isn't working as intended SSL support for/problems with SSL features Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TO API - DeliveryServices AddSSLKeys Incorrectly stores X509 certificate missing Subject/Authority Key as zero length string.

5 participants