search

SUSE / velum

Dashboard for CaaS Platform clusters (v1, v2 and v3)
https://www.suse.com/
Apache License 2.0
54 stars 30 forks source link

fix CVE-2018-1000539 for velum (bsc#1099243) (json-jwt) #617

Closed jordimassaguerpla closed 5 years ago

jordimassaguerpla commented 6 years ago

Signed-off-by: Jordi Massaguer Pla jmassaguerpla@suse.de

jordimassaguerpla commented 6 years ago

@bear454 : Hi, updating sprockets makes this test fail, that seems related to GCE. Would you mind taking a look?

Failures: 1) InternalApi::V1::PillarsController when in GCE framework has cloud configuration Failure/Error: expect(json).to eq(expected_response) expected: {:registries=>[], :system_certificates=>[], :kubelet=>{:"compute-resources"=>{}, :"eviction-hard"=>""...uster_node=>{:size=>"custom-instance-type", :network=>"gcenetwork", :subnetwork=>"gcesubnetwork"}}}} got: {:cloud=>{:framework=>"gce", :profiles=>{:cluster_node=>{:size=>"custom-instance-type", :network=>"gc...{:"compute-resources"=>{}, :"eviction-hard"=>""}, :system_certificates=>[], :dex=>{:connectors=>[]}} (compared using ==) Diff: @@ -1,4 +1,5 @@ :cloud => {:framework=>"gce", :profiles=>{:cluster_node=>{:size=>"custom-instance-type", :network=>"gcenetwork", :subnetwork=>"gcesubnetwork"}}}, +:dex => {:connectors=>[]}, :kubelet => {:"compute-resources"=>{}, :"eviction-hard"=>""}, :registries => [], :system_certificates => [],

./spec/controllers/internal_api/v1/pillars_controller_spec.rb:351:in `block (3 levels) in <top (required)>'

vitoravelino commented 6 years ago

595 wasn't rebased before merging it. Just need to add https://github.com/kubic-project/velum/pull/613/files#diff-7383844f0b247b97f7dc5cf5ade168e6R313

jordimassaguerpla commented 6 years ago

@vitoravelino : Sorry I don't understand. The commit you mention is from an open PR. Should this PR depen on yours?

vitoravelino commented 6 years ago

No, just saying that the issue is the missing piece from the other PR because the GCE PR wasn't rebased before merging it.

jordimassaguerpla commented 6 years ago

Should we create a PR that only fixes the tests? I don't feel comfortable approving this if tests fail...

vitoravelino commented 6 years ago

Should we create a PR that only fixes the tests?

I think so. It's an option.

jordimassaguerpla commented 6 years ago

Let me try it

jordimassaguerpla commented 6 years ago

https://github.com/kubic-project/velum/pull/619

jordimassaguerpla commented 6 years ago

rebased from master. Triggering test runs

jordimassaguerpla commented 6 years ago

@vitoravelino : any idea why the tests fail?

ereslibre commented 6 years ago

@jordimassaguerpla From the error raised by jenkins it looks like this time it's really related to this change... JSON::JWT::InvalidFormat in OidcController#done Invalid JSON Format Extracted source (around line #52): 50 51 52 53 54 55 def decode_id(id_token) OpenIDConnect::ResponseObject::IdToken.decode(id_token, oidc_config.jwks) end def done Rails.root: /srv/velum Application Trace | Framework Trace | Full Trace app/controllers/oidc_controller.rb:52:indecode_id' app/controllers/oidc_controller.rb:60:in done' Request Parameters: {\"code\"=>\"oiz4as23z2oxgnlqsw7ldersa\", \"state\"=>\"85bf46530e79701800143476e767f1a3\"} Toggle session dump Toggle env dump Response Headers: None...

vitoravelino commented 6 years ago

Since it's a minor level upgrade, there was probably a breaking change that now conflicts with the json created from openid_connect gem.

Our current openid_connect gem is 1.1.3 but the current one is 1.1.6. I saw two commits related to json but the code and commit message didn't mention much about why that. Maybe we should try to upgrade this too?

Both gems had new releases on the same date, so...

https://rubygems.org/gems/json-jwt https://rubygems.org/gems/openid_connect

jordimassaguerpla commented 6 years ago

@vitoravelino : Updating openid_connect didn't fix the tests.

ereslibre commented 5 years ago

What's the status of this PR? Should we close it?

jordimassaguerpla commented 5 years ago

@ereslibre : we should fix it. Sorry I don't have much time to look at why the tests fail and how to fix that.

MalloZup commented 5 years ago

This GitHub PR is unactive since 30. Is this GitHub PR still needed? Please close or update it accordingly. This reminder is autogenerated by https://github.com/MalloZup/blacktango