From 6bc5c0899a9eb6c134c0079406fe28113e5cbeaf Mon Sep 17 00:00:00 2001 From: Asanka Herath Date: Sun, 17 Jun 2007 05:49:32 +0000 Subject: [PATCH] windows-nim-afs-20070617 Don't assume that WM_DESTROY is the final message received by a window. Verify dialog data structures when handling messages and reset the window data field when freeing the data structure. Zero should be considered a valid credentials type identifier in Network Identity Manager. When checking if an identity is configured to obtain a token for a specific cell, don't go through the list of cells if AFS tokens are disabled for the identity. Similarly, when removing a token for a specific cell from all identities, don't bother modifying identities for whom AFS tokens are disabled. Keep track of whether a specific cell was added to the list of cells to authenticate for an identity because it was listed in the configuration or because a token for the cell already existed. Correct an off-by-one error when calculating buffer sizes for multi strings which failed to account for a double NULL terminator. Don't update the cell->identity mapping if a token for that cell could not be obtained. If the list of cell to authenticate for an identity is empty, we still need to write the empty string to the configuration. Otherwise, removing all the tokens from an identity will not result in a configuration change reflecting that. --- src/WINNT/netidmgr_plugin/afsconfigdlg.c | 37 ++++++++++-- src/WINNT/netidmgr_plugin/afsfuncs.c | 2 +- src/WINNT/netidmgr_plugin/afsnewcreds.c | 71 +++++++++++++++++++----- src/WINNT/netidmgr_plugin/afsnewcreds.h | 4 ++ 4 files changed, 95 insertions(+), 19 deletions(-) diff --git a/src/WINNT/netidmgr_plugin/afsconfigdlg.c b/src/WINNT/netidmgr_plugin/afsconfigdlg.c index f121007a7..908050f1a 100644 --- a/src/WINNT/netidmgr_plugin/afsconfigdlg.c +++ b/src/WINNT/netidmgr_plugin/afsconfigdlg.c @@ -85,7 +85,10 @@ afs_cfg_ids_proc(HWND hwnd, d = (afs_ids_dlg_data *) (LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); - PFREE(d); + if (d != NULL) { + PFREE(d); + SetWindowLongPtr(hwnd, DWLP_USER, 0); + } } return TRUE; @@ -94,6 +97,9 @@ afs_cfg_ids_proc(HWND hwnd, d = (afs_ids_dlg_data *) (LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); + if (d == NULL) + return FALSE; + if (wParam == MAKEWPARAM(IDC_CFG_OBTAIN, BN_CLICKED)) { d->afs_enabled = (IsDlgButtonChecked(hwnd, IDC_CFG_OBTAIN) == @@ -110,6 +116,9 @@ afs_cfg_ids_proc(HWND hwnd, d = (afs_ids_dlg_data *) (LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); + if (d == NULL) + return FALSE; + if (HIWORD(wParam) == WMCFG_APPLY) { khm_int32 t; @@ -154,6 +163,11 @@ afs_cfg_id_proc(HWND hwnd, rv = afs_dlg_proc(hwnd, uMsg, wParam, 0); d = (afs_dlg_data *) (LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); +#ifdef DEBUG + assert(d != NULL); +#endif + if (d == NULL) + return rv; d->cfg = *((khui_config_init_data *) lParam); @@ -187,7 +201,9 @@ afs_cfg_id_proc(HWND hwnd, #ifdef DEBUG assert(d && d->ident); #endif - kcdb_identity_release(d->ident); + if (d && d->ident) { + kcdb_identity_release(d->ident); + } return afs_dlg_proc(hwnd, uMsg, wParam, lParam); } @@ -199,6 +215,9 @@ afs_cfg_id_proc(HWND hwnd, d = (afs_dlg_data *) (LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); + if (d == NULL) + return TRUE; + if (HIWORD(wParam) == WMCFG_APPLY) { afs_cred_write_ident_data(d); } @@ -553,9 +572,11 @@ afs_cfg_main_proc(HWND hwnd, node = (khui_config_node) (DWORD_PTR) GetWindowLongPtr(hwnd, DWLP_USER); - khui_cfg_set_flags(node, - KHUI_CNFLAG_MODIFIED, - KHUI_CNFLAG_MODIFIED); + if (node != NULL) { + khui_cfg_set_flags(node, + KHUI_CNFLAG_MODIFIED, + KHUI_CNFLAG_MODIFIED); + } } break; } @@ -573,6 +594,12 @@ afs_cfg_main_proc(HWND hwnd, node = (khui_config_node) (DWORD_PTR) GetWindowLongPtr(hwnd, DWLP_USER); +#ifdef DEBUG + assert(node != NULL); +#endif + if (node == NULL) + break; + kmm_get_plugin_config(AFS_PLUGIN_NAME, KHM_PERM_WRITE, &csp_afscred); diff --git a/src/WINNT/netidmgr_plugin/afsfuncs.c b/src/WINNT/netidmgr_plugin/afsfuncs.c index 223fba298..9b416a026 100644 --- a/src/WINNT/netidmgr_plugin/afsfuncs.c +++ b/src/WINNT/netidmgr_plugin/afsfuncs.c @@ -521,7 +521,7 @@ _no_krb5: &krb4_credtype_id); } - if (krb4_credtype_id > 0 && + if (krb4_credtype_id >= 0 && KHM_SUCCEEDED(kcdb_credset_find_filtered(NULL, -1, afs_filter_krb4_tkt, (void *) cell, diff --git a/src/WINNT/netidmgr_plugin/afsnewcreds.c b/src/WINNT/netidmgr_plugin/afsnewcreds.c index de5c3896f..e8dcd9e7d 100644 --- a/src/WINNT/netidmgr_plugin/afsnewcreds.c +++ b/src/WINNT/netidmgr_plugin/afsnewcreds.c @@ -377,6 +377,7 @@ afs_remove_token_from_identities(wchar_t * cell) { khm_size cb; wchar_t vbuf[1024]; wchar_t * tbuf = NULL; + khm_int32 enabled = 0; kcdb_identity_create(t, 0, &h_id); if (h_id == NULL) { @@ -393,6 +394,10 @@ afs_remove_token_from_identities(wchar_t * cell) { 0, &csp_afs))) goto _cleanup_loop; + if (KHM_SUCCEEDED(khc_read_int32(csp_afs, L"AFSEnabled", &enabled)) && + !enabled) + goto _cleanup_loop; + if (khc_read_multi_string(csp_afs, L"Cells", NULL, &cb) != KHM_ERROR_TOO_LONG) goto _cleanup_loop; @@ -472,6 +477,7 @@ afs_check_add_token_to_identity(wchar_t * cell, khm_handle ident, khm_size cb; wchar_t vbuf[1024]; wchar_t * tbuf = NULL; + khm_int32 enabled = 0; kcdb_identity_create(t, 0, &h_id); if (h_id == NULL) { @@ -493,6 +499,10 @@ afs_check_add_token_to_identity(wchar_t * cell, khm_handle ident, 0, &csp_afs))) goto _cleanup_loop; + if (KHM_SUCCEEDED(khc_read_int32(csp_afs, L"AFSEnabled", &enabled)) && + !enabled) + goto _cleanup_loop; + if (khc_read_multi_string(csp_afs, L"Cells", NULL, &cb) != KHM_ERROR_TOO_LONG) goto _cleanup_loop; @@ -603,8 +613,8 @@ afs_cred_get_identity_creds(afs_cred_list * l, StringCbCopy(r->cell, cb, s); r->realm = NULL; - r->method = 0; - r->flags = 0; + r->method = AFS_TOKEN_AUTO; + r->flags = DLGROW_FLAG_CONFIG; if(KHM_SUCCEEDED(khc_open_space(h_cells, s, 0, &h_cell))) { @@ -852,6 +862,9 @@ nc_dlg_show_tooltip(HWND hwnd, d = (afs_dlg_data *)(LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); + if (d == NULL) + return; + ZeroMemory(&ti, sizeof(ti)); ti.cbSize = sizeof(ti); ti.hwnd = hwnd; @@ -890,6 +903,9 @@ nc_dlg_hide_tooltip(HWND hwnd, UINT_PTR id) d = (afs_dlg_data *)(LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); + if (d == NULL) + return; + if(!d->tooltip_visible) return; @@ -973,6 +989,9 @@ nc_dlg_del_token(HWND hwnd) { d = (afs_dlg_data *)(LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); + if (d == NULL) + return; + if (d->nc) khui_cw_find_type(d->nc, afs_credtype_id, &nct); @@ -1059,6 +1078,9 @@ nc_dlg_add_token(HWND hwnd) { d = (afs_dlg_data *)(LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); + if (d == NULL) + return; + if (d->nc) khui_cw_find_type(d->nc, afs_credtype_id, &nct); else @@ -1526,6 +1548,9 @@ afs_dlg_proc(HWND hwnd, d = (afs_dlg_data *)(LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); + if (d == NULL) + return TRUE; + EnterCriticalSection(&d->cs); if (d->nc) { @@ -1540,6 +1565,8 @@ afs_dlg_proc(HWND hwnd, DeleteCriticalSection(&d->cs); PFREE(d); + + SetWindowLongPtr(hwnd, DWLP_USER, 0); } return TRUE; @@ -1551,6 +1578,9 @@ afs_dlg_proc(HWND hwnd, d = (afs_dlg_data *)(LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); + if (d == NULL) + return FALSE; + EnterCriticalSection(&d->cs); if (d->nc) @@ -1605,6 +1635,9 @@ afs_dlg_proc(HWND hwnd, d = (afs_dlg_data *)(LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); + if (d == NULL) + return TRUE; + EnterCriticalSection(&d->cs); if (d->nc) @@ -1885,6 +1918,9 @@ afs_dlg_proc(HWND hwnd, d = (afs_dlg_data *)(LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); + if (d == NULL) + return FALSE; + EnterCriticalSection(&d->cs); hw = GetDlgItem(hwnd, IDC_NCAFS_TOKENLIST); @@ -1939,6 +1975,9 @@ afs_dlg_proc(HWND hwnd, d = (afs_dlg_data *)(LONG_PTR) GetWindowLongPtr(hwnd, DWLP_USER); + if (d == NULL) + return FALSE; + EnterCriticalSection(&d->cs); pnmi = (LPNMITEMACTIVATE) lpnmh; @@ -2158,13 +2197,13 @@ afs_cred_write_ident_data(afs_dlg_data * d) { lru_cell, &cbt); } else { - cbcell = MAXCELLCHARS * sizeof(wchar_t) * - l->n_rows; - if (cbcell > 0) { + cbcell = MAXCELLCHARS * sizeof(wchar_t) * l->n_rows + sizeof(wchar_t); + if (l->n_rows > 0) { lru_cell = PMALLOC(cbcell); ZeroMemory(lru_cell, cbcell); } else { lru_cell = NULL; + cbcell = 0; } } @@ -2182,21 +2221,23 @@ afs_cred_write_ident_data(afs_dlg_data * d) { lru_realm, &cbt); } else { - cbrealm = MAXCELLCHARS * sizeof(wchar_t) * l->n_rows; - if (cbrealm > 0) { + cbrealm = MAXCELLCHARS * sizeof(wchar_t) * l->n_rows + sizeof(wchar_t); + if (l->n_rows > 0) { lru_realm = PMALLOC(cbrealm); ZeroMemory(lru_realm, cbrealm); } else { lru_cell = NULL; + cbrealm = 0; } } - cbidcell = MAXCELLCHARS * sizeof(wchar_t) * l->n_rows; - if (cbidcell > 0) { + cbidcell = MAXCELLCHARS * sizeof(wchar_t) * l->n_rows + sizeof(wchar_t); + if (l->n_rows > 0) { id_cell = PMALLOC(cbidcell); ZeroMemory(id_cell, cbidcell); } else { id_cell = NULL; + cbidcell = 0; } for(i=0; i < l->n_rows; i++) @@ -2244,10 +2285,12 @@ afs_cred_write_ident_data(afs_dlg_data * d) { khc_close_space(h_acell); } - if (h_cellmap) { - khc_write_string(h_cellmap, - l->rows[i].cell, - idname); + if (l->rows[i].flags & DLGROW_FLAG_DONE) { + if (h_cellmap) { + khc_write_string(h_cellmap, + l->rows[i].cell, + idname); + } } } @@ -2260,6 +2303,8 @@ afs_cred_write_ident_data(afs_dlg_data * d) { if (id_cell) khc_write_multi_string(h_afs, L"Cells", id_cell); + else + khc_write_multi_string(h_afs, L"Cells", L"\0"); if (d->config_dlg) { if (d->dirty) diff --git a/src/WINNT/netidmgr_plugin/afsnewcreds.h b/src/WINNT/netidmgr_plugin/afsnewcreds.h index 091caf107..6eb69dd12 100644 --- a/src/WINNT/netidmgr_plugin/afsnewcreds.h +++ b/src/WINNT/netidmgr_plugin/afsnewcreds.h @@ -56,6 +56,10 @@ typedef struct tag_afs_cred_row { /* tokens for this cell exist and are expired */ #define DLGROW_FLAG_EXPIRED 0x00000040 +/* this cell was added because it was listed in the identity + configuration */ +#define DLGROW_FLAG_CONFIG 0x00000080 + /* the subitem indexes for each data field */ enum afs_ncwnd_subitems { NCAFS_IDX_CELL=0, -- 2.39.5