Re: [Freeipa-devel] [PATCH] - Add DRM to IPA
On 08/21/2014 10:53 PM, Ade Lee wrote: On Thu, 2014-08-21 at 21:52 +0200, Martin Kosek wrote: On 08/21/2014 05:27 PM, Petr Viktorin wrote: On 08/21/2014 03:48 PM, Ade Lee wrote: As agreed on #irc, disabling uninstallation for now. Please apply this new patch on top of the big one. I'm fine with pushing a patch with incomplete functionality, after all I did this all the time with permissions. The incomplete parts (apart from the plugin which is entirely out of scope) are: - The agent PEM issue (will be sorted out as the plugin is implemented) https://fedorahosted.org/freeipa/ticket/4503 - Missing man page (will be written before the plugin is implemented) https://fedorahosted.org/freeipa/ticket/4504 - Uninstall (will be fixed on Dogtag side, re-tested and enabled) https://fedorahosted.org/freeipa/ticket/4505 I'll open tickets for these before pushing. ACK from me if Rob agrees. On IRC, Rob said he'd rather delay pushing until the man page is written, but delegated the decision to Martin. So, Martin, can we push now and trust Ade's promise that he'll write the docs? Yes, if you open ticket(s) for all missing parts and put them in the same milestone. I would rather have the patches in than waiting for man page and then have a conflict and postpone the patch set. I trust Ade to provide the man page later, I am sure he does not want to meet with my whip otherwise :-) Perfect, thanks! I don't have commit rights, so please commit the patches for me. Pushed to master: a25fe00c62117cb11a1e75fbcc4960a0cfa72aab -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 720-729 OTP usability improvements
On 12.8.2014 17:59, Endi Sukma Dewata wrote:
On 8/5/2014 6:31 AM, Petr Vobornik wrote:
ticket: https://fedorahosted.org/freeipa/ticket/4402
snip (ACK of 720, 721) but patch 720 was replaced by a new version
[PATCH] 722 webui: add token from user page
Add 'Add OTP Token' action to user action menu.
This option is disabled in self-service when viewing other users.
ACK, but I'm just wondering if it would be more intuitive to define an
enabled condition (you're an admin or editing your own user) rather than
a disabled condition (you're in self-service mode but editing other user).
probably
snip (ACK of 723)
[PATCH] 724 webui: display fields based on otp token type
- in adder dialog
is it an ACK?
snip (ACK of 725, 726)
[PATCH] 727 webui: hide empty fields and sections
Hide widgets without a value. Must be explicitly turned on. In widget by
`hidden_if_empty` flag. Or globally by `hide_empty_widgets` flag. Global
hiding can be individually turned off by `ignore_empty_hiding` flag.
In item #2 of ticket #4402 I was originally thinking the widget
visibility would be determined by the token type.
Originally I wrote it that way but with this patch it became redundant.
Any plan to add the
token type field in the future?
maybe, I don't have any strong feelings about it. Will users benefit
from adding it? If yes, it should be also added to CLI.
Atm. token type is derived from object classes. It exists only in add
operation as a virtual attribute. We can check a presence of
ipatokenhotp or ipatokentotp object classes to simulate the field.
Will the counter field strictly have a
value with HOTP only and clock offset interval fields strictly have
value with TOTP only? Do these fields contain the configured values or
the effective values? I was just thinking maybe in the future some of
these fields might be configured empty and they will use the default
values instead. If that's not a problem then ACK.
Respective fields are present only in corresponding object classes -
there won't be an HTOP token with 'clock offset'. If they are present,
they have a default value. - No false hiding - Shouldn't be a problem.
Btw: Other of my other original intents was to hide it based on ACI
rights. The issues is that the rights are not present without
corresponding OC. Hiding in such case is dangerous - explicitly disabled
in patch 728.
What do you think about setting `hide_empty_widgets` global setting to
True?
[PATCH] 728 webui: hide non-readable fields
hide widgets if associated field had received attribute level rights
without 'r' right.
Explicit rights are required to avoid hiding of special widgets which
are not associated with any LDAP attribute.
ACK.
[PATCH] 729 webui: hide otp fields based on token type
- uses hide empty feature
Assuming patch #727 is fine then it's ACKed too.
Some other comments/questions:
1. The Validity start/end fields don't show the date/time format. When
you type the first letter then it will show the validation message with
the format. It's not a big deal, but it's not very intuitive because
people might not know what letter to type in the first place. Usually
the field label should indicate the format/unit and the validation
message will only appear if the value entered doesn't match the
format/unit.
+1
We could set one of supported format as a placeholder + later is should
be transformed into proper datetime select.
Also some of our validators could benefit from more intelligent logic,
e.g. don't show error while typing if the entered value is a beginning
of a correct form
2. The Digits field by itself sounds a bit weird. How about Number of
digits, or OTP length and add the unit in the value (e.g. 6 digits)?
Both UIs should use the same terminology if possible. I like the OTP
length example but it doesn't work for CLI much.
Number of digits sounds reasonable. Should be changed in
ipalib.otptoken though.
3. The Clock offset field doesn't have a unit.
Fixed in patch 720-1, patch 729 was rebased
4. If no Owner is specified when adding a token, it will default to
admin. Is this the intended behavior?
Sadly yes.
--
Petr Vobornik
From 63e940a0f2858114fb311a9154d8ae04b11048a0 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 24 Jul 2014 13:11:35 +0200
Subject: [PATCH] webui: add measurement unit to otp token time fields
https://fedorahosted.org/freeipa/ticket/4402
---
install/ui/src/freeipa/otptoken.js | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/install/ui/src/freeipa/otptoken.js b/install/ui/src/freeipa/otptoken.js
index 56c7684a0925488053508b9b364969ef9557d1bb..c9570a697b3fc077a1d0c00ef91c0abaf92f75b6 100644
--- a/install/ui/src/freeipa/otptoken.js
+++ b/install/ui/src/freeipa/otptoken.js
@@ -198,8 +198,14 @@ return {
'ipatokenserial',
'ipatokenotpalgorithm',
[Freeipa-devel] [PATCH] 743 webui: do not show login error when switching back from otp sync screen
Errors should reflect only a result of last operation.
https://fedorahosted.org/freeipa/ticket/4470
Fixes issue found by Endi:
Try logging in with an incorrect password/OTP. After you get a login
error click Sync OTP Token. Once the sync is completed it will go
back to the login page with a Token was synchronized message that
disappears in a few seconds, but the old login error still appears
which is confusing. Error messages in the UI should only reflect the
last executed operation.
--
Petr Vobornik
From 789a8a2fd5ad42510e5bc08251f877d4f7445924 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 22 Aug 2014 13:47:11 +0200
Subject: [PATCH] webui: do not show login error when switching back from otp
sync screen
Errors should reflect only a result of last operation.
https://fedorahosted.org/freeipa/ticket/4470
---
install/ui/src/freeipa/widget.js | 27 +++
install/ui/src/freeipa/widgets/LoginScreen.js | 1 +
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index 4bdf273dbb9cd8e379aa7a28802e5b32e0bc7762..84afac0216e943663c871b2994cd832fc99eb1b7 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -5677,7 +5677,8 @@ exp.alert_helper = IPA.alert_helper = {
name: name,
text: text,
cls: 'alert alert-danger',
-icon: 'fa fa-exclamation-circle'
+icon: 'fa fa-exclamation-circle',
+type: 'error'
};
},
@@ -5692,7 +5693,8 @@ exp.alert_helper = IPA.alert_helper = {
name: name,
text: text,
cls: 'alert alert-warning',
-icon: 'fa fa-warning'
+icon: 'fa fa-warning',
+type: 'warning'
};
},
@@ -5707,7 +5709,8 @@ exp.alert_helper = IPA.alert_helper = {
name: name,
text: text,
cls: 'alert alert-info',
-icon: 'fa fa-info-circle'
+icon: 'fa fa-info-circle',
+type: 'info'
};
},
@@ -5722,7 +5725,8 @@ exp.alert_helper = IPA.alert_helper = {
name: name,
text: text,
cls: 'alert alert-success',
-icon: 'fa fa-check-circle-o'
+icon: 'fa fa-check-circle-o',
+type: 'success'
};
},
@@ -5818,6 +5822,21 @@ exp.validation_summary_widget = IPA.validation_summary_widget = function(spec) {
that.render_items();
};
+that.remove_all = function(type) {
+
+if (!type) that.items.empty();
+
+for (var i=0, l=that.items.length; il; i++) {
+var alert = that.items.get_value_by_index(i);
+if (alert.type !== type) continue;
+that.items.remove(alert.name);
+i--;
+l--;
+}
+
+that.render_items();
+};
+
return that;
};
diff --git a/install/ui/src/freeipa/widgets/LoginScreen.js b/install/ui/src/freeipa/widgets/LoginScreen.js
index 2c5002016f9f70802c52aa905c71a48eaf76ce7f..fb7c6c34d9c1c7115dd95809f3a39de488eb 100644
--- a/install/ui/src/freeipa/widgets/LoginScreen.js
+++ b/install/ui/src/freeipa/widgets/LoginScreen.js
@@ -170,6 +170,7 @@ define(['dojo/_base/declare',
on_sync: function() {
var user = this.get_field('username').get_value()[0];
+this.get_widget('validation').remove_all('error');
this.emit('require-otp-sync', { source: this, user: user });
},
--
1.9.3
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] Notice: vakwetu/dogtag COPR repo required in order to run master on F20
For those of you who haven't read the Add DRM to IPA thread: you have to use Dogtag 10.2 which is only available in COPR repo. Otherwise httpd won't start because of missing pki.crypto. Could be done simply by: # dnf copr enable vakwetu/dogtag Might save you some time... -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] - Add DRM to IPA
On 22.8.2014 10:03, Petr Viktorin wrote: On 08/21/2014 10:53 PM, Ade Lee wrote: On Thu, 2014-08-21 at 21:52 +0200, Martin Kosek wrote: On 08/21/2014 05:27 PM, Petr Viktorin wrote: On 08/21/2014 03:48 PM, Ade Lee wrote: As agreed on #irc, disabling uninstallation for now. Please apply this new patch on top of the big one. I'm fine with pushing a patch with incomplete functionality, after all I did this all the time with permissions. The incomplete parts (apart from the plugin which is entirely out of scope) are: - The agent PEM issue (will be sorted out as the plugin is implemented) https://fedorahosted.org/freeipa/ticket/4503 - Missing man page (will be written before the plugin is implemented) https://fedorahosted.org/freeipa/ticket/4504 - Uninstall (will be fixed on Dogtag side, re-tested and enabled) https://fedorahosted.org/freeipa/ticket/4505 I'll open tickets for these before pushing. ACK from me if Rob agrees. On IRC, Rob said he'd rather delay pushing until the man page is written, but delegated the decision to Martin. So, Martin, can we push now and trust Ade's promise that he'll write the docs? Yes, if you open ticket(s) for all missing parts and put them in the same milestone. I would rather have the patches in than waiting for man page and then have a conflict and postpone the patch set. I trust Ade to provide the man page later, I am sure he does not want to meet with my whip otherwise :-) Perfect, thanks! I don't have commit rights, so please commit the patches for me. Pushed to master: a25fe00c62117cb11a1e75fbcc4960a0cfa72aab Should the requirement of Dogtag 10.2 be reflected in a spec file? -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 736-740 webui: various minor fixes
On 8/21/2014 7:18 AM, Simo Sorce wrote: On Thu, 2014-08-21 at 14:11 +0200, Petr Vobornik wrote: On 13.8.2014 17:20, Endi Sukma Dewata wrote: 2. Can the UI parse the new key and display it the same way as other keys that are already saved? That will make it more seamless. Would be nice, but is it worth the effort? We would have to reimplement ipapython.ssh into JavaScript + pull in crypto.js or other lib for sha1and sha256 functions since Web Cryptography API is still only a draft [1]. I do not do this lightly, but you have my veto to do any crypto in javascript unless you convince me first it make sense. Simo. Just to clarify, the point is to display some kind of information about the public keys the user just entered in the UI (that have not been saved) so that: a) If user enters multiple keys at once, the user can distinguish which keys have been added rather than displaying a generic New: key set for all new keys. b) The UI can detect if the key already exists on the server. If this can be done without any cryptographic operations that would be great, but I'm not sure about the details. For (a) I was wondering if the UI can decode the base-64 encoded public key entered by the user and display some user-friendly information about the key itself, maybe just the hexdump of the first few bytes of the key, or maybe the key type too if possible, or at least just the first few chars of the undecoded key. For (b) the UI would have to use some crypto functions to generate the key fingerprints as generated by the server. But even if we do that, we are not doing any data encryption or dealing with secret information. Alternatively, instead of displaying the fingerprints of the saved keys, the UI can display the first few bytes/chars of hexdump/encoded key to match (a) such that they can be compared without cryptography. BTW, WebCrypto implementation has been making its way into Firefox: https://docs.google.com/spreadsheet/ccc?key=0AiAcidBZRLxndE9LWEs2R1oxZ0xidUVoU3FQbFFobkEusp=sharing -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 736-740 webui: various minor fixes
On Fri, 2014-08-22 at 09:52 -0500, Endi Sukma Dewata wrote: On 8/21/2014 7:18 AM, Simo Sorce wrote: On Thu, 2014-08-21 at 14:11 +0200, Petr Vobornik wrote: On 13.8.2014 17:20, Endi Sukma Dewata wrote: 2. Can the UI parse the new key and display it the same way as other keys that are already saved? That will make it more seamless. Would be nice, but is it worth the effort? We would have to reimplement ipapython.ssh into JavaScript + pull in crypto.js or other lib for sha1and sha256 functions since Web Cryptography API is still only a draft [1]. I do not do this lightly, but you have my veto to do any crypto in javascript unless you convince me first it make sense. Simo. Just to clarify, the point is to display some kind of information about the public keys the user just entered in the UI (that have not been saved) so that: a) If user enters multiple keys at once, the user can distinguish which keys have been added rather than displaying a generic New: key set for all new keys. b) The UI can detect if the key already exists on the server. If this can be done without any cryptographic operations that would be great, but I'm not sure about the details. For (a) I was wondering if the UI can decode the base-64 encoded public key entered by the user and display some user-friendly information about the key itself, maybe just the hexdump of the first few bytes of the key, or maybe the key type too if possible, or at least just the first few chars of the undecoded key. For (b) the UI would have to use some crypto functions to generate the key fingerprints as generated by the server. But even if we do that, we are not doing any data encryption or dealing with secret information. I do not think you need crypto functions for (a) and it is unclear to me what (b) is, if the key already exist on the IPA server you can find it out with a simple memcmp, why should you need a fingeprint ? Alternatively, instead of displaying the fingerprints of the saved keys, the UI can display the first few bytes/chars of hexdump/encoded key to match (a) such that they can be compared without cryptography. BTW, WebCrypto implementation has been making its way into Firefox: https://docs.google.com/spreadsheet/ccc?key=0AiAcidBZRLxndE9LWEs2R1oxZ0xidUVoU3FQbFFobkEusp=sharing Yes and I find it a particularly bad/dangerous thing, I tend to agree with this: http://tonyarcieri.com/whats-wrong-with-webcrypto see also the links it points to which explains why crypto in javascript (ie in the user-hostile sandbox running in the browser) is mostly a bad idea, and can give a false sense of security and a slippery slope in actually unprotecting data. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0637 upgradeinstance: Restore listeners on failure
https://fedorahosted.org/freeipa/ticket/4499
Actually I wonder why we use backup_state/restore_state for these settings.
Rob, was there a reason for not just always setting nsslapd-port: 389
and nsslapd-security: on?
--
Petr³
From 8799b97aaa1a0954c6978f6f1133c757d14f4c39 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Fri, 22 Aug 2014 14:22:06 +0200
Subject: [PATCH] upgradeinstance: Restore listeners on failure
Allow running some installation after failure,
and use this for the upgradeinstance cleanup steps.
https://fedorahosted.org/freeipa/ticket/4499
---
ipaserver/install/service.py | 30 --
ipaserver/install/upgradeinstance.py | 19 +--
2 files changed, 37 insertions(+), 12 deletions(-)
diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 585c903bdd4dee66d93c426299d59de95d0ea625..018c369ffb3c8f782b3d59b82e3bb71c898b9b9f 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -23,6 +23,7 @@
import pwd
import time
import datetime
+import traceback
from ipapython import sysrestore, ipautil, dogtag, ipaldap
from ipapython.dn import DN
@@ -329,8 +330,8 @@ def get_state(self, key):
def print_msg(self, message):
print_msg(message, self.output_fd)
-def step(self, message, method):
-self.steps.append((message, method))
+def step(self, message, method, run_after_failure=False):
+self.steps.append((message, method, run_after_failure))
def start_creation(self, start_message=None, end_message=None,
show_service_name=True, runtime=-1):
@@ -375,15 +376,32 @@ def start_creation(self, start_message=None, end_message=None,
else:
self.print_msg(start_message)
-step = 0
-for (message, method) in self.steps:
-self.print_msg( [%d/%d]: %s % (step+1, len(self.steps), message))
+def run_step(message, method):
+self.print_msg(message)
s = datetime.datetime.now()
method()
e = datetime.datetime.now()
d = e - s
root_logger.debug( duration: %d seconds % d.seconds)
-step += 1
+
+step = 0
+steps_iter = iter(self.steps)
+try:
+for message, method, run_after_failure in steps_iter:
+full_msg = [%d/%d]: %s % (step+1, len(self.steps), message)
+run_step(full_msg, method)
+step += 1
+except BaseException as e:
+# show the traceback, so it's not lost if the cleanup method fails
+root_logger.debug(%s % traceback.format_exc())
+self.print_msg(' [error] %s: %s' % (type(e).__name__, e))
+
+# run through remaining methods marked run_after_failure
+for message, method, run_after_failure in steps_iter:
+if run_after_failure:
+run_step( [cleanup]: %s % message, method)
+
+raise
self.print_msg(end_message)
diff --git a/ipaserver/install/upgradeinstance.py b/ipaserver/install/upgradeinstance.py
index 062d5da22c32dd905189309d83a0295a3f6dbdd8..b553721eba615f324f9e52331cbd0910c3e14be6 100644
--- a/ipaserver/install/upgradeinstance.py
+++ b/ipaserver/install/upgradeinstance.py
@@ -82,8 +82,11 @@ def create_instance(self):
if self.schema_files:
self.step(updating schema, self.__update_schema)
self.step(upgrading server, self.__upgrade)
-self.step(stopping directory server, self.__stop_instance)
-self.step(restoring configuration, self.__restore_config)
+
+self.step(stopping directory server, self.__stop_instance,
+ run_after_failure=True)
+self.step(restoring configuration, self.__restore_config,
+ run_after_failure=True)
self.step(starting directory server, self.start)
self.start_creation(start_message=Upgrading IPA:,
@@ -103,10 +106,14 @@ def __restore_config(self):
port = self.restore_state('nsslapd-port')
security = self.restore_state('nsslapd-security')
-installutils.set_directive(self.filename, 'nsslapd-port',
-port, quotes=False, separator=':')
-installutils.set_directive(self.filename, 'nsslapd-security',
-security, quotes=False, separator=':')
+if port is not None:
+installutils.set_directive(
+self.filename, 'nsslapd-port', port,
+quotes=False, separator=':')
+if security is not None:
+installutils.set_directive(
+self.filename, 'nsslapd-security', security,
+quotes=False, separator=':')
def __disable_listeners(self):
installutils.set_directive(self.filename, 'nsslapd-port',
--
1.9.3
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
[Freeipa-devel] [PATCH] 744 webui: switch associators if default doesn't work
Ticket: https://fedorahosted.org/freeipa/ticket/4507
Support for delegating RBAC roles to service principals added new
attribute members. [1][2] Most of Web UI was automatically extended but
the defaults chose wrong associator for service's memberof_role facet
traditionally it would be solved by
{
$type: 'association',
name: 'memberof_role',
associator: IPA.serial_associator
}
This patch tries to make the auto-magic functionality little bit less
stupid to eliminate a need for ^^ patches. It's far from perfect -
doesn't support things like:
{
$type: 'association',
name: 'memberof_sudorule',
associator: IPA.serial_associator,
add_method: 'add_user',
remove_method: 'remove_user'
}
[1]
https://git.fedorahosted.org/cgit/freeipa.git/commit/?id=8fabd6dde152fc394bd4f093d93c8a46e5b2851b
[2] https://fedorahosted.org/freeipa/ticket/3164
--
Petr Vobornik
From 21d2df52e89f4e6fb3576f9e81e6e7e3c094fee1 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 22 Aug 2014 17:58:48 +0200
Subject: [PATCH] webui: switch associators if default doesn't work
Make association auto-magic little bit less stupid. Now it supports
adding of new attribute member with add_member and remove_member
methods only on one side of the relationship.
https://fedorahosted.org/freeipa/ticket/4507
---
install/ui/src/freeipa/association.js | 10 ++
1 file changed, 10 insertions(+)
diff --git a/install/ui/src/freeipa/association.js b/install/ui/src/freeipa/association.js
index b78903ad734af6488cd9902bc5af2885f99c6bb3..8fe612e913c3fc8bb8c2fa86423eb4b46f224ddc 100644
--- a/install/ui/src/freeipa/association.js
+++ b/install/ui/src/freeipa/association.js
@@ -826,6 +826,16 @@ exp.association_facet_pre_op = function(spec, context) {
spec.other_entity = spec.other_entity ||
spec.name.substring(index+1);
+if (!spec.associator) {
+// batch associator (default) calls entity_command, serial associator
+// calls other_entity_command -- if entity doesn't support the command,
+// switch associators to try the other_entity
+var add_command = spec.add_command || 'add_member';
+if (!metadata_provider.get('@mc:'+entity.name+'_'+add_command)) {
+spec.associator = IPA.serial_associator;
+}
+}
+
spec.add_title = '@i18n:association.add.'+spec.attribute_member;
spec.remove_title = '@i18n:association.remove.'+spec.attribute_member;
--
1.9.3
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 736-740 webui: various minor fixes
On 22.8.2014 17:51, Simo Sorce wrote: On Fri, 2014-08-22 at 09:52 -0500, Endi Sukma Dewata wrote: On 8/21/2014 7:18 AM, Simo Sorce wrote: On Thu, 2014-08-21 at 14:11 +0200, Petr Vobornik wrote: On 13.8.2014 17:20, Endi Sukma Dewata wrote: 2. Can the UI parse the new key and display it the same way as other keys that are already saved? That will make it more seamless. Would be nice, but is it worth the effort? We would have to reimplement ipapython.ssh into JavaScript + pull in crypto.js or other lib for sha1and sha256 functions since Web Cryptography API is still only a draft [1]. I do not do this lightly, but you have my veto to do any crypto in javascript unless you convince me first it make sense. Simo. Just to clarify, the point is to display some kind of information about the public keys the user just entered in the UI (that have not been saved) so that: a) If user enters multiple keys at once, the user can distinguish which keys have been added rather than displaying a generic New: key set for all new keys. b) The UI can detect if the key already exists on the server. If this can be done without any cryptographic operations that would be great, but I'm not sure about the details. For (a) I was wondering if the UI can decode the base-64 encoded public key entered by the user and display some user-friendly information about the key itself, maybe just the hexdump of the first few bytes of the key, or maybe the key type too if possible, or at least just the first few chars of the undecoded key. For (b) the UI would have to use some crypto functions to generate the key fingerprints as generated by the server. But even if we do that, we are not doing any data encryption or dealing with secret information. I do not think you need crypto functions for (a) and it is unclear to me what (b) is, if the key already exist on the IPA server you can find it out with a simple memcmp, why should you need a fingeprint ? in both (a) and (b) the key does not exist on the server - user just added the key(s) but he had not yet clicked on 'update' button to upload them. Will displaying of the fingerprints prior saving help(improve UX) the user? The issue is that user can't distinguish multiple added keys. The first few chars of the undecoded key variant of (a) will solve the issue in the most efficient way - user will see what he just added. No crypto needed. Alternatively, instead of displaying the fingerprints of the saved keys, the UI can display the first few bytes/chars of hexdump/encoded key to match (a) such that they can be compared without cryptography. BTW, WebCrypto implementation has been making its way into Firefox: https://docs.google.com/spreadsheet/ccc?key=0AiAcidBZRLxndE9LWEs2R1oxZ0xidUVoU3FQbFFobkEusp=sharing Yes and I find it a particularly bad/dangerous thing, I tend to agree with this: http://tonyarcieri.com/whats-wrong-with-webcrypto see also the links it points to which explains why crypto in javascript (ie in the user-hostile sandbox running in the browser) is mostly a bad idea, and can give a false sense of security and a slippery slope in actually unprotecting data. Simo. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 736-740 webui: various minor fixes
On 8/22/2014 12:18 PM, Petr Vobornik wrote: On 22.8.2014 17:51, Simo Sorce wrote: On Fri, 2014-08-22 at 09:52 -0500, Endi Sukma Dewata wrote: On 8/21/2014 7:18 AM, Simo Sorce wrote: On Thu, 2014-08-21 at 14:11 +0200, Petr Vobornik wrote: On 13.8.2014 17:20, Endi Sukma Dewata wrote: 2. Can the UI parse the new key and display it the same way as other keys that are already saved? That will make it more seamless. Would be nice, but is it worth the effort? We would have to reimplement ipapython.ssh into JavaScript + pull in crypto.js or other lib for sha1and sha256 functions since Web Cryptography API is still only a draft [1]. I do not do this lightly, but you have my veto to do any crypto in javascript unless you convince me first it make sense. Simo. Just to clarify, the point is to display some kind of information about the public keys the user just entered in the UI (that have not been saved) so that: a) If user enters multiple keys at once, the user can distinguish which keys have been added rather than displaying a generic New: key set for all new keys. b) The UI can detect if the key already exists on the server. If this can be done without any cryptographic operations that would be great, but I'm not sure about the details. For (a) I was wondering if the UI can decode the base-64 encoded public key entered by the user and display some user-friendly information about the key itself, maybe just the hexdump of the first few bytes of the key, or maybe the key type too if possible, or at least just the first few chars of the undecoded key. For (b) the UI would have to use some crypto functions to generate the key fingerprints as generated by the server. But even if we do that, we are not doing any data encryption or dealing with secret information. I do not think you need crypto functions for (a) and it is unclear to me what (b) is, if the key already exist on the IPA server you can find it out with a simple memcmp, why should you need a fingeprint ? in both (a) and (b) the key does not exist on the server - user just added the key(s) but he had not yet clicked on 'update' button to upload them. Right, all the UI has is the base-64 encoded key entered by the user. So without crypto, all the UI can do is probably decoding the key and, if possible, parsing the key structure to obtain some useful information. Can we obtain the key type this way, or does it require crypto? The existing keys on the other hand are displayed in the UI with their fingerprints and key type, for example: A8:50:ED:5A:8A:78:81:8D:B9:34:CC:99:D4:6A:E1:32 (ssh-rsa) The UI does not show the base-64 encoded key unless the user clicks the Show/Set key button. To check if a new key already exists on the server, the UI can compare either the base-64 encoded keys or the fingerprints (if available). Will displaying of the fingerprints prior saving help(improve UX) the user? The issue is that user can't distinguish multiple added keys. The first few chars of the undecoded key variant of (a) will solve the issue in the most efficient way - user will see what he just added. No crypto needed. Displaying the fingerprints of the new keys is not necessary to solve (a), but the display will not be seamless without it. For example, it could become like this: A8:50:ED:5A:8A:78:81:8D:B9:34:CC:99:D4:6A:E1:32 (ssh-rsa) B3NzaC1yc2EBIwAAAQEAueevoxw+... (new key) On the other hand, since existing keys are displayed with their fingerprints, it's not possible to open a public key file and compare it to the list before adding it to the UI. Are the fingerprints actually only used internally by the server? Do the users have any use of the fingerprints? If the fingerprints are internal only, the UI shouldn't need to display it. I'd say the UI should just display the key itself, not the whole key but just long enough to be distinguishable, either as a hexdump or base-64 encoded. BTW, WebCrypto implementation has been making its way into Firefox: https://docs.google.com/spreadsheet/ccc?key=0AiAcidBZRLxndE9LWEs2R1oxZ0xidUVoU3FQbFFobkEusp=sharing Yes and I find it a particularly bad/dangerous thing, I tend to agree with this: http://tonyarcieri.com/whats-wrong-with-webcrypto see also the links it points to which explains why crypto in javascript (ie in the user-hostile sandbox running in the browser) is mostly a bad idea, and can give a false sense of security and a slippery slope in actually unprotecting data. Simo. If for some reason we decided to generate fingerprints in the UI, in my opinion using WebCrypto will not pose any security problem or mislead users into a false sense of security because the operation is purely informational, it does not do any encryption/decryption/validation. Any fingerprint generated on the UI side will be discarded anyway, the server will still generate its own fingerprints. This belongs to a separate discussion, but the article was
