Commit d032bd8f authored by David Byers's avatar David Byers

Server

        Fixed permission-checking bugs
        Fixed some potential core dumps.
        Fixed regexp matching with collate table.

Testing
        Test cases for regexp matching with collate table.
        Fixed old test cases to function with collate table regexp
        matching is on by default.
parent 7ab4425f
1999-06-05 David Byers <davby@ida.liu.se>
* regexp-match-cov.exp: New test case.
* src/server/regex-match.c (lookup_regexp): Don't use strdup to
copy the collate table.
* src/server/text.c (modify_text_info): Check for read access on
text.
* src/server/conference.c (modify_conf_info): Check for secret
confs.
1999-05-31 David Byers <davby@ida.liu.se>
* src/server/text.c (add_comment): Moved access checks to avoid
leaking information.
(add_footnote): Same here.
* src/server/membership.c (set_membership_type): Move permission
checks to avoid leaking information.
* doc/lyskomd.texi (Adding a New Protocol Request): Added notes on
CHK_CONNECTION, CHK_LOGIN and how to avoid leaking secret
information.
* src/server/text.c (add_recipient): Moved GET_X_STAT and
permission checks around to avoid leaking secret information.
* src/server/text-garb.c (start_garb): Same here.
* src/server/debug.c (get_memory_info): Call CHK_CONNECTION.
(backdate_text): Same here.
* src/server/aux-items.c (query_predefined_aux_items): Call
CHK_CONNECTION.
* src/server/admin.c (get_info_old): Call CHK_CONNECTION.
(get_version_info): Same here.
(set_info): Same here.
(set_motd_of_lyskom): Same here.
(sync_kom): Same here.
* src/server/session.c (get_time): Call CHK_CONNECTION.
(who_is_on_old): Same here.
* src/server/text.c (local_to_global): Moved CHK_CONNECTION to
very start of function.
(get_last_text): Call CHK_CONNECTION.
* src/server/person.c (set_priv_bits): Call CHK_CONNECTION.
* src/server/session.c (get_client_version): Call CHK_CONNECTION.
(get_client_name): Same here.
(get_session_info): Same here.
(get_session_info_ident): Same here.
(get_static_session_info): Same here.
1999-06-04 Per Cederqvist <ceder@lysator.liu.se>
Clean up file inclusion. Avoid including files from .h files.
......
......@@ -3,6 +3,18 @@ server.
* Showstoppers
** Testing
*** No test case for failed locking or failed unlocking of database
*** No test case for attempting to garbage collect a text that has an
aux item with a dont-garb flag. No test for cc-recpt preventing
garbage collection that would be allowed by recpt. Ditto for
bcc-recpt. No test for cc-recpt that permits garbage collection
when recpt also does. No test for sent-at preventing garbage
collection.
** We still leak secrets
*** login, login_old
......@@ -44,9 +56,6 @@ server.
information by returning error for deleted texts and non-error for
all other texts.
*** modify_conf_info
No check for secret confs. Might permit modification of any conf.
*** lookup_name and similar functions
Not really checked.
......@@ -101,11 +110,6 @@ server.
No access check on conf before attempting to locate membership. We
can use this to map out all secret conferences.
*** modify_text_info
No check for read access on text before check if we can delete
items. This can be used to map out all secret texts. Deletion
proceeds even for secret texts. This is bad, bad, bad.
*** sub_footnote
*** sub_comment
No read access check on texts before checking if they are
......@@ -125,12 +129,16 @@ server.
Returns secret persons and secret working conferences. No
filtering is done on the result.
** Regex matching with the collate table does not work.
The test for param.regexps_use_collate_table is inverted.
It doesn't work anyway. Results are *strange*.
* High priority, but they can wait until after the next release.
** modify_conf_info can operate on an rd_protected conf.
Perhaps we should have a flag that says an item can be added only
by members of the conference?
** get_conf_stat returns aux_items for re_prot confs.
We should almost certainly have a way of filtering out some aux
items so they are not visible to non-members.
** Remove src/include/compiler.h and use autoconf tests instead.
Is NORETURN used where it should be used?
......@@ -857,6 +865,21 @@ server.
** Add CHK_CONNECTION to remaining RPC handlers.
DONE
*** modify_conf_info
No check for secret confs. Might permit modification of any conf.
FIXED.
*** modify_text_info
No check for read access on text before check if we can delete
items. This can be used to map out all secret texts. Deletion
proceeds even for secret texts. This is bad, bad, bad.
FIXED.
** Regex matching with the collate table does not work.
The test for param.regexps_use_collate_table is inverted.
It doesn't work anyway. Results are *strange*.
FIXED.
* In progress
......
\input texinfo
@c $Id: lyskomd.texi,v 1.24 1999/05/24 09:33:55 ceder Exp $
@c $Id: lyskomd.texi,v 1.25 1999/06/05 12:18:20 byers Exp $
@c %**start of header
@setfilename lyskomd.info
@include version.texi
......@@ -1320,6 +1320,9 @@ base. Most of the time this is the only complete database file!
* Adding Configuration Parameters:: How to add configuration options.
* Adding Asynchronous Messages:: Adding protocol A messages.
* Adding a New Protocol Request:: Adding protocol A calls.
* Adding New Input Types::
* Adding New Result Types::
* Modifying Output Types::
* Adding Aux-Item Types:: Adding predefined aux item types.
* Modifying Stored Types:: Modifying types stored in the DB.
* Notes:: Mixed notes.
......@@ -1702,19 +1705,70 @@ not logged on) and @file{03.exp} (normal behavior.)
@item Run the testsuite to make sure nothing old has been broken.
@end enumerate
It is no longer necessary to update @code{prot_a_is_legal_fnc} since it
is generated automatically.
Every request handler should call the @code{CHK_CONNECTION} macro before
doing anything else. This ensures that the @code{active_connection}
variable is non-NULL. The only time when this might not be the case is
if the request handler is not called in response to a client request.
This should never happen, but might if someone gets careless.
If your function should not be available before the user is logged in,
call the CHK_LOGIN macro after doing @code{CHK_CONNECTION}.
@menu
* Adding New Input Types:: Procedure for adding input types.
* Adding New Result Types:: Procedure for adding output types.
* Modifying Output Types:: Procedure for modifying output types.
@end menu
Take care returning errors to the client. Previous versions of
@samp{lyskomd} leaked secret information through error returns. For
example, the following code leaks information:
@example
Success service(Conf_no conf_no, Text_no text_no)
@{
Conference *conf_stat;
Text_stat *text_stat;
CHK_LOGIN(FAILURE);
GET_C_STAT(conf_stat, conf_no, FAILURE);
GET_T_STAT(text_stat, text_no, FAILURE);
if (access_perm(conf_no, conf_stat, active_connection <= none))
@{
err_stat = conf_no;
kom_errno = KOM_UNDEF_CONF;
return FAILURE;
@}
if (!text_read_access(active_connection, text_no, text_stat))
@{
err_stat = text_no;
kom_errno = KOM_NO_SUCH_TEXT;
return FAILURE;
@}
/* Permissions checked. Do the deed. */
return OK;
@}
@end example
This request can be used to gain precise information on which
conferences and texts exist in the system. If an unprivileged user makes
a request for any conference and readable text, and the user receives a
@samp{KOM_NO_SUCH_TEXT} error, the user can deduce that the conference
exists, but is secret. If the user makes a request for a conference
known to be secret and a text known not to be readable (either secret or
deleted), and the user receives a @samp{KOM_UNDEF_CONF} error, the user
can deduce that the text does exist.
To avoid traps like these, do permission checks for objects immediately
after attempting to get them from the database.
See also:
@itemize @bullet
@item @ref{Adding New Input Types}
@item @ref{Adding New Result Types}
@item @ref{Modifying Output Types}
@end itemize
@node Adding New Input Types
@subsection Adding New Input Types
@section Adding New Input Types
Changes need to be made in the following files:
......@@ -1750,7 +1804,7 @@ Initialize the contents of the field in @code{init_connection}.
@node Adding New Result Types
@subsection Adding New Result Types
@section Adding New Result Types
Changes need to be made in the following files:
......@@ -1773,7 +1827,7 @@ to a connection. Use the existing functions as templates.
@node Modifying Output Types
@subsection Modifying Output Types
@section Modifying Output Types
When you modify an existing type you have to rename the old version of
the type since it will still be used in existing calls. The convention
......
/*
* $Id: conference.c,v 0.52 1999/06/03 22:11:05 ceder Exp $
* $Id: conference.c,v 0.53 1999/06/05 12:19:03 byers Exp $
* Copyright (C) 1991-1999 Lysator Academic Computer Association.
*
* This file is part of the LysKOM server.
......@@ -35,7 +35,7 @@
static const char *
rcsid = "$Id: conference.c,v 0.52 1999/06/03 22:11:05 ceder Exp $";
rcsid = "$Id: conference.c,v 0.53 1999/06/05 12:19:03 byers Exp $";
#include "rcs.h"
USE(rcsid);
......@@ -502,6 +502,7 @@ Conf_no
create_conf_old(const String name,
Conf_type type)
{
/* CHK_CONNECTION in create_conf_generic */
return create_conf_generic(name, type, NULL);
}
......@@ -512,6 +513,7 @@ create_conf(const String name,
{
Conf_no conf;
/* CHK_CONNECTION in create_conf_generic */
conf = create_conf_generic(name, type, aux);
if (conf != 0)
{
......@@ -1381,6 +1383,12 @@ modify_conf_info(Conf_no conf_no,
}
GET_C_STAT(conf, conf_no, FAILURE);
if (fast_access_perm(conf_no, active_connection) <= none)
{
err_stat = conf_no;
kom_errno = KOM_UNDEF_CONF;
return FAILURE;
}
/* Check if we may delete and add the items */
......
/*
* $Id: debug.c,v 1.2 1999/05/24 09:34:24 ceder Exp $
* $Id: debug.c,v 1.3 1999/06/05 12:19:03 byers Exp $
* Copyright (C) 1991, 1993-1996, 1999 Lysator Academic Computer Association.
*
* This file is part of the LysKOM server.
......@@ -33,7 +33,7 @@
#endif
static const char *
rcsid = "$Id: debug.c,v 1.2 1999/05/24 09:34:24 ceder Exp $";
rcsid = "$Id: debug.c,v 1.3 1999/06/05 12:19:03 byers Exp $";
#include "rcs.h"
USE(rcsid);
......@@ -59,6 +59,8 @@ get_memory_info (Memory_info *result)
{
struct mallinfo info;
CHK_CONNECTION(FAILURE);
#ifdef HAVE_MALLINFO
info = mallinfo();
result->arena = info.arena;
......@@ -117,4 +119,6 @@ backdate_text(Text_no text_no,
return OK;
}
/* start_garb is in text-garb.c since it needs access to static variables */
#endif
/*
* $Id: manipulate.h,v 0.23 1999/05/31 12:17:08 byers Exp $
* $Id: manipulate.h,v 0.24 1999/06/05 12:19:04 byers Exp $
* Copyright (C) 1991-1994, 1996-1999 Lysator Academic Computer Association.
*
* This file is part of the LysKOM server.
......@@ -23,7 +23,7 @@
* Please mail bug reports to bug-lyskom@lysator.liu.se.
*/
/*
* $Id: manipulate.h,v 0.23 1999/05/31 12:17:08 byers Exp $
* $Id: manipulate.h,v 0.24 1999/06/05 12:19:04 byers Exp $
*
* manipulate.h
*
......@@ -56,8 +56,6 @@
{ \
if ( !active_connection || !ACTPERS ) \
{ \
if (!active_connection) \
kom_log("CHK_LOGIN: active_connection == NULL"); \
err_stat = 0; \
kom_errno = KOM_LOGIN; \
return errortype; \
......
/*
* $Id: membership.c,v 0.47 1999/06/03 22:11:07 ceder Exp $
* $Id: membership.c,v 0.48 1999/06/05 12:19:05 byers Exp $
* Copyright (C) 1991-1999 Lysator Academic Computer Association.
*
* This file is part of the LysKOM server.
......@@ -38,7 +38,7 @@
#define DEBUG_MARK_AS_READ
static const char *
rcsid = "$Id: membership.c,v 0.47 1999/06/03 22:11:07 ceder Exp $";
rcsid = "$Id: membership.c,v 0.48 1999/06/05 12:19:05 byers Exp $";
#include "rcs.h"
USE(rcsid);
......@@ -948,8 +948,8 @@ add_member_common(Conf_no conf_no,
CHK_CONNECTION(FAILURE);
CHK_LOGIN(FAILURE);
GET_C_STAT(conf_c, conf_no, FAILURE);
GET_C_STAT(pers_c, pers_no, FAILURE);
GET_P_STAT(pers_p, pers_no, FAILURE);
GET_C_STAT(pers_c, pers_no, FAILURE);
/* Force invitation bit if not adding as admin or supervisor */
if (param.invite_by_default &&
......@@ -1047,6 +1047,7 @@ add_member_old(Conf_no conf_no,
{
Membership_type type;
/* CHK_CONNECTION in add_member_common */
set_membership_type_bits(&type, 0,0,0,0,0,0,0,0);
return add_member_common(conf_no, pers_no, priority, where, &type, TRUE);
}
......@@ -1060,6 +1061,7 @@ add_member(Conf_no conf_no,
Membership_type *type
)
{
/* CHK_CONNECTION in add_member_common */
return add_member_common(conf_no, pers_no, priority, where, type, FALSE);
}
......@@ -1369,6 +1371,7 @@ get_membership_old (Pers_no pers_no,
long i;
/* CHK_CONNECTION in do_get_membership */
result = do_get_membership (pers_no,
first,
no_of_confs,
......@@ -1399,6 +1402,7 @@ get_membership (Pers_no pers_no,
Membership_list * memberships
)
{
/* CHK_CONNECTION in do_get_membership */
return do_get_membership (pers_no,
first,
no_of_confs,
......@@ -1493,6 +1497,7 @@ get_members (Conf_no conf_no,
Member_list *members
)
{
/* CHK_CONNECTION in do_get_members */
return do_get_members(conf_no,
first,
no_of_members,
......@@ -1505,6 +1510,7 @@ get_members_old (Conf_no conf_no,
unsigned short no_of_members,
Member_list * members)
{
/* CHK_CONNECTION in do_get_members */
return do_get_members (conf_no,
first,
no_of_members,
......@@ -1640,12 +1646,8 @@ extern Success set_membership_type(Pers_no pers_no,
/* Find the conference and person in question */
GET_C_STAT(conf_c, conf_no, FAILURE);
GET_C_STAT(pers_c, pers_no, FAILURE);
GET_P_STAT(pers_p, pers_no, FAILURE);
/* Make sure that ACTPERS may know about conf */
acc = access_perm(conf_no, conf_c, active_connection);
if (acc == error)
{
return FAILURE;
......@@ -1658,6 +1660,8 @@ extern Success set_membership_type(Pers_no pers_no,
return FAILURE;
}
GET_C_STAT(pers_c, pers_no, FAILURE);
GET_P_STAT(pers_p, pers_no, FAILURE);
/* Check that ACTPERS may modify memberships of person */
acc = access_perm(pers_no, pers_c, active_connection);
if (acc != unlimited &&
......
/*
* $Id: person.c,v 0.53 1999/06/03 22:11:08 ceder Exp $
* $Id: person.c,v 0.54 1999/06/05 12:19:06 byers Exp $
* Copyright (C) 1991-1999 Lysator Academic Computer Association.
*
* This file is part of the LysKOM server.
......@@ -35,7 +35,7 @@
#endif
static const char *
rcsid = "$Id: person.c,v 0.53 1999/06/03 22:11:08 ceder Exp $";
rcsid = "$Id: person.c,v 0.54 1999/06/05 12:19:06 byers Exp $";
#include "rcs.h"
USE(rcsid);
......@@ -460,6 +460,7 @@ extern Success
mark_text_old(Text_no text_no,
u_char mark_type)
{
/* CHK_CONNECTION in mark_text and unmark_text */
if (mark_type == 0)
return unmark_text(text_no);
else
......@@ -702,6 +703,7 @@ create_person_old(const String name,
{
Personal_flags flags;
/* CHK_CONNECTION in create_person_generic */
init_personal_flags(&flags);
return create_person_generic(name, passwd, NULL, flags, TRUE);
}
......@@ -714,6 +716,7 @@ create_person(const String name,
{
Pers_no pers;
/* CHK_CONNECTION in create_person_generic */
pers = create_person_generic(name, passwd, conf_aux, flags, FALSE);
if (pers != 0)
{
......@@ -921,8 +924,9 @@ extern Success
set_priv_bits( Pers_no person,
Priv_bits privileges )
{
Person *p;
Person *p;
CHK_CONNECTION(FAILURE);
CHK_LOGIN(FAILURE);
GET_P_STAT(p, person, FAILURE);
......@@ -1066,6 +1070,7 @@ query_read_texts(Pers_no victim,
Conf_no conf_no,
Membership * result) /* Points to area to store result in */
{
/* CHK_CONNECTION in do_query_read_texts */
return do_query_read_texts(victim,
conf_no,
result);
......@@ -1077,6 +1082,7 @@ query_read_texts_old(Pers_no victim,
Conf_no conf_no,
Membership *result)
{
/* CHK_CONNECTION in do_query_read_texts */
return do_query_read_texts(victim, conf_no, result);
}
......
/*
* $Id: regex-match.c,v 1.23 1999/06/03 22:11:12 ceder Exp $
* $Id: regex-match.c,v 1.24 1999/06/05 12:19:07 byers Exp $
* Copyright (C) 1992-1999 Lysator Academic Computer Association.
*
* This file is part of the LysKOM server.
......@@ -33,7 +33,7 @@
#endif
static const char *
rcsid = "$Id: regex-match.c,v 1.23 1999/06/03 22:11:12 ceder Exp $";
rcsid = "$Id: regex-match.c,v 1.24 1999/06/05 12:19:07 byers Exp $";
#include "rcs.h"
USE(rcsid);
......@@ -44,6 +44,9 @@ USE(rcsid);
#ifdef HAVE_STDLIB_H
# include <stdlib.h>
#endif
#ifdef HAVE_STRING_H
# include <string.h>
#endif
#include "s-string.h"
#include "misc-types.h"
......@@ -81,16 +84,24 @@ lookup_regexp (Connection *conn,
re_syntax_options = RE_SYNTAX_GREP;
if (param.regex_use_collate_table)
if (!param.regex_use_collate_table)
{
pat_buf.translate = NULL;
}
else
{
/* regfree() uses free() to free this memory. The match will
be performed without the translation table if strdup fails.
That is an acceptable fail-safe mode. */
pat_buf.translate = strdup(DEFAULT_COLLAT_TAB);
/* regfree() uses free() to free this memory, so we have to
use malloc to allocate it. If malloc fails we will continue
without the collate table. This is an acceptable failure
mode */
char *tmp = malloc(COLLAT_TAB_SIZE);
if (tmp)
{
memcpy(tmp, DEFAULT_COLLAT_TAB, COLLAT_TAB_SIZE);
}
pat_buf.translate = tmp;
}
pat_buf.fastmap = 0;
......
/*
* $Id: session.c,v 0.51 1999/06/03 22:11:13 ceder Exp $
* $Id: session.c,v 0.52 1999/06/05 12:19:08 byers Exp $
* Copyright (C) 1991-1994, 1996-1999 Lysator Academic Computer Association.
*
* This file is part of the LysKOM server.
......@@ -34,7 +34,7 @@
#endif
static const char *
rcsid = "$Id: session.c,v 0.51 1999/06/03 22:11:13 ceder Exp $";
rcsid = "$Id: session.c,v 0.52 1999/06/05 12:19:08 byers Exp $";
#include "rcs.h"
USE(rcsid);
......@@ -648,6 +648,7 @@ get_session_info (Session_no session_no,
{
Connection *cptr;
CHK_CONNECTION(FAILURE);
CHK_LOGIN(FAILURE);
cptr = get_conn_by_number(session_no);
......@@ -682,6 +683,7 @@ get_static_session_info (Session_no session_no,
{
Connection *cptr;
CHK_CONNECTION(FAILURE);
CHK_LOGIN(FAILURE);
cptr = get_conn_by_number(session_no);
......@@ -711,6 +713,7 @@ get_session_info_ident (Session_no session_no,
{
Connection *cptr;
CHK_CONNECTION(FAILURE);
CHK_LOGIN(FAILURE);
cptr = get_conn_by_number(session_no);
......@@ -797,6 +800,7 @@ who_is_on_old( Who_info_list_old *result )
Session_no session;
CHK_CONNECTION(FAILURE);
session = traverse_connections(0);
while (session != 0)
{
......@@ -840,6 +844,7 @@ who_is_on_old( Who_info_list_old *result )
extern Success
get_time( time_t *clk )
{
CHK_CONNECTION(FAILURE);
*clk = current_time;
return OK;
......@@ -891,6 +896,7 @@ get_client_name (Session_no session_no,
{
Connection *cptr;
CHK_CONNECTION(FAILURE);
CHK_LOGIN(FAILURE);
cptr = get_conn_by_number(session_no);
......@@ -914,6 +920,7 @@ get_client_version (Session_no session_no,
{
Connection *cptr;
CHK_CONNECTION(FAILURE);
CHK_LOGIN(FAILURE);
cptr = get_conn_by_number(session_no);
......
......@@ -63,7 +63,9 @@
# (This is client 2)
read_versions
lyskomd_start
lyskomd_start "" "\
Regexps use collate table: off
"
# Preamble: create a "typical database" containing some persons and
# texts created with traditional calls, and some created using newer
......
# Test suite for lyskomd.
# Copyright (C) 1999 Lysator Academic Computer Association.
#
# This file is part of the LysKOM server.
#
# LysKOM is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 1, or (at your option)
# any later version.
#
# LysKOM is distributed in the hope that it will be useful, but WITHOUT
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
# for more details.
#
# You should have received a copy of the GNU General Public License
# along with LysKOM; see the file COPYING. If not, write to
# Lysator, c/o ISY, Linkoping University, S-581 83 Linkoping, SWEDEN,
# or the Free Software Foundation, Inc., 675 Mass Ave, Cambridge,
# MA 02139, USA.
#
# Please mail bug reports to bug-lyskom@lysator.liu.se.
# Additional test cases for session.c
source "$srcdir/config/prot-a.exp"
read_versions
proc startup {onoff} {
lyskomd_start "" "\
Regexps use collate table: $onoff
"
client_start 0
talk_to client 0
kom_connect "DejaGnu test suite"
kom_accept_async "0 { }";
kom_login 5 "gazonk" 0
kom_create_person "QERSON 6" "" "00000000" "0 { }"
kom_create_person "qerson 7" "" "00000000" "0 { }"
kom_create_person "QersoN 8" "" "00000000" "0 { }"
kom_create_person "CONFERENCE 9" "" "00000000" "0 { }"
kom_create_person "conference 10" "" "00000000" "0 { }"
kom_create_person "ConferencE 11" "" "00000000" "0 { }"
kom_create_conference "CONFERENCE 12" "00000000" "0 { }"
kom_create_conference "conference 13" "00000000" "0 { }"
kom_create_conference "ConferencE 14" "00000000" "0 { }"
kom_create_conference "QERSON 15" "00000000" "0 { }"
kom_create_conference "qerson 16" "00000000" "0 { }"
kom_create_conference "QersoN 17" "00000000" "0 { }"
}
proc shutdown {} {
kom_logout
kom_login 5 "gazonk" 0
kom_enable 255
send "9999 44 0\n"
simple_expect "=9999"
client_death 0
lyskomd_death
}
startup "off"
send "1000 74 [holl "Q.*N"] 0 0\n"
simple_expect "=1000 0 \\*"
send "1001 74 [holl "Q.*N"] 0 1\n"
simple_expect "=1001 2 { [holl "QERSON 15"] 0000 15 [holl "QersoN 17"] 0000 17 }"
send "1002 74 [holl "Q.*N"] 1 0\n"
simple_expect "=1002 2 { [holl "QERSON 6"] 1001 6 [holl "QersoN 8"] 1001 8 }"
send "1003 74 [holl "Q.*N"] 1 1\n"
simple_expect "=1003 4 { [holl "QERSON 6"] 1001 6 [holl "QersoN 8"] 1001 8 [holl "QERSON 15"] 0000 15 [holl "QersoN 17"] 0000 17 }"