From 50a68a489a85a8a57280a3a53d1ee9f589fb3487 Mon Sep 17 00:00:00 2001 From: Dan Breslau Date: Mon, 15 Aug 2016 15:24:24 -0400 Subject: [PATCH] Implemented "Clear Trust Anchor" button in Edit Identity Dialog. Modified the "user verified" flag to be stored in the keystore (or local flat file), instead of in a Properties file. Modified the TrustAnchor class to be read-only, other than the "user_verified" field. Many logging updates. --- src/moonshot-id.vala | 54 +++++++++++++----- src/moonshot-identity-dialog.vala | 13 +++-- src/moonshot-identity-management-view.vala | 8 ++- src/moonshot-identity-manager-app.vala | 8 ++- src/moonshot-keyring-store.vala | 26 +++++++-- src/moonshot-local-flat-file-store.vala | 13 +++-- src/moonshot-provisioning-common.vala | 89 +++++++++++++++++++----------- src/moonshot-server.vala | 8 +-- 8 files changed, 153 insertions(+), 66 deletions(-) diff --git a/src/moonshot-id.vala b/src/moonshot-id.vala index 46fd088..5b927c9 100644 --- a/src/moonshot-id.vala +++ b/src/moonshot-id.vala @@ -35,6 +35,8 @@ using Gee; extern char* get_cert_valid_before(char* cert, int certlen, char* datebuf, int buflen); +// A TrustAnchor object can be imported or installed via the API, but cannot +// be modified by the user, other than being cleared. Hence the fields are read-only. public class TrustAnchor : Object { private static const string CERT_HEADER = "-----BEGIN CERTIFICATE-----"; @@ -50,31 +52,41 @@ public class TrustAnchor : Object private string _subject_alt = ""; private string _server_cert = ""; + public bool user_verified = false; + + public TrustAnchor(string ca_cert, string server_cert, string subject, string subject_alt, bool user_verified) { + _ca_cert = ca_cert; + _server_cert = server_cert; + _subject = subject; + _subject_alt = subject_alt; + this.user_verified = user_verified; + } + + public TrustAnchor.empty() { + _ca_cert = ""; + _server_cert = ""; + _subject = ""; + _subject_alt = ""; + this.user_verified = false; + } + + public string ca_cert { get { return _ca_cert; } - set { - _ca_cert = (value ?? ""); - } } public string subject { get { return _subject; } - set { - _subject = (value ?? ""); - } } public string subject_alt { get { return _subject_alt; } - set { - _subject_alt = (value ?? ""); - } } @@ -82,9 +94,6 @@ public class TrustAnchor : Object get { return _server_cert; } - set { - _server_cert = (value ?? ""); - } } public bool is_empty() { @@ -105,6 +114,8 @@ public class TrustAnchor : Object return 1; if (this.server_cert != other.server_cert) return 1; + if (this.user_verified != other.user_verified) + return 1; return 0; } @@ -254,7 +265,21 @@ public class IdCard : Object public bool temporary {get; set; default = false; } - public TrustAnchor trust_anchor { get; set; default = new TrustAnchor (); } + private TrustAnchor _trust_anchor = new TrustAnchor.empty(); + public TrustAnchor trust_anchor { + get { + return _trust_anchor; + } + } + + // For use by storage implementations. + internal void set_trust_anchor_from_store(TrustAnchor ta) { + _trust_anchor = ta; + } + + internal void clear_trust_anchor() { + _trust_anchor = new TrustAnchor.empty(); + } public unowned string nai { get { _nai = username + "@" + issuer; return _nai;}} @@ -300,6 +325,9 @@ public class IdCard : Object diff |= 1 << DiffFlags.TRUST_ANCHOR; // stdout.printf("Diff Flags: %x\n", diff); + if (this.display_name == other.display_name && diff != 0) { + logger.trace("Compare: Two IDs with display_name '%s', but diff_flags=%0x".printf(this.display_name, diff)); + } return diff; } diff --git a/src/moonshot-identity-dialog.vala b/src/moonshot-identity-dialog.vala index 3c96d49..7872fb4 100644 --- a/src/moonshot-identity-dialog.vala +++ b/src/moonshot-identity-dialog.vala @@ -69,6 +69,9 @@ class IdentityDialog : Dialog private Label selected_item = null; + // Whether to clear the card's TrustAnchor after the user selects OK + internal bool clear_trust_anchor = false; + public string display_name { get { return displayname_entry.get_text(); } } @@ -199,7 +202,7 @@ class IdentityDialog : Dialog this.show_all(); } - private static Widget make_trust_anchor_box(IdCard id) + private Widget make_trust_anchor_box(IdCard id) { Label ta_label = new Label(_("Trust anchor: ") @@ -218,7 +221,11 @@ class IdentityDialog : Dialog int row = 0; var ta_clear_button = new Button.with_label(_("Clear Trust Anchor")); - ta_clear_button.clicked.connect((w) => {id.trust_anchor = new TrustAnchor();}); + ta_clear_button.clicked.connect((w) => { + clear_trust_anchor = true; + ta_table.set_sensitive(false); + } + ); ta_table.attach(ta_label, 0, 1, row, row + 1, opts, opts, 0, 0); ta_table.attach(ta_clear_button, 1, 2, row, row + 1, fill, fill, 0, 0); @@ -467,6 +474,4 @@ class IdentityDialog : Dialog return services_vbox; } - - } diff --git a/src/moonshot-identity-management-view.vala b/src/moonshot-identity-management-view.vala index 9d728e9..fb36685 100644 --- a/src/moonshot-identity-management-view.vala +++ b/src/moonshot-identity-management-view.vala @@ -251,6 +251,10 @@ public class IdentityManagerView : Window { id_card.update_services_from_list(dialog.get_services()); + if (dialog.clear_trust_anchor) { + id_card.clear_trust_anchor(); + } + return id_card; } @@ -591,7 +595,7 @@ public class IdentityManagerView : Window { private bool check_and_confirm_trust_anchor(IdCard id) { if (!id.trust_anchor.is_empty() && id.trust_anchor.get_anchor_type() == TrustAnchor.TrustAnchorType.SERVER_CERT) { - if (get_string_setting("TrustAnchors", id.nai) != id.trust_anchor.server_cert) { + if (!id.trust_anchor.user_verified) { bool ret = false; int result = ResponseType.CANCEL; @@ -601,7 +605,7 @@ public class IdentityManagerView : Window { switch (result) { case ResponseType.OK: - set_string_setting("TrustAnchors", id.nai, id.trust_anchor.server_cert); + id.trust_anchor.user_verified = true; ret = true; break; default: diff --git a/src/moonshot-identity-manager-app.vala b/src/moonshot-identity-manager-app.vala index 39d84f5..4de64be 100644 --- a/src/moonshot-identity-manager-app.vala +++ b/src/moonshot-identity-manager-app.vala @@ -137,7 +137,7 @@ public class IdentityManagerApp { } public void select_identity(IdentityRequest request) { - logger.trace("select_identity"); + logger.trace("select_identity: request.nai=%s".printf(request.nai ?? "[null]")); IdCard identity = null; @@ -157,6 +157,7 @@ public class IdentityManagerApp { /* If NAI matches, use this id card */ if (has_nai && request.nai == id.nai) { + logger.trace("select_identity: request has nai; returning " + id.display_name); identity = id; break; } @@ -165,6 +166,7 @@ public class IdentityManagerApp { if (has_srv) { if (id.services.contains(request.service)) { + logger.trace(@"select_identity: request has service '$(request.service); matched on '$(id.display_name)'"); request.candidates.append(id); } } @@ -173,6 +175,7 @@ public class IdentityManagerApp { /* If more than one candidate we dissasociate service from all ids */ if ((identity == null) && has_srv && request.candidates.length() > 1) { + logger.trace(@"select_identity: multiple candidates; removing service '$(request.service) from all."); foreach (IdCard id in request.candidates) { id.services.remove(request.service); @@ -182,6 +185,7 @@ public class IdentityManagerApp { /* If there are no candidates we use the service matching rules */ if ((identity == null) && (request.candidates.length() == 0)) { + logger.trace("select_identity: No candidates; using service matching rules."); foreach (IdCard id in model.get_card_list()) { foreach (Rule rule in id.rules) @@ -189,6 +193,7 @@ public class IdentityManagerApp { if (!match_service_pattern(request.service, rule.pattern)) continue; + logger.trace(@"select_identity: ID $(id.display_name) matched on service matching rules."); request.candidates.append(id); if (rule.always_confirm == "true") @@ -198,6 +203,7 @@ public class IdentityManagerApp { } if ((identity == null) && has_nai) { + logger.trace("select_identity: Creating temp identity"); // create a temp identity string[] components = request.nai.split("@", 2); identity = new IdCard(); diff --git a/src/moonshot-keyring-store.vala b/src/moonshot-keyring-store.vala index 9045c95..e24a6fe 100644 --- a/src/moonshot-keyring-store.vala +++ b/src/moonshot-keyring-store.vala @@ -105,9 +105,18 @@ public class KeyringStore : Object, IIdentityCardStore { int rules_patterns_index = -1; int rules_always_confirm_index = -1; string store_password = null; + string ca_cert = ""; + string server_cert = ""; + string subject = ""; + string subject_alt = ""; + bool user_verified = false; for (i = 0; i < entry.attributes.len; i++) { var attribute = ((GnomeKeyring.Attribute *) entry.attributes.data)[i]; - string value = attribute.string_value; + string value = ""; + if (attribute.type == GnomeKeyring.AttributeType.STRING) { + value = attribute.string_value; + } + if (attribute.name == "Issuer") { id_card.issuer = value; } else if (attribute.name == "Username") { @@ -121,17 +130,23 @@ public class KeyringStore : Object, IIdentityCardStore { } else if (attribute.name == "Rules-AlwaysConfirm") { rules_always_confirm_index = i; } else if (attribute.name == "CA-Cert") { - id_card.trust_anchor.ca_cert = value.strip(); + ca_cert = value.strip(); } else if (attribute.name == "Server-Cert") { - id_card.trust_anchor.server_cert = value; + server_cert = value; } else if (attribute.name == "Subject") { - id_card.trust_anchor.subject = value; + subject = value; } else if (attribute.name == "Subject-Alt") { - id_card.trust_anchor.subject_alt = value; + subject_alt = value; } else if (attribute.name == "StorePassword") { store_password = value; + } else if (attribute.name == "CACert_User_Verified") { + user_verified = (value == "true"); } } + + var ta = new TrustAnchor(ca_cert, server_cert, subject, subject_alt, user_verified); + id_card.set_trust_anchor_from_store(ta); + if ((rules_always_confirm_index != -1) && (rules_patterns_index != -1)) { string rules_patterns_all = ((GnomeKeyring.Attribute *) entry.attributes.data)[rules_patterns_index].string_value; string rules_always_confirm_all = ((GnomeKeyring.Attribute *) entry.attributes.data)[rules_always_confirm_index].string_value; @@ -189,6 +204,7 @@ public class KeyringStore : Object, IIdentityCardStore { attributes.append_string("Server-Cert", id_card.trust_anchor.server_cert); attributes.append_string("Subject", id_card.trust_anchor.subject); attributes.append_string("Subject-Alt", id_card.trust_anchor.subject_alt); + attributes.append_string("CACert_User_Verified", id_card.trust_anchor.user_verified ? "true" : "false"); attributes.append_string("StorePassword", id_card.store_password ? "yes" : "no"); GnomeKeyring.Result result = GnomeKeyring.item_create_sync(null, diff --git a/src/moonshot-local-flat-file-store.vala b/src/moonshot-local-flat-file-store.vala index c129072..55c535b 100644 --- a/src/moonshot-local-flat-file-store.vala +++ b/src/moonshot-local-flat-file-store.vala @@ -111,11 +111,13 @@ public class LocalFlatFileStore : Object, IIdentityCardStore { } // Trust anchor - id_card.trust_anchor.ca_cert = key_file.get_string(identity, "CA-Cert").strip(); - id_card.trust_anchor.subject = key_file.get_string(identity, "Subject"); - id_card.trust_anchor.subject_alt = key_file.get_string(identity, "SubjectAlt"); - id_card.trust_anchor.server_cert = key_file.get_string(identity, "ServerCert"); - + string ca_cert = key_file.get_string(identity, "CA-Cert").strip(); + string server_cert = key_file.get_string(identity, "ServerCert"); + string subject = key_file.get_string(identity, "Subject"); + string subject_alt = key_file.get_string(identity, "SubjectAlt"); + bool user_verified = key_file.get_boolean(identity, "CACert_User_Verified"); + var ta = new TrustAnchor(ca_cert, server_cert, subject, subject_alt, user_verified); + id_card.set_trust_anchor_from_store(ta); id_card_list.add(id_card); } catch (Error e) { @@ -177,6 +179,7 @@ public class LocalFlatFileStore : Object, IIdentityCardStore { key_file.set_string(id_card.display_name, "Subject", id_card.trust_anchor.subject ?? ""); key_file.set_string(id_card.display_name, "SubjectAlt", id_card.trust_anchor.subject_alt ?? ""); key_file.set_string(id_card.display_name, "ServerCert", id_card.trust_anchor.server_cert ?? ""); + key_file.set_boolean(id_card.display_name, "CACert_User_Verified", id_card.trust_anchor.user_verified); } var text = key_file.to_data(null); diff --git a/src/moonshot-provisioning-common.vala b/src/moonshot-provisioning-common.vala index b18cd8b..cd85f9a 100644 --- a/src/moonshot-provisioning-common.vala +++ b/src/moonshot-provisioning-common.vala @@ -33,12 +33,12 @@ namespace WebProvisioning -{ +{ bool check_stack(SList stack, string[] reference) { if (stack.length() < reference.length) return false; - + for (int i = 0; i < reference.length; i++) { if (stack.nth_data(i) != reference[i]) @@ -51,74 +51,74 @@ namespace WebProvisioning bool always_confirm_handler(SList stack) { string[] always_confirm_path = {"always-confirm", "rule", "selection-rules", "identity", "identities"}; - + return check_stack(stack, always_confirm_path); } - + bool pattern_handler(SList stack) { string[] pattern_path = {"pattern", "rule", "selection-rules", "identity", "identities"}; - + return check_stack(stack, pattern_path); } bool server_cert_handler(SList stack) { string[] server_cert_path = {"server-cert", "trust-anchor", "identity", "identities"}; - + return check_stack(stack, server_cert_path); } bool subject_alt_handler(SList stack) { string[] subject_alt_path = {"subject-alt", "trust-anchor", "identity", "identities"}; - + return check_stack(stack, subject_alt_path); } bool subject_handler(SList stack) { string[] subject_path = {"subject", "trust-anchor", "identity", "identities"}; - + return check_stack(stack, subject_path); } - + bool ca_cert_handler(SList stack) { string[] ca_path = {"ca-cert", "trust-anchor", "identity", "identities"}; - + return check_stack(stack, ca_path); } bool realm_handler(SList stack) { string[] realm_path = {"realm", "identity", "identities"}; - + return check_stack(stack, realm_path); } bool password_handler(SList stack) { string[] password_path = {"password", "identity", "identities"}; - + return check_stack(stack, password_path); } bool user_handler(SList stack) { string[] user_path = {"user", "identity", "identities"}; - + return check_stack(stack, user_path); } bool display_name_handler(SList stack) { string[] display_name_path = {"display-name", "identity", "identities"}; - + return check_stack(stack, display_name_path); } - + public class Parser : Object { private static MoonshotLogger logger = new MoonshotLogger("WebProvisioning"); @@ -126,7 +126,7 @@ namespace WebProvisioning private void start_element_func(MarkupParseContext context, string element_name, string[] attribute_names, - string[] attribute_values) throws MarkupError + string[] attribute_values) throws MarkupError { if (element_name == "identity") { @@ -145,10 +145,10 @@ namespace WebProvisioning string text, size_t text_len) throws MarkupError { unowned SList stack = context.get_element_stack(); - + if (text_len < 1) return; - + logger.trace("text_element_func (%p): text='%s'".printf(this, stack.nth_data(0))); if (stack.nth_data(0) == "display-name" && display_name_handler(stack)) @@ -175,7 +175,7 @@ namespace WebProvisioning /* Rules */ else if (stack.nth_data(0) == "pattern" && pattern_handler(stack)) { - /* use temp array to workaround valac 0.10 bug accessing array property length */ + /* use temp array to workaround valac 0.10 bug accessing array property length */ var temp = card.rules; card.rules[temp.length - 1].pattern = text; } @@ -187,22 +187,49 @@ namespace WebProvisioning card.rules[temp.length - 1].always_confirm = text; } } - /*Trust anchor*/ + // This is ugly, but... we use the TrustAnchor field in the IdCard as a placeholder, + // replacing it with a new one every time we read a new element. + // "user_verified" is always false, since we're reading the TrustAnchor from XML. else if (stack.nth_data(0) == "ca-cert" && ca_cert_handler(stack)) { - card.trust_anchor.ca_cert = text; + string ca_cert = text; + var ta = new TrustAnchor(ca_cert, + card.trust_anchor.server_cert, + card.trust_anchor.subject, + card.trust_anchor.subject_alt, + false); + card.set_trust_anchor_from_store(ta); } - else if (stack.nth_data(0) == "subject" && subject_handler(stack)) + else if (stack.nth_data(0) == "server-cert" && server_cert_handler(stack)) { - card.trust_anchor.subject = text; + string server_cert = text; + var ta = new TrustAnchor(card.trust_anchor.ca_cert, + server_cert, + card.trust_anchor.subject, + card.trust_anchor.subject_alt, + false); + card.set_trust_anchor_from_store(ta); + } - else if (stack.nth_data(0) == "subject-alt" && subject_alt_handler(stack)) + else if (stack.nth_data(0) == "subject" && subject_handler(stack)) { - card.trust_anchor.subject_alt = text; + string subject = text; + var ta = new TrustAnchor(card.trust_anchor.ca_cert, + card.trust_anchor.server_cert, + subject, + card.trust_anchor.subject_alt, + false); + card.set_trust_anchor_from_store(ta); } - else if (stack.nth_data(0) == "server-cert" && server_cert_handler(stack)) + else if (stack.nth_data(0) == "subject-alt" && subject_alt_handler(stack)) { - card.trust_anchor.server_cert = text; + string subject_alt = text; + var ta = new TrustAnchor(card.trust_anchor.ca_cert, + card.trust_anchor.server_cert, + card.trust_anchor.subject, + subject_alt, + false); + card.set_trust_anchor_from_store(ta); } } @@ -233,7 +260,7 @@ namespace WebProvisioning this.path = path; var file = File.new_for_path(path); - + try { var dis = new DataInputStream(file.read()); @@ -243,7 +270,7 @@ namespace WebProvisioning // Preserve newlines -- important for certificate import. // (X509 certs can't be parsed without the newlines.) - // + // // This may add an extra newline at EOF. Maybe use // dis.read_upto("\n", ...) followed by dis.read_byte() instead? text += "\n"; @@ -253,7 +280,7 @@ namespace WebProvisioning { error("Could not retreive file size"); } - + logger.trace(@"Parser(): read text to parse; length=$(text.length)"); } @@ -265,7 +292,7 @@ namespace WebProvisioning catch(GLib.Error e) { error("Could not parse %s, invalid content", path); - } + } } } } diff --git a/src/moonshot-server.vala b/src/moonshot-server.vala index fb68864..5f0fa2f 100644 --- a/src/moonshot-server.vala +++ b/src/moonshot-server.vala @@ -117,7 +117,7 @@ public class MoonshotServer : Object { if (subject_alt_name_constraint == null) subject_alt_name_constraint = ""; - logger.trace("MoonshotServer.get_identity: returning true"); + logger.trace(@"MoonshotServer.get_identity: returning with nai_out=$nai_out"); return true; } @@ -198,10 +198,8 @@ public class MoonshotServer : Object { idcard.store_password = true; idcard.issuer = realm; idcard.update_services(services); - idcard.trust_anchor.ca_cert = ca_cert; - idcard.trust_anchor.subject = subject; - idcard.trust_anchor.subject_alt = subject_alt; - idcard.trust_anchor.server_cert = server_cert; + var ta = new TrustAnchor(ca_cert, server_cert, subject, subject_alt, false); + idcard.set_trust_anchor_from_store(ta); logger.trace("install_id_card: Card '%s' has services: '%s'" .printf(idcard.display_name, idcard.get_services_string("; "))); -- 2.1.4