summaryrefslogtreecommitdiffstats
path: root/src/plugins
Commit message (Collapse)AuthorAgeFilesLines
* Correct spellingBen Kaduk2014-12-151-7/+7
| | | | | Remove extra 'i' from "create_standalone_prinicipal". While here, pick a slightly shorter name for the variable.
* Add helper for freeing arrays of berval pointersBen Kaduk2014-12-151-11/+21
| | | | | This eliminates a potential leak of the bv_val members from krb5_encode_krbsecretkey().
* Remove some dead codeBen Kaduk2014-12-151-19/+1
| | | | | | | | The secretkey variable is initialized to NULL and compared against NULL, but never actually set to anything after initialization. Remove the variable and all code that would have executed if it was non-NULL.
* Support keyless principals in LDAP [CVE-2014-5354]Ben Kaduk2014-12-151-8/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Operations like "kadmin -q 'addprinc -nokey foo'" or "kadmin -q 'purgekeys -all foo'" result in principal entries with no keys present, so krb5_encode_krbsecretkey() would just return NULL, which then got unconditionally dereferenced in krb5_add_ber_mem_ldap_mod(). Apply some fixes to krb5_encode_krbsecretkey() to handle zero-key principals better, correct the test for an allocation failure, and slightly restructure the cleanup handler to be shorter and more appropriate for the usage. Once it no longer short-circuits when n_key_data is zero, it will produce an array of length two with both entries NULL, which is treated as an empty list by the LDAP library, the correct behavior for a keyless principal. However, attributes with empty values are only handled by the LDAP library for Modify operations, not Add operations (which only get a sequence of Attribute, with no operation field). Therefore, only add an empty krbprincipalkey to the modlist when we will be performing a Modify, and not when we will be performing an Add, which is conditional on the (misspelled) create_standalone_prinicipal boolean. CVE-2014-5354: In MIT krb5, when kadmind is configured to use LDAP for the KDC database, an authenticated remote attacker can cause a NULL dereference by inserting into the database a principal entry which contains no long-term keys. In order for the LDAP KDC backend to translate a principal entry from the database abstraction layer into the form expected by the LDAP schema, the principal's keys are encoded into a NULL-terminated array of length-value entries to be stored in the LDAP database. However, the subroutine which produced this array did not correctly handle the case where no keys were present, returning NULL instead of an empty array, and the array was unconditionally dereferenced while adding to the list of LDAP operations to perform. Versions of MIT krb5 prior to 1.12 did not expose a way for principal entries to have no long-term key material, and therefore are not vulnerable. CVSSv2 Vector: AV:N/AC:M/Au:S/C:N/I:N/A:P/E:H/RL:OF/RC:C ticket: 8041 (new) tags: pullup target_version: 1.13.1 subject: kadmind with ldap backend crashes when putting keyless entries
* Fix LDAP misused policy name crash [CVE-2014-5353]Greg Hudson2014-12-151-3/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In krb5_ldap_get_password_policy_from_dn, if LDAP_SEARCH returns successfully with no results, return KRB5_KDB_NOENTRY instead of returning success with a zeroed-out policy object. This fixes a null dereference when an admin attempts to use an LDAP ticket policy name as a password policy name. CVE-2014-5353: In MIT krb5, when kadmind is configured to use LDAP for the KDC database, an authenticated remote attacker can cause a NULL dereference by attempting to use a named ticket policy object as a password policy for a principal. The attacker needs to be authenticated as a user who has the elevated privilege for setting password policy by adding or modifying principals. Queries to LDAP scoped to the krbPwdPolicy object class will correctly not return entries of other classes, such as ticket policy objects, but may return success with no returned elements if an object with the requested DN exists in a different object class. In this case, the routine to retrieve a password policy returned success with a password policy object that consisted entirely of zeroed memory. In particular, accesses to the policy name will dereference a NULL pointer. KDC operation does not access the policy name field, but most kadmin operations involving the principal with incorrect password policy will trigger the crash. Thanks to Patrik Kis for reporting this problem. CVSSv2 Vector: AV:N/AC:M/Au:S/C:N/I:N/A:C/E:H/RL:OF/RC:C [kaduk@mit.edu: CVE description and CVSS score] ticket: 8051 (new) target_version: 1.13.1 tags: pullup
* Use new error message wrapping APIsNicolas Williams2014-12-077-42/+23
| | | | | | | | | | | | | | Define internal names k5_prendmsg and k5_wrapmsg and use them where we amend error messages. This slightly changes the error message when we fail to construct FAST AP-REQ armor, decrypt a FAST reply, or store credentials in a gic_opts output ccache. Adjust the test suite for the latter of those changes. [ghudson@mit.edu: define and use internal names for brevity; pull in test fix from later commit; expand commit message; fix redundant separators in LDAP messages] ticket: 8046
* Remove length limit on PKINIT PKCS#12 promptGreg Hudson2014-10-291-7/+10
| | | | | | | | | | Long pathnames can trigger the 128-byte prompt length limit in pkinit_get_certs_pkcs12. Use asprintf instead of snprintf. Also check the result of the prompter invocation. ticket: 8011 target_version: 1.13.1 tags: pullup
* Autodetect OpenSSL CMS for LibreSSL compatibilitymaurerpe2014-08-151-2/+2
| | | | | | | | | | | | LibreSSL currently does not support CMS, so checking for CMS via OPENSSL_VERSION_NUMBER isn't reliable. Detect CMS support via autoconf instead. [ghudson@mit.edu: clarified commit message; minor style changes] ticket: 7993 (new) target_version: 1.13 tags: pullup
* Fix LDAP key data segmentation [CVE-2014-4345]Tomas Kuthan2014-08-071-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For principal entries having keys with multiple kvnos (due to use of -keepold), the LDAP KDB module makes an attempt to store all the keys having the same kvno into a single krbPrincipalKey attribute value. There is a fencepost error in the loop, causing currkvno to be set to the just-processed value instead of the next kvno. As a result, the second and all following groups of multiple keys by kvno are each stored in two krbPrincipalKey attribute values. Fix the loop to use the correct kvno value. CVE-2014-4345: In MIT krb5, when kadmind is configured to use LDAP for the KDC database, an authenticated remote attacker can cause it to perform an out-of-bounds write (buffer overrun) by performing multiple cpw -keepold operations. An off-by-one error while copying key information to the new database entry results in keys sharing a common kvno being written to different array buckets, in an array whose size is determined by the number of kvnos present. After sufficient iterations, the extra writes extend past the end of the (NULL-terminated) array. The NULL terminator is always written after the end of the loop, so no out-of-bounds data is read, it is only written. Historically, it has been possible to convert an out-of-bounds write into remote code execution in some cases, though the necessary exploits must be tailored to the individual application and are usually quite complicated. Depending on the allocated length of the array, an out-of-bounds write may also cause a segmentation fault and/or application crash. CVSSv2 Vector: AV:N/AC:M/Au:S/C:C/I:C/A:C/E:POC/RL:OF/RC:C [ghudson@mit.edu: clarified commit message] [kaduk@mit.edu: CVE summary, CVSSv2 vector] ticket: 7980 (new) target_version: 1.12.2 tags: pullup
* Disallow unlocked iteration of hash databasesTom Yu2014-08-071-0/+3
| | | | | | | | | | It's not clear whether unlocked iteration over a hash DB2 database will omit unaffected entries if database additions or deletions occur concurrently with the iteration. Avoid this situation by disabling unlocked iteration in the unlikely event that someone is still using a hash database for their KDB. ticket: 7977
* Support unlocked iteration in DB2Tom Yu2014-08-022-23/+174
| | | | | | | | | | Add support to the DB2 KDB back end to optionally release the lock when calling the iterator callback. This prevents the blocking of other processes when dumps of large databases are taking place. Also add support for reversed iteration. ticket: 7977
* Support write locks in DB2 iterationTom Yu2014-08-021-1/+7
| | | | | | | Add support to krb5_db_iterate() for requesting write locks in the DB2 back end. ticket: 7977
* Add flag word to KDB iteration APIsTom Yu2014-08-026-10/+10
| | | | | ticket: 7977 (new) subject: Enable unlocked KDB iteration
* Add kiprop/<master-hostname> during KDB creationNeng Xue2014-08-011-0/+7
| | | | | | | | | | | To reduce the number of steps in the deployment of iprop, create the kiprop/hostname principal for the master KDC during KDB creation. Adjust tests to match the new behavior. [ghudson@mit.edu: clarified commit message; avoided applying kadmin flags/lifetime to kiprop principal] ticket: 7979 (new)
* Simplify kdb5_ldap_util special princ creationGreg Hudson2014-08-011-161/+104
| | | | | In kdb5_ldap_realm.c, factor out special principal creation into three helper functions to reduce the amount of verbiage in kdb5_ldap_create.
* Modify k5buf interfaces for easier useGreg Hudson2014-07-304-12/+12
| | | | | | | | Make struct k5buf less opaque and get rid of k5buf-int.h. Make it easy to initialize a k5buf in an error state so that it can be freed in a cleanup handler. Add a function k5_buf_status which returns 0 or ENOMEM. Remove k5_buf_data and k5_buf_len. Rename k5_free_buf to k5_buf_free. Adjust all callers to match.
* Add SASL support to LDAP KDB moduleGreg Hudson2014-07-193-10/+149
| | | | | | | | | | | | | Add variables for the SASL mechanism, authcid, authzid, and realm. If a SASL mechanism is set, perform an interactive bind with that mechanism. If <sasl/sasl.h> is found at build time, provide the authcid, authzid, and realm in the interaction function, and provide a SASL secret read from the service password file (under the authcid) if we found one. Based on a patch from Zoran Pericic <zpericic@netst.org>. ticket: 7944 (new)
* Modernize some LDAP sourcesGreg Hudson2014-07-1915-1611/+1003
| | | | | | | | | | | | | | | | | | | | | | | | | | | Bring ldap_misc.c up to date with current practices and make limited changes to other files. Of note: * krb5_decode_krbsecretkey was freeing its bvalues argument; make that the caller's responsibility. * Make is_principal_in_realm and has_modify_increment return krb5_boolean, reversing the sense of their results. * Remove broken code path in decode_tl_data when an integer value has a length other than 2 (which should never happen). * Simplify krb5_ldap_readpassword and make it take filename/name parameters instead of an LDAP context. * Make krb5_ldap_bind (renamed to authenticate) responsible for setting a useful error message, so that its caller doesn't assume knowledge of the bind parameters. * Make krb5_ldap_initialize (renamed to initialize_server) responsible for updating the handle list, and remove the otherwise unused krb5_update_ldap_handle. * Remove remaining skeletal certificate support, including the unused has_sasl_external_mech function. * Remove unused krb5_get_containerdn and KDB_TL_CONTAINERDN. * Remove kdb_xdr.h; all of its prototypes were for functions that don't exist in the module or were duplicated in other headers. * Remove krb5_ldap_get_strings and use ldap_get_values directly at its call sites; there was no need to copy the result.
* Rename --with-proxy-tls-impl to --with-tls-implGreg Hudson2014-07-193-6/+6
| | | | | | | | Make the configure option for TLS implementation more generic, in case we use the k5tls module for something other than KDC proxy support. Rename all of the associated symbols for consistency. ticket: 7929
* Move KKDCP OpenSSL code to an internal pluginGreg Hudson2014-07-195-0/+671
| | | | | | | | | | Create an internal pluggable interface "tls" with one in-tree dynamic plugin module named "k5tls". Move all of the OpenSSL calls to the plugin module, and make the libkrb5 code load and invoke the plugin. This way we do not load or initialize libssl unless an HTTP proxy is used. ticket: 7929
* Fix error check in krb5_ldap_parse_principal_nameLukas Slebodnik2014-07-121-1/+1
| | | | | | Test the correct variable for NULL to detect a strdup failure. [ghudson@mit.edu: clarified commit message]
* Remove unused variablesLukas Slebodnik2014-07-123-13/+4
| | | | [ghudson@mit.edu: squashed with similar commits]
* Fix several memory leaks in LDAP KDB modulesGreg Hudson2014-07-127-38/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix memory leaks discovered by running valgrind over kdbtest, and some related leaks. Many of them result from not calling ldap_msgfree after an unsuccessful search (as the OpenLDAP documentation requires) or after an exception following a search, so many of the fixes move or add ldap_msgfree calls to cleanup labels. ldap_osa_free_princ_ent was not used, and could not be used because it frees the container while krb5_lookup_tl_kadm_data uses a caller-allocated container. Change it to leave the container alone, but to correctly destroy xdrs. Use it in krb5_ldap_put_principal where princ_ent was leaked. In krb5_ldap_put_principal, subtreelist is declared twice in interior scopes and not properly freed; move it to function scope and free it up in the cleanup label. Also in krb5_ldap_put_principal, avoiding decoding multiple KBR5_TL_KADM_DATA values (which we don't expect to see) as later decodes would cause earlier decodes to leak. In krb5_encode_krbsecretkey, fix a leak of the krb5_data container and also add an error check when calling asn1_encode_sequence_of_keys; otherwise we would dereference a null pointer if we run out of memory encoding keys (very unlikely). ticket: 7941 (new) target_version: 1.12.2 tags: pullup
* make dependGreg Hudson2014-07-081-9/+14
|
* Include autoconf.h before system headersGreg Hudson2014-07-0824-100/+30
| | | | | | | | | Include autoconf.h (either directly or via proxy) before system headers, so that feature test macros defined there can affect the system namespace. Where include order was changed, eliminate some redundant or unnecessary includes. ticket: 7961
* Simplify usage of strerror_rGreg Hudson2014-07-081-5/+0
| | | | | | | | Take advantage of the strerror_r portability wrapper to simplify code using it. Remove unused macros related to strerror_r in ldap_service_stash.c and plugins.c. ticket: 7961
* Fix error checking in PKINIT authdata creationGreg Hudson2014-06-201-20/+15
| | | | | | | | | | | | | | | | | In create_identifiers_from_stack: check for allocation errors from PKCS7_ISSUER_AND_SERIAL_new and M_ASN1_INTEGER_dup. Use PKCS7_ISSUER_AND_SERIAL_free to more concisely clean up the OpenSSL issuer variable, and make sure that any partially processed value is cleaned up on error. Use calloc to allocate krb5_cas so that all of its pointers are initially nulled, so that free_krb5_external_principal_identifier can operate on it safely in case of error. Eliminate the retval variable as it was not used safely. Rename the error label from "cleanup" to "oom" and separate it from the successful return path (which has nothing to clean up). ticket: 7943 (new) target_version: 1.12.2 tags: pullup
* Tidy up k5-int.h variable name constantsGreg Hudson2014-06-161-1/+1
| | | | | Fix three mismatched constant names, and properly alphabetize and columnize the lists of definitions. No functional changes.
* Remove pkinit_win2k_require_binding optionGreg Hudson2014-06-133-37/+4
| | | | | | | | | | | | When constructing a draft9 PKINIT request, always include KRB5_PADATA_AS_CHECKSUM padata to ask for an RFC 4556 ReplyKeyPack. Do not accept a draft9 ReplyKeyPack in the KDC response. For now, retain the krb5_reply_key_pack_draft9 ASN.1 codec and the KDC support for generating a draft9 ReplyKeyPack when a draft9 PKINIT request does not contain KRB5_PADATA_AS_CHECKSUM. ticket: 7933
* Remove PKINIT longhorn compatibility optionGreg Hudson2014-06-123-239/+45
| | | | | | | | Remove the PKINIT Windows Server 2008 beta compatibility code conditionalized under the "longhorn" variable. It is not required to interoperate with any released version of Windows. ticket: 7934 (new)
* Remove stub pkinit_win2k codeGreg Hudson2014-06-113-7/+0
| | | | | | As contributed, the PKINIT module contained code to read the pkinit_win2k variable, but never used it. Get rid of the structure field and the code to populate it.
* Treat LDAP KrbKey salt field as optionalGreg Hudson2014-06-051-2/+4
| | | | | | | | | | | | | | | | | | | | | | | | | Per the ASN.1 definition, the KrbKey salt field is optional. Since 1.7, we have been treating it as mandatory in the encoder; since 1.11, we have been treating it as mandatory in the decoder. Mostly by luck, we have been encoding a salt type of 0 when key_data_ver is 1, but we really should not be looking at key_data_type[1] or key_data_length[1] in this situation. Treat the salt field as optional in the encoder and decoder. Although the previous commit ensures that we continue to always encode a salt (without any dangerous assumptions about krb5_key_data constructors), this change will allow us to decode key data encoded by 1.6 without salt fields. This also fixes issue #7918, by properly setting key_data_ver to 2 if a salt type but no salt value is present. It is difficult to get the decoder to actually assign 2 to key_data_ver just because the salt field is there, so take care of that in asn1_decode_sequence_of_keys. Adjust kdbtest.c to match the new behavior by setting key_data_ver to 2 in both test keys. ticket: 7919 target_version: 1.12.2 tags: pullup
* Always include salt in LDAP KrbKey encodingGreg Hudson2014-06-051-1/+20
| | | | | | | | | | | | | | | | | | | In the LDAP KDB module, ensure that every krb5_key_data we pass to asn1_encode_sequence_of_keys includes a salt type, for compatibility with the decoder in unpatched krb5 1.11 and 1.12. This is not a behavior change by itself; since 1.7 the encoder has always included a KrbKey salt field because it erroneously treats that field as non-optional. (Luckily, the encoded salt always happens to have salt type 0 because krb5_key_data constructors start with zeroed memory.) The next commit will fix the encoder and decoder to properly treat the KrbKey salt field as optional, so we need this change to ensure that our encodings remain compatible. Also fix the ASN.1 tests to set key_data_ver correctly for the sample test key data. ticket: 7919
* Use k5_setmsgGreg Hudson2014-06-0513-129/+101
| | | | | | Replace most calls to krb5_set_error_message with k5_setmsg for brevity. Leave alone plugin sources where we don't include k5-int.h (mostly PKINIT).
* Remove stub pkinit_mapping_file codeGreg Hudson2014-06-033-7/+0
| | | | | | As contributed, the PKINIT code contained code to read a mapping filename, but never used the resulting structure variable. Get rid of the structure field and the code to populate it.
* Properly handle PKCS11 label in PKINITGreg Hudson2014-05-241-10/+18
| | | | | | | | | | | The CK_TOKEN_INFO label field is defined to be zero-filled, but it may not be zero-terminated if all bytes of the field are used. Use only length-counted operations to process it. Also avoid underrunning the buffer pointer if the label is empty or contains only whitespace. ticket: 7917 target_version: 1.12.2 tags: pullup
* Don't blindly use PKCS11 slot IDs in PKINITGreg Hudson2014-05-241-14/+13
| | | | | | | | | | | Passing invalid slot IDs to C_OpenSession can cause some PKCS #11 implementations (such as the Solaris one) to crash. If a PKINIT identity specifies a slotid, use it to filter the result of C_GetSlotList, but don't try it if it does not appear in the list. ticket: 7916 target_version: 1.12.2 tags: pullup
* Use case insensitive DNS SAN matching in PKINITTomas Kuthan2014-05-191-1/+1
| | | | | | | Matching Subject Alternative Name from certificate with pkinit_kdc_hostname value from krb5.conf should disregard case. ticket: 7913 (new)
* Fix invalid JSON handling in KDC OTP moduleGreg Hudson2014-05-191-0/+2
| | | | | | | | | | | If the OTP configuration for a principal contains invalid JSON, the KDC OTP module calls k5_json_get_tid on a null pointer, causing the KDC process to crash. Fix this bug by checking the return value of k5_json_decode in decode_config_json. ticket: 7912 (new) target_version: 1.12.2 tags: pullup
* In PKINIT, use library initializer for OpenSSLTomas Kuthan2014-04-151-15/+10
| | | | | | | | | | | | | Use a library initializer to prevent multiple threads using PKINIT from concurently initializing OpenSSL functions. For cases where MT-safety is not assured by registering OpenSSL locking callbacks, this significantly lowers the odds of crashes caused by races in OpenSSL initialization. (If OpenSSL initialization functions are called by some other thread directly, crashes are still possible.) [ghudson@mit.edu: simplify code changes and commit message] ticket: 6413
* Use anonymous OIDs in pkinit_crypto_openssl.cGreg Hudson2014-03-251-53/+43
| | | | | | | | | | | | Stop adding OIDs to the global OpenSSL table. It isn't thread-safe (even with locking callbacks registered), and calling OBJ_cleanup could break other uses of OpenSSL. Instead, use anonymous OIDs created with OBJ_txt2oid. Anonymous OIDs need to be managed more careful to avoid double-freeing, so create a copy before calling PKCS7_add_signed_attribute, and don't free the result of pkinit_pkcs7type2oid in cms_contentinfo_create. ticket: 7889
* Stop shadowing id-pkcs7-data OIDGreg Hudson2014-03-252-72/+45
| | | | | | | | | | | pkinit_crypto_openssl.c currently creates a shadow entry for id-pkcs7-data so that OpenSSL will expect to see the corresponding octet string in d.other instead than d.data. This shadowing is very unfriendly to other uses of OpenSSL and we should stop. Eliminate the shadowing and rewrite create_contentinfo so that it sets up the PKCS7 object correctly if the OID is id-pkcs7-data. ticket: 7889
* Improve PKINIT client memory managementGreg Hudson2014-03-183-103/+54
| | | | | | | | | | | | | | | | In pkinit_as_req_create, create and encode stack-allocated auth-pack structures containing only alias pointers, instead of heap-allocated structures containing a mix of alias pointers, owner pointers, and appropriated caller memory. Keep everything we temporarily allocate in separate local variables and free them through those variables. In pa_pkinit_gen_req, use safer memory practices to avoid problems like issue #7878. Free the checksum since pkinit_as_req_create no longer takes ownership it. Remove a broken overly defensive check after calling pkinit_as_req_create. Remove init_krb5_auth_pack and init_krb5_auth_pack_draft9 as they are no longer required.
* Fix unlikely double free in PKINIT client codeGreg Hudson2014-03-181-2/+1
| | | | | | | | | | | | In pa_pkinit_gen_req, if the cleanup handler is reached with non-zero retval and non-null out_data, out_data is freed, then dereferenced, then freed again. This can only happen if one of the small fixed-size malloc requests fails after pkinit_as_req_create succeeds, so it is unlikely to occur in practice. ticket: 7878 (new) target_version: 1.12.2 tags: pullup
* Conditionalize use of LDAP_OPT_DEBUG_LEVELGreg Hudson2014-02-281-0/+2
| | | | | | | | | The LDAP debug level option (#7551) causes a build failure with the Solaris LDAP library, which does not have LDAP_OPT_DEBUG_LEVEL. ticket: 7870 (new) target_version: 1.12.2 tags: pullup
* Assume <stdint.h> and fixed-width typesGreg Hudson2014-02-261-6/+0
| | | | | | | Make unconditional use of <stdint.h> and fixed-width types such as uint32_t. k5-plugin.h doesn't use any special integer types, so remove the conditional include block there. Nothing uses INT64_FMT/UINT64_FMT, so leave those out of k5-platform.h for now.
* Use system dictionary for db2 tests againGreg Hudson2014-02-191-4/+13
| | | | | | | | | | The built-in word list is not long enough for all of the libdb2 tests to run properly. Revert d21a86e47a7cda29225013e08d060095b94b2ee7 and go back to using the system dictionary if we find one. However, omit any lines from the chosen word list which contain non-alphabetical characters. ticket: 7860
* Use TAILQ macros instead of CIRCLEQ in libdb2Greg Hudson2014-02-192-27/+24
| | | | | | | | | The optimizer in gcc 4.8.1 (but not the current gcc head revision) breaks the queue.h CIRCLEQ macros, apparently due to an overzealous strict aliasing deduction. Use TAILQ macros in the libdb2 mpool code instead. ticket: 7860
* Don't use system dictionary files for DB2 testsGreg Hudson2014-02-111-8/+0
| | | | | | | | | The system dictionary may contain entries with punctuation, which can confuse the shell. It's more predictable to always use the word list from the source tree. ticket: 7860 status: open
* Move OTP sockets to KDC_RUN_DIRNathaniel McCallum2014-02-061-1/+1
| | | | | | | | | | | | Some system configurations expect Unix-domain sockets to live under /run or /var/run, and not other parts of /var where persistent application state lives. Define a new directory KDC_RUN_DIR using $runstatedir (new in autoconf 2.70, so fall back to $localstatedir/run if it's not set) and use that for the default socket path. [ghudson@mit.edu: commit message, otp.rst formatting fix] ticket: 7859 (new)