Alon Bar-Lev has posted comments on this change.
Change subject: engine: Integrate noVNC support
......................................................................
Patch Set 7: (4 inline comments)
Thanks!
Few more...
What about the SIGCHLD issue we discussed? When do you receive SIGCHLD?
What about overriding the do_SIGINT and raising TerminateException?
The last major security issue is the ticket of novnc that we extract at client
side without validation, if we can do this at server side, then it would be the
best. If not, there is no point in adding it into the ticket.
Thanks!
....................................................
File backend/manager/modules/root/src/main/webapp/ovirt-engine-novnc-main.html
Line 128: 'shared':
WebUtil.getQueryVar('shared', true),
Line 129: 'view_only':
WebUtil.getQueryVar('view_only', false),
Line 130: 'updateState': updateState,
Line 131: 'onPasswordRequired': passwordRequired});
Line 132: rfb.connect(host, port, ticket, path);
passing the vnc otp here is not authenticated... can't we do this at server
side?
Line 133: }catch(e) {alert(e);}
Line 134: }
Line 135:
Line 136: if (window.addEventListener) {
....................................................
File packaging/services/ovirt-websocket-proxy.py
Line 147: listen_host=self._config.getString('PROXY_HOST'),
Line 148: listen_port=self._config.getString('PROXY_PORT'),
Line 149: source_is_ipv6=self._config.getBoolean('SOURCE_IS_IPV6'),
Line 150: verbose=self._config.getBoolean('LOG_VERBOSE'),
Line 151: ticketDecoder=TicketDecoder(insecure,
data_verification_cert),
:)
No need to alter that much for pep8... :)
ticketDecoder=TicketDecoder(
insecure=not self._config.getBoolean(
'FORCE_DATA_VERIFICATION'
),
certificate=self._config.getString(
'CERT_FOR_DATA_VERIFICATION'
),
),
cert=...
Line 152: cert=self._config.getString('SSL_CERTIFICATE'),
Line 153: key=self._config.getString('SSL_KEY'),
Line 154: ssl_only=self._config.getBoolean('SSL_ONLY'),
Line 155: daemon=False,
....................................................
File packaging/services/ovirt-websocket-proxy.systemd.in
Line 4: [Service]
Line 5: Type=simple
Line 6: User=@ENGINE_USER@
Line 7: Group=@ENGINE_GROUP@
Line 8: LimitNOFILE=2048
No...
You did not notice the change...
Should be:
LimitNOFILE=65535
LimitNPROC=2048
Line 9: ExecStart=@ENGINE_USR@/services/ovirt-websocket-proxy.py $EXTRA_ARGS
start
Line 10:
Line 11: [Install]
....................................................
File packaging/services/ovirt-websocket-proxy.sysv.in
Line 31: echo $"Insufficient privilege" 1>&2
Line 32: exit 4
Line 33: fi
Line 34: echo -n $"Starting $PROG: "
Line 35: ulimit -n ${FILENO:-2048}
No...
You did not notice the change...
Should be:
ulimit -n ${FILENO:-65535}
ulimit -u ${NPROC:-2048}
Line 36: touch "${PIDFILE}"
Line 37: chown "${USER}" "${PIDFILE}"
Line 38: daemon --user "${USER}" --pidfile="${PIDFILE}" \
Line 39:
"${ENGINE_USR}/services/ovirt-websocket-proxy.py" \
--
To view, visit http://gerrit.ovirt.org/13931
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I44e9870b88537360a1886e89c08f18865eae2ef0
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Martin Beták <mbe...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
