WIP: in depth tls certificate validations for routes #1824

pweil- · 2015-04-20T21:42:36Z

Part 2 of strengthening the router validations. This PR includes validation for

is the cert/key a valid pair
can the ca cert blocks be parsed, including if it is a concatenated cert chain
can the dest cert blocks be parsed, including if it is a concatenated cert chain
did the first ca cert parsed actually sign the cert/key pair

pweil- · 2015-04-20T21:42:47Z

smarterclayton · 2015-04-20T21:57:43Z

pkg/route/api/validation/validation.go

+	if err != nil {
+		msg := fmt.Sprintf("the certificate and key were not able to be parsed: %s", err.Error())
+		//no value set on this, it is a multi-field validation
+		result = append(result, fielderrors.NewFieldInvalid("certificate/key", "", msg))


json path - "certificate" or "key", not /

liggitt · 2015-04-24T14:10:00Z

pkg/route/api/validation/validation.go

+}
+
+// decodeNewlines is utility to remove the json formatted newlines from a cert
+func decodeNewlines(s string) string {


the fact that we need to do this makes me feel like we're missing something... newline to []byte and back should be transparent... we shouldn't need to fiddle with escaped newlines

this is a holdover from defining a cert inside the route's json. The newlines need escaped in the json and translate just fine when you write the file to disk. When you try to load the json value like I'm doing though...not so much. If there is a better way then we should definitely use it and get rid of this method.

newlines should be in the JSON strings as "\n"... are they not? if so, parsing the JSON should result in a string with newlines, not literal "\n" strings

smarterclayton · 2015-04-27T16:37:28Z

What's the status of this pull?

pweil- · 2015-04-27T16:43:00Z

in progress, just has been neglected in favor of security contexts and default certs. Will finish this week.

openshift-bot · 2015-04-27T20:30:11Z

Origin Action Required: Pull request cannot be automatically merged, please rebase your branch from latest HEAD and push again

smarterclayton · 2015-05-27T03:20:24Z

Post 3.0

smarterclayton · 2015-07-02T19:35:50Z

Needs rebase

danmcp · 2015-08-10T15:05:00Z

@pweil- Bump

pweil- · 2015-09-15T20:21:00Z

closing. The router will soon have a mechanism to report route status and will be able to specify reason codes such as an invalid certificate. This should move to the router.

karamfil · 2016-08-31T15:48:07Z

Hello has this been fixed? And in what version?

Thanks!

pweil- · 2016-08-31T16:56:08Z

Fixed in the latest 3.2.x release.

…service-catalog/' changes from c3e3071633..231772fcc0 231772fcc0 origin build: add origin tooling 98af588 v0.1.11 release changes 01e2f90 v0.1.10 release changes 49af948 clear polling queue before starting new operation (openshift#1855) 252958e Refactor common serviceclass validations (openshift#1858) 68f55c6 Catalog Controller should listen on https and serve metrics over TLS secured channel (openshift#1851) 5d0f773 Refactor common broker validations (openshift#1865) eeaf285 Add NamespacedServiceBroker switch to helm chart (openshift#1864) d2c960c Add NamespacedServiceBroker Feature (openshift#1863) ef15310 Fix NamespaceScoped doc text for ns types (openshift#1862) 8d0a637 fix async deprovision retry (openshift#1832) a918a16 Update registry code from serviceclass to clusterserviceclass (openshift#1852) 4dfd13c Bump dependency on go-open-service-broker-client to 0.0.6 (openshift#1856) 958b7cd Rename SharedServicePlanSpec to CommonServicePlanSpec (openshift#1850) 426aec3 pick a better random port to listen on in integration tests (openshift#1844) 6a59ada OrphanMitigation condition and different handling of retry timeout (openshift#1789) c9b8f60 Extracting common broker spec elements into embeddable struct (openshift#1841) 3f8fab6 Extract common service class spec fields (openshift#1834) 93dab13 Support for OSB [PR#452](openservicebrokerapi/servicebroker#452). (openshift#1849) 5e1e90d [WIP] Pass correct plan ID in deprovision request (for both deleting and orphan mitigation) (openshift#1847) 74f73c0 disable tests for deployment stage (openshift#1845) 82fc6e4 Revert "Pass correct plan ID in deprovision request (for both deleting and orphan mitigation) (openshift#1803)" (openshift#1843) 94b5795 gitignore integration.test binary (openshift#1840) 014c468 Extracting common plan spec into embeddable struct (openshift#1833) 5d7041b Use k8s NewUUID method exclusively (openshift#1836) eac3f96 A new test was added after prechecks happened for last pr. (openshift#1838) 4b5d159 Pass correct plan ID in deprovision request (for both deleting and orphan mitigation) (openshift#1803) cc02f0e [svcat] Adding a filter to get plan. (openshift#1758) 70afb56 reset RemovedFromBroker flag on plans that are re-added by broker (openshift#1824) 712dd4a Add behavior to print deleted instance name (openshift#1806) 55505be Update catalog charts README configuration (openshift#1823) 6426c98 Controller reconciliation rework - part 2 (ServiceBinding) (openshift#1819) c606560 Integrate svcat docs with Service Catalog's (openshift#1784) e9aeeb0 Synchronize some generated code that was missed along the way (openshift#1801) a63ebf7 Fix failing test: TestReconcileServiceInstanceWithFailedCondition (openshift#1813) 07ef743 Controller reconciliation rework - part 1 (ServiceInstance) (openshift#1779) a7d602b Export the touch instance command (openshift#1809) bddb9a7 Allow retries for instances with Failed condition after spec changes (openshift#1751) a777af5 Add enhanced parameter options to provision (openshift#1785) fd1a0b9 Print deleted bindings (openshift#1799) 36d437a Adding the ability to sync a service instance (openshift#1762) 1f60676 Remove Failed condition if there was no terminal failure (openshift#1788) cd831de allow brokers to respond to getCatalog() with no services (openshift#1772) (openshift#1781) e5c37ad Add ObservedGeneration and Provisioned into ServiceInstanceStatus (openshift#1748) 9021d8b Add carolynvs to OWNERS (openshift#1780) b7643d6 Add all-namespaces flag to svcat (openshift#1782) 01e652f Use docker to interact with files made by docker (openshift#1777) REVERT: c3e3071633 origin build: add origin tooling git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog git-subtree-split: 231772fcc00be08b6b2665a39c4a3bacb0b2271f

…service-catalog/' changes from c3e3071633..abec8bcc89 abec8bcc89 UPSTREAM: Only validate status on updates df5ab09c9a UPSTREAM: reset RemovedFromBroker flag on plans that are re-added by broker (openshift#1824) git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog git-subtree-split: abec8bcc8984d5612aa51aef445a4aaf7d910963

…service-catalog/' changes from c3e3071633..b65141ce43 b65141ce43 reset RemovedFromBroker flag on plans that are re-added by broker (openshift#1824) git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog git-subtree-split: b65141ce4380126d83d2968f3f4d02a3af958601

pweil- mentioned this pull request Apr 20, 2015

HAProxy routes stop working when invalid cert/key is uploaded for any of route #1706

Closed

smarterclayton reviewed Apr 20, 2015
View reviewed changes

in depth tls certificate validations for routes

1c90733

pweil- force-pushed the tls-validation branch from 4a6cb40 to 1c90733 Compare April 21, 2015 14:53

pweil- changed the title ~~in depth tls certificate validations for routes~~ WIP: in depth tls certificate validations for routes Apr 21, 2015

liggitt reviewed Apr 24, 2015
View reviewed changes

smarterclayton modified the milestone: 1.1.0 May 27, 2015

pweil- closed this Sep 15, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: in depth tls certificate validations for routes #1824

WIP: in depth tls certificate validations for routes #1824

pweil- commented Apr 20, 2015

pweil- commented Apr 20, 2015

smarterclayton Apr 20, 2015

liggitt Apr 24, 2015

pweil- Apr 24, 2015

liggitt Apr 24, 2015

smarterclayton commented Apr 27, 2015

pweil- commented Apr 27, 2015

openshift-bot commented Apr 27, 2015

smarterclayton commented May 27, 2015

smarterclayton commented Jul 2, 2015

danmcp commented Aug 10, 2015

pweil- commented Sep 15, 2015

karamfil commented Aug 31, 2016

pweil- commented Aug 31, 2016

WIP: in depth tls certificate validations for routes #1824

WIP: in depth tls certificate validations for routes #1824

Conversation

pweil- commented Apr 20, 2015

pweil- commented Apr 20, 2015

smarterclayton Apr 20, 2015

Choose a reason for hiding this comment

liggitt Apr 24, 2015

Choose a reason for hiding this comment

pweil- Apr 24, 2015

Choose a reason for hiding this comment

liggitt Apr 24, 2015

Choose a reason for hiding this comment

smarterclayton commented Apr 27, 2015

pweil- commented Apr 27, 2015

openshift-bot commented Apr 27, 2015

smarterclayton commented May 27, 2015

smarterclayton commented Jul 2, 2015

danmcp commented Aug 10, 2015

pweil- commented Sep 15, 2015

karamfil commented Aug 31, 2016

pweil- commented Aug 31, 2016