Projects/Database Access Layer cleanup
This project page describes ideas for cleaning up the KDB Database Access Layer (DAL). No resources or schedule have been assigned to this project yet. It is likely that this project will be narrowed or split out into sub-projects before implementation.
Contents
Master key encryption
DB2 and LDAP encrypt key entries in a master key, which is obtained either through password entry at KDC/kadmind startup time or from a stash file. Encryption and decryption of key data is currently performed explicitly in the KDC and libkadm5.
It is possible to construct a database plugin which does not encrypt key data in a master key, but it is hackish: the plugin overrides the default fetch_master_key function to return a dummy key, and overrides the default dbekd_encrypt_key_data and dbekd_decrypt_key_data functions to perform no encryption or decryption.
It would perhaps be cleaner if key encryption and decryption were performed underneath the DAL. Since the user interface features associated with master keys necessarily exist above the DAL, we would still need entry points to implement those features.
Jump points (db_set_option and db_invoke)
The functions db_set_option and db_invoke accept a method number and arguments (a void * for db_set_option, and a pair of krb5_data objects for db_invoke) whose interpretation depends on the method number. They are each used for a variety of unrelated purposes.
Both functions should be split out into separate, type-safe interfaces; there is no good reason to combine multiple unrelated operations into a single vtable pointer.
tl_data
The krb5_db_entry structure contains a field named tl_data, which was originally introduced in order to make the DB2 back end extensible.
The TL-data concept should exist below the DAL, and the krb5_db_entry structure should contain the actual fields represented in TL-data.
krb5_key_data
The krb5_key_data structure is very representation-oriented. It contains a "version" (separate from the kvno) which indicates whether or not there is an explicit salt associated with the key, and then the type/length/contents pointers are arrays of two elements, the second of which determines the explicit salt. This structure should be redefined in a more logical fashion, and the conversion to the format used by the DB2 back end should be performed underneath the DAL.
The flags field for db_get_principal does not explicitly indicate whether aliases are okay. (They should be allowed for TGS requests, and for AS requests if the client indicates a willingness to accept them.) Currently the LDAP back end uses a combination of CLIENT_REFERRALS_ONLY and CANONICALIZE to decide whether to return aliases. There should be an explicit flag.
Separation of KDC and admin DAL interfaces
Currently the KDC and libkadm5srv libraries use the same set of interfaces, each requiring overlapping subsets of the functions. It might be cleaner to have separate vtables for the KDC and for administrative access. This would make the required functionality clearer for modules which only wish to target the KDC, and would also clean up situations where functionality in the overlapping functions (like db_get_principal) varies subtly between KDC access and administrative access scenarios.
Miscellaneous cleanup
The db_get_principal function takes "nentries" and "more" parameters which are never productively used. A principal name can only have one associated principal. The db_get_policy function has a similar argument "cnt" which makes equally little sense.
The db_put_principal function takes a list of krb5_db_entry structures. We never use it to put more than one principal, and we have no expectation of needing transactional updates across principals. It should take only a single krb5_db_entry.
Some krb5_db functions are never used, such as krb5_db_set_option. Since we have no compatibility promises for libkdb across releases, these should be inventoried and removed.
There are two different "not supported" error codes used by krb5_db. The LDAP plugin mostly returns KRB5_PLUGIN_OP_NOTSUPP (except in db_invoke), while kdb5.h and hdb use KRB5_KDB_DBTYPE_NOSUP. kdb5_util dump expects to see KRB5_PLUGIN_OP_NOTSUPP from a database which does not support locks. These error codes should be collapsed into one.
There appears to be no precise rule as to whether kdb5.c will check for the nullity of a function pointer, assert non-nullity, or simply use it without checking. At a minimum, kdb5.c should be consistent about whether to assert non-nullity before using a mandatory function, and certain obviously mandatory functions like db_get_principal should not be treated as optional.
krb5_db_iterate returns success if the module does not support iteration. This is not a correct answer; it should return an error instead. (Existing modules all support iteration, so this would not be a functional change for current modules.)
errcode_2_string is not productively used. In db2 it is not implemented; in LDAP it has the overall effect of calling krb5_get_error_message on the error code and then krb5_set_error_message on the result, which is a complicated no-op. This function and release_errcode_string should be removed.
The purpose of krb5_db_alloc and krb5_db_free is unclear. All current modules implement it with realloc() and free(). There are a handful of callers in libkadm5srv and libkdb. It is possible that this is a workaround for an issue involving different implementations of realloc/free being used by libkdb5 and plugin shared objects, but that would only come into play if memory allocated by libkdb5 or libkadm5srv is being freed by the plugin, which suggests a layering violation of some kind.
