We have a check for symlinks and hardlinks so that we refuse to open
ccaches through symlinks or which have hardlinks. This check is too
strict, checking for `st_nlink != 1`, which runs into trouble when a
ccache is mounted as a file into a container, in which case `stat(2)`
reports it as having zero links.
The fix is to check for `st_nlink > 1`:
- if (sb2.st_nlink != 1) {
+ if (sb2.st_nlink > 1) {
krb5_set_error_message(context, EPERM, N_("Refuses to open hardlinks for caches FILE:%s", ""), filename);
Though I question the utility of the hardlink check. MIT Kerberos
doesn't have it.
As the atomics are signed on AIX, we better try to use the largest
possible max value.
The 'int' API uses 32-bit values for both 32-bit and 64-bit binaries:
typedef int *atomic_p;
int fetch_and_add(atomic_p addr, int value);
The 'long' API uses 32-bit values for 32-bit binaries and 64-bit values
for 64-bit binaries:
typedef long *atomic_l;
long fetch_and_addlp(atomic_l addr, long value);
So we better use the 'long' API in order to avoid any potential
problems with the heim_base_atomic_integer_max magic value, where
INT[32]_MAX would be a little bit low compared to 64-bit pointer space.
Signed-off-by: Stefan Metzmacher <metze@samba.org>
The API looks like this on AIX:
typedef int *atomic_p;
int fetch_and_add(atomic_p addr, int value);
The strange thing is that the xlc compiler ignores missing arguments by
default. (It warns but doesn't fail to compile)
As a result the value argument was just uninitialized memory,
which means that the ref_cnt variable of struct heim_base,
gets unpredictable values during heim_retain() and heim_release(),
resulting in memory leaks.
Signed-off-by: Stefan Metzmacher <metze@samba.org>
The lib/kadm5/test_marshall program allows one to construct and check
encodings for various struct types for which we have
{kadm5,krb5}_{ret,store}_<type>() functions.
Currently supported are:
- krb5_keyblock
- krb5_principal
- krb5_times
- krb5_address
- krb5_addresses
- krb5_authdata
- krb5_creds
- krb5_key_data
- krb5_tl_data
- kadm5_principal_ent_rec
With this we'll be able to a) construct test vectors, b) use those to
drive fuzzing with AFL or other fuzzers.
Add rk_undumptext(), which NUL-terminates the contents it reads.
rk_undumptext(), and now also rk_undumpdata(), can read from regular and
non-regular files (e.g., ttys, pipes, devices, but -on Windows- not
sockets).
This means that `asn1_print` can now read from `/dev/stdin`, which can
be a pipe.
There's a way to set a limit on how much to read from non-regular files,
and that limit defaults to 10MB.
At any rate, the rk_dumpdata(), rk_undumpdata(), and rk_undumptext() functions
really do not belong in lib/roken but in lib/base. There are other utility
functions in lib/roken that don't belong there too. A rationalization of the
split between lib/roken and lib/base is overdue. And while we're at it -lest I
forget- it'd be nice to move all the krb5_storage functions out of lib/krb5 and
into lib/base, as those could come in handy for, e.g., implementing OpenSSH
style certificates and other things outside the krb5 universe.
The bug fixed herein almost certainly means that PKINIT was never
working on Windows, since lib/hx509 uses rk_undumpdata() to read regular
files containing certificates and keys, but then since rk_undumpdata()
was using net_read(), that can't have worked. On Windows net_read()
insists on the FD being a socket, and because of winsock, the namespaces
of socket and file descriptors on Windows are distinct.
hxtool is a very useful command, with a very user-friendly interface, at
least compared to OpenSSL's openssl(1). We should document it better.
Currently there are no manual pages for hxtool(1)'s subcommands, though
their --help message is pretty self-explanatory. Now the hxtool(1) page
provides better clues to the user, including examples.
This will allow us to add a --no-roots option to
hxtool copy-certificate
which is convenient when copying certificate chains from stores that may
include root CA certificates.
We have a CSR authorizer plugin for calling to an IPC service.
In this commit we add test implementation of such a service.
We also remove the simple_csr_authorizer plugin and fold its
functionality into the new test_csr_authorizer functionality.
We use the CSR authorizer system for /get-tgt and /get-tgts because,
well, the CSR authorizer system knows how to deal with principal names
("PKINIT SANs").
The caller of the /get-tgts end-point is a batch API that is meant for
super-user clients that implement orchestration for automation. For
this end-point it's important to be able to return TGTs for just the
requested principals that are authorized rather than fail the whole
request because one principal isn't. A principal might be rejected by
the authorizer if, for example, it's not meant to exist, and that might
be desirable because "synthetic" HDB entries might be configured, and we
might not want principals that don't exist to appear to exist for such
an orchestration service.
The hx509 CSR related functions allow one to mark specific requested
EKUs and SANs as authorized or not. Until now we have simply rejected
all requests that don't have all attributes approved, but for /get-tgts
we need partial request approval. This commit implements partial
request approval for the /get-tgts end-point.
Now we can use heredocs to execute multiple kadmin commands with just
one `execve()`:
```
${kadmin} <<EOF || exit 1
init ...
add ...
...
EOF
```
This will allow tests to use heredocs in setting up an HDB so as to go a
bit faster.
While we no longer have a decoder CHOICE element 0 bug, we did still
have one encode and copy and free that was leading to a memory leak (and
_save trashing) prior to the fix for
asn1: Fix 1-byte leaks in der_copy_octet_string()
This commit fixes that.
We sometimes do things like `memset(&cert, 0, sizeof(cert))` then
`copy_Certificate(&cert, &cert_copy)`, and then we end up leaking a
byte in `der_copy_octet_string()` due to it having this code:
```C
der_copy_octet_string (const heim_octet_string *from, heim_octet_string *to)
{
assert(from->length == 0 || (from->length > 0 && from->data != NULL));
if (from->length == 0)
to->data = calloc(1, 1);
else
to->data = malloc(from->length);
...
}
```
The traces where this happens always involve the `_save` field of
`Name` or `TBSCertificate`.
This code was assuming that length 0 octet strings are expected to have
a non-NULL `data`, probably in case the C library's allocator returns
non-NULL pointers for `malloc(0)`, but then, why not just call
`malloc(0)`? But calling `malloc(0)` would then still lead to this leak
in on such systems.
Now, `der_free_octet_string()` does unconditionally `free()` the
string's `data`, so the leak really is not there but elsewhere, probably
in `lib/asn1/template.c:_asn1_free()`, but it clearly does
`der_free_octet_string()` the `_save` field of types that have it.
5398425c introduced support for propagating ASN.1 default values to the emitted
JSON, but it neglected to quote string values, which caused ASN.1 parsing
errors. Correct this.
Nicolas Williams