Bug 1286696

My understanding is that instead of a jsessionid cookie, we would this HTTP header instead:
Authorization: Bearer <token>

Hi, I have an oVirt patch [1] currently blocked by this, any estimates or progress on this?

[1] https://gerrit.ovirt.org/#/c/49278/

Is oVirt already adding the needed SSO token through the .vv file (similarly to what is done for the currently used jsessionid field)?
Can you confirm that the way to use this token is by adding a 'Authorization: Bearer <token>' header to the HTTP requests?

(In reply to Christophe Fergeau from comment #3)
> Is oVirt already adding the needed SSO token through the .vv file (similarly
> to what is done for the currently used jsessionid field)?

The patch from comment #2 makes it possible. I can modify this patch to embed the SSO token into .vv file, effectively replacing jsessionid (in scope of oVirt 4.0 and above):

  sso-token=...

Is the above change OK?

@Martin, do we want virt-viewer to keep supporting legacy jsessionid in .vv file for oVirt < 4.0? (If so, sso-token should always take priority over jsessionid.)

> Can you confirm that the way to use this token is by adding a
> 'Authorization: Bearer <token>' header to the HTTP requests?

That is correct.

Just to clarify, in oVirt 4.0 and above, we'd like the UI to stop using oVirt REST API persistent session mechanism (based on JSESSIONID cookie) which led us to problems in the past:

  http://www.ovirt.org/Features/RESTSessionManagement

Instead of acquiring oVirt REST API session on behalf of other applications (such as UI plugins and virt-viewer), we'll simply provide the SSO token that can be used to authenticate HTTP requests via 'Authorization: Bearer xxx' header.

(In reply to vszocs from comment #4)
> (In reply to Christophe Fergeau from comment #3)
> > Is oVirt already adding the needed SSO token through the .vv file (similarly
> > to what is done for the currently used jsessionid field)?
> 
> The patch from comment #2 makes it possible. I can modify this patch to
> embed the SSO token into .vv file, effectively replacing jsessionid (in
> scope of oVirt 4.0 and above):
> 
>   sso-token=...
> 
> Is the above change OK?
> 

Sure, though I hope it's the last one for a long time ;) Something (ugly) that I wondered about while adding support for this to virt-viewer/libgovirt.
What if you keep setting a field named 'jsessionid' in the .vv file (with the value to use for the Authorization header), and if we set both a jsessionid cookie and an Authorization header on our REST requests. Is this going to work as expected? (ie oVirt 3.6 is going to make use of the cookie and ignore the Authorization header, and oVirt 4.0 is going to do the opposite).

This is not what I have implemented, but I've been curious about this evilness ;)

I think I'm done with the clientside changes, but how do we test all of this now? Is there a test instance with a test VM I can connect to? Actually, you just provide me with a .vv file for an instance with a long-lived ticket, and this should be good enough for testing.

I can also prepare some libgovirt/virt-viewer scratch build if you want to test that from your side Let me know which distro you use if you want scratch builds.

(In reply to Christophe Fergeau from comment #6)
> (In reply to vszocs from comment #4)
> > (In reply to Christophe Fergeau from comment #3)
> > > Is oVirt already adding the needed SSO token through the .vv file (similarly
> > > to what is done for the currently used jsessionid field)?
> > 
> > The patch from comment #2 makes it possible. I can modify this patch to
> > embed the SSO token into .vv file, effectively replacing jsessionid (in
> > scope of oVirt 4.0 and above):
> > 
> >   sso-token=...
> > 
> > Is the above change OK?
> > 
> 
> Sure, though I hope it's the last one for a long time ;) Something (ugly)
> that I wondered about while adding support for this to virt-viewer/libgovirt.
> What if you keep setting a field named 'jsessionid' in the .vv file (with
> the value to use for the Authorization header), and if we set both a
> jsessionid cookie and an Authorization header on our REST requests. Is this
> going to work as expected? (ie oVirt 3.6 is going to make use of the cookie
> and ignore the Authorization header, and oVirt 4.0 is going to do the
> opposite).

In context of oVirt REST API webapp:

- `Authorization` header is used to authenticate incoming HTTP request, supporting basic-auth (`Basic <base64-string>`) and token-based auth (`Bearer <token>`)

- `JSESSIONID` cookie is used to map incoming HTTP request to REST API's HttpSession - effectively utilize Java server-side session mechanism specifically for this webapp (see link in comment #5 for details)

- `Authorization` header is processed **before** processing the HttpSession created from `JSESSIONID` cookie [1]

  [1] backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml
      SSORestApiAuthFilter runs before RestApiSessionMgmtFilter

This is simply because the `JSESSIONID` cookie does not authenticate a request, it's just a way to correlate it to (existing) server-side session. The `Authorization` header does authenticate the request. When processing HTTP request, authentication happens before session management.

> 
> This is not what I have implemented, but I've been curious about this
> evilness ;)

Mixing the two entities (cookie vs. object) is not a good idea [2].

  [2] EnforceAuthFilter ensures that failed auth (wrong `Authorization` value) always returns HTTP 401

Always use the correct value for given entity:

- if .vv file contains `jsessionid` then apply `JSESSIONID` cookie to the request (as today)

- if .vv file contains `sso-token` then apply `Authorization` header to the request (as proposed)

> 
> I think I'm done with the clientside changes, but how do we test all of this
> now? Is there a test instance with a test VM I can connect to? Actually, you
> just provide me with a .vv file for an instance with a long-lived ticket,
> and this should be good enough for testing.
> 
> I can also prepare some libgovirt/virt-viewer scratch build if you want to
> test that from your side Let me know which distro you use if you want
> scratch builds.

Testing from oVirt side is probably better; once we have the patch from comment #2 merged (and `sso-token` embedded into .vv file), we can test this locally on oVirt devel environment.

Forgot to mention - thanks Christophe for your quick response and for making the necessary changes, can you please provide virt-viewer scratch build for Fedora so we can test this? (I'll try to coordinate with Martin Betak)

Which fedora version? Fedora 22/23/rawhide?

(In reply to Christophe Fergeau from comment #9)
> Which fedora version? Fedora 22/23/rawhide?

F22 and F23 please, thanks :-)

Fedora 23:

libgovirt-0.3.3-2.fc23.1.teuf http://koji.fedoraproject.org/koji/taskinfo?taskID=12676761
virt-viewer-3.0-1.fc23.1.teuf http://koji.fedoraproject.org/koji/taskinfo?taskID=12676743


Fedora 22:

libgovirt-0.3.3-1.fc22.1.teuf http://koji.fedoraproject.org/koji/taskinfo?taskID=12676757
virt-viewer-3.0-1.fc22.1.teuf http://koji.fedoraproject.org/koji/taskinfo?taskID=12676773

Thanks, Christophe!

I forgot to mention that running this with 'REST_DEBUG=proxy remote-viewer ~/Downloads/console.vv' will dump the REST communication to the terminal.

(In reply to Christophe Fergeau from comment #13)
> I forgot to mention that running this with 'REST_DEBUG=proxy remote-viewer
> ~/Downloads/console.vv' will dump the REST communication to the terminal.

Thanks, this is very useful info.

oVirt Engine / UI changes are finalized and a valid SSO token is embedded into console.vv file.

Proceeding to test virt-viewer with `REST_DEBUG=proxy` option.

> oVirt Engine / UI changes are finalized and a valid SSO token is embedded
> into console.vv file.

Found an oVirt UI bug which caused `admin=0` to be passed into console.vv file, regardless if the user is an admin or not.

This bug is tracked via https://bugzilla.redhat.com/show_bug.cgi?id=1304755

> Proceeding to test virt-viewer with `REST_DEBUG=proxy` option.

Found an issue with virt-viewer: when using SSO token, `Prefer: persistent-auth` is still sent, which causes creation of oVirt REST webapp session in consequence (which is not what we want here).

The behavior of virt-viewer should be following:

If `sso-token` is present:
- set `Authorization: Bearer <token>` header --> authenticate request

Else if `jsessionid` is present:
- set `JSESSIONID` cookie --> associate request with oVirt REST webapp session
- set `Prefer: persistent-auth` header --> prevent closing oVirt REST webapp session after the request is processed [*]
- set `JSESSIONID` *header* with value taken from `jsessionid` --> this is the CSRF token for oVirt REST webapp session

[*] this session is used by all oVirt UI plugins

(In reply to vszocs from comment #16)
> > Proceeding to test virt-viewer with `REST_DEBUG=proxy` option.
> 
> Found an issue with virt-viewer: when using SSO token, `Prefer:
> persistent-auth` is still sent, which causes creation of oVirt REST webapp
> session in consequence (which is not what we want here).
> 
> The behavior of virt-viewer should be following:
> 
> If `sso-token` is present:
> - set `Authorization: Bearer <token>` header --> authenticate request
> 
> Else if `jsessionid` is present:
> - set `JSESSIONID` cookie --> associate request with oVirt REST webapp
> session
> - set `Prefer: persistent-auth` header --> prevent closing oVirt REST webapp
> session after the request is processed [*]
> - set `JSESSIONID` *header* with value taken from `jsessionid` --> this is
> the CSRF token for oVirt REST webapp session
> 

I don't think we have ever set the JSESSIONID header, but just the cookie. A bit reluctant tochange that now as this is being phased out anyway.

I've put new scratch build at
http://koji.fedoraproject.org/koji/taskinfo?taskID=12902880 (f23)
http://koji.fedoraproject.org/koji/taskinfo?taskID=12902936 (f22)
which should fix the persistent-auth issue.

(In reply to Christophe Fergeau from comment #17)
> I don't think we have ever set the JSESSIONID header, but just the cookie. A
> bit reluctant tochange that now as this is being phased out anyway.

Up until now (oVirt 3.6 including), all UI plugins and virt-viewer (launched via UI-generated console.vv) shared the same oVirt REST session.

In oVirt 3.5 [a] and 3.6 [b], oVirt UI code was changed so that the above REST session is created as CSRF-protected. This effectively means that clients using this session must send "JSESSIONID" header with value equal to "JSESSIONID" cookie value.

  [a] https://gerrit.ovirt.org/#/c/29850/
  [b] https://gerrit.ovirt.org/#/c/29682/

This was also announced on oVirt devel list:

  http://lists.ovirt.org/pipermail/devel/2014-July/008148.html

If the virt-viewer does not send "JSESSIONID" header (in addition to the cookie) then it should, in theory, receive HTTP 403 from oVirt REST service. Are we absolutely sure that not sending "JSESSIONID" header works with virt-viewer + oVirt 3.5 & 3.6?

(Personally, I'd advise to adapt virt-viewer and include this header, just to be on safe side.)

> I've put new scratch build at
> http://koji.fedoraproject.org/koji/taskinfo?taskID=12902880 (f23)
> http://koji.fedoraproject.org/koji/taskinfo?taskID=12902936 (f22)
> which should fix the persistent-auth issue.

Thank you, Christophe. I'll try it out.

(In reply to vszocs from comment #16)
> Found an oVirt UI bug which caused `admin=0` to be passed into console.vv
> file, regardless if the user is an admin or not.
> 
> This bug is tracked via https://bugzilla.redhat.com/show_bug.cgi?id=1304755

Fix landed in both oVirt master + 3.6 branch, need to finalize testing virt-viewer and we should be good to go.

(In reply to vszocs from comment #19
> Fix landed in both oVirt master + 3.6 branch, need to finalize testing
> virt-viewer and we should be good to go.

Any news on the testing?

(In reply to Christophe Fergeau from comment #20)
> Any news on the testing?

Sorry, I've been busy with some other things.

Action items related to oVirt UI using SSO token (and passing it to virt-viewer, among other things) are listed here:

  https://bugzilla.redhat.com/show_bug.cgi?id=1236976#c5

Currently testing virt-viewer with latest oVirt UI changes, I'll keep you updated.

Ok, thanks for the update. As long as you comment here once you have done the testing, feel free to do it on your own schedule. The reason I was asking for a status is that I'm not going to move forward with adding this upstream/in RHEL until I get confirmation that these changes are working as expected.

Hi Christophe, since you only rebuilt libgovirt [1] (0.3.3-2), I assume that virt-viewer stays the same [2] (3.0-1).

[1] http://koji.fedoraproject.org/koji/taskinfo?taskID=12902936
[2] http://koji.fedoraproject.org/koji/taskinfo?taskID=12676773

In Koji, going to specific buildArch doesn't show any RPMs to download, info for buildroot says "repo state deleted".

Can you please rebuild / provide link to libgovirt RPM [1] above? (Sorry if I'm missing something.)

Scratch builds are deleted after a while, better to save them locally :) I've repushed new ones:

Fedora 23
libgovirt-0.3.3-2.fc23.2.teuf http://koji.fedoraproject.org/koji/taskinfo?taskID=13425748
virt-viewer-3.0-1.fc23.1.teuf http://koji.fedoraproject.org/koji/taskinfo?taskID=13425840


Fedora 22
libgovirt-0.3.3-2.fc22.2.teuf http://koji.fedoraproject.org/koji/taskinfo?taskID=13425813
virt-viewer-3.0-1.fc22.1.teuf http://koji.fedoraproject.org/koji/taskinfo?taskID=13425831

(In reply to Christophe Fergeau from comment #24)
> Scratch builds are deleted after a while, better to save them locally :)
> I've repushed new ones:
> 
> Fedora 23
> libgovirt-0.3.3-2.fc23.2.teuf
> http://koji.fedoraproject.org/koji/taskinfo?taskID=13425748
> virt-viewer-3.0-1.fc23.1.teuf
> http://koji.fedoraproject.org/koji/taskinfo?taskID=13425840
> 
> 
> Fedora 22
> libgovirt-0.3.3-2.fc22.2.teuf
> http://koji.fedoraproject.org/koji/taskinfo?taskID=13425813
> virt-viewer-3.0-1.fc22.1.teuf
> http://koji.fedoraproject.org/koji/taskinfo?taskID=13425831

Verified that oVirt UI SSO-related change:

  https://gerrit.ovirt.org/#/c/49278/

works with above build of libgovirt + virt-viewer, thanks Christophe!

@Christophe, since the libgovirt/virt-viewer changes are now verified, please let me know once they land in upstream/RHEL and I'll proceed with merging the relevant oVirt UI patch. Thanks!

@Christophe, I see the bug is still in NEW state, any updates regarding the changes landing in Fedora/RHEL?

(In reply to vszocs from comment #32)
> @Christophe, I see the bug is still in NEW state, any updates regarding the
> changes landing in Fedora/RHEL?

This will get added to a RHEL 7.3 build at some point. Regarding fedora, I'd prefer to have an upstream libgovirt release first, but certificate handling is currently broken in git master :(

Patches sent upstream
https://www.redhat.com/archives/virt-tools-list/2016-May/msg00047.html

I verified this bug between old/new remote-viewer and old/new ovirt server. The function of remote-viewer works well. 

1. Connect to vm in ovirt 3.6 with virt-viewer-2.0-8.el7.x86_64.
   1.1) $ remote-viewer --ovirt-ca-file=ca_36 ovirt://rhevm3.6-hp-dl360eg8-03/xiaodwan-test
        OK
   1.2) $ remote-viewer console.vv
        OK
2. Connect to vm in ovirt 3.6 with virt-viewer-2.0-7.el7.x86_64.
   No need testing

3. Connect to vm in ovirt 4.0 with virt-viewer-2.0-8.el7.x86_64.
   3.1) $ remote-viewer --ovirt-ca-file=ca_36 ovirt://rhevm3.6-hp-dl360eg8-03/xiaodwan-test
       FAIL 
       Bug 1346215 - Wrong VM state get for vms in ovirt4.0 
   3.2) $ remote-viewer console.vv
       OK   

4. Connect to vm in ovirt 4.0 with virt-viewer-2.0-7.el7.x86_64.   
   4.1) $ remote-viewer --ovirt-ca-file=ca_36 ovirt://rhevm3.6-hp-dl360eg8-03/xiaodwan-test
       FAIL 
       Bug 1346215 - Wrong VM state get for vms in ovirt4.0 
   4.2) $ remote-viewer console.vv
       OK
       No "Change CD" menu display, this result is as expected.

For bug 1346215, it's not a problem of virt-viewer. 

So move this bug from ON_QA to VERIFIED.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2016-2229.html