[openssl-dev] [openssl.org #4602] Missing accessors

[Richard Levitte via RT]

Mattias,
Can you explain why this is needed, what the code is trying to do?
Kurt

Hi!
The modification of the extension flags happens in at least four
different packages. The modification they do is to add the
EXFLAG_PROXY
bit to the flags.

Ok, I just had a look:
https://sources.debian.net/src/globus-gsi-callback/5.8-2/library/globus_gsi_callback.c/#L692

This looks like an old workaround, and I wonder if it's really needed any more.
If it's still needed, I'd say this may uncover a bug within OpenSSL, but in
that case, I'd rather fix that in 1.1

https://sources.debian.net/src/voms/2.0.13-1/src/sslutils/sslutils.c/#L1665
https://sources.debian.net/src/voms/2.0.13-1/src/sslutils/sslutils.c/#L1740

I see what this code does, it makes a name constraint check that should have
been present in OpenSSL but wasn't... until 1.1. However, there's other stuff
in that function that looks odd..

https://sources.debian.net/src/canl-c/2.1.6-2/src/proxy/sslutils.c/#L1655
https://sources.debian.net/src/canl-c/2.1.6-2/src/proxy/sslutils.c/#L1719

This is the same code as the voms you pointed at above.
https://sources.debian.net/src/nordugrid-arc/5.1.2-1/src/hed/libs/credential/CertUtil.cpp/#L184

This is the same code as the globus-gsi-callback pointer above.

I guess having a more restrictive accessor that only sets the
EXFLAG_PROXY bit could work. I suggested the more general solution of
having set/clear accessors for arbitrary flags since it was - well
more
general.

Mm, I'm really unsure about this one. ex_flags is part of a cache of
information that OpenSSL fiddles with whenever it checks the extensions for a
certificate. Calling anything that ends up calling X509_check_issued(),
X509_check_ca() or X509_check_purpose() will cause values to be checked and
cached for the certificates involved in the call of those functions. In the
proxy certificate case, EXFLAG_PROXY will be set for a certificate any time the
proxyCertInfo is found among its extensions.

To be blunt, I would much rather see a bug report that shows when that cache
isn't being built properly, and possibly a fix for it.

Cheers,
Richard

--
Richard Levitte
***@openssl.org

[Mattias Ellert via RT]

/* Add to include/openssl/x509_vfy.h : */
typedef int (*X509_STORE_CTX_get_issuer)(X509 **issuer,
X509_STORE_CTX
*ctx, X509 *x);
typedef int (*X509_STORE_CTX_check_issued)(X509_STORE_CTX *ctx,
X509
*x, X509 *issuer);
void X509_STORE_CTX_set_get_issuer(X509_STORE_CTX *ctx,
X509_STORE_CTX_get_issuer
get_issuer);
X509_STORE_CTX_get_issuer
X509_STORE_CTX_get_get_issuer(X509_STORE_CTX
*ctx);
void X509_STORE_CTX_set_check_issued(X509_STORE_CTX *ctx,
X509_STORE_CTX_check_issued
check_issued);
X509_STORE_CTX_check_issued
X509_STORE_CTX_get_check_issued(X509_STORE_CTX *ctx);

For this part, https://github.com/openssl/openssl/pull/1294

So, looking at this again after some sleep, there's a part of this
solution
that I'm unsure of, and it all comes back to X509_STORE_CTX_init(),
where the
X509_STORE context gets initialised from the X509_STORE, including
all the
function pointers. This has me wonder if the X509_STORE_CTX setters
should
really be made available (perhaps with the exception of the verify
and
verify_cb ones). Doesn't it make more sense to set those function
pointers when
creating the X509_STORE itself? Why would those functions need to be
changed in
the context?
Cheers,
Richard
--
Richard Levitte

Looking at the various places in the code where get_issuer
and check_issued are accessed, they mostly use the context rather than
the store. Here are the places I have found:

https://sources.debian.net/src/nordugrid-arc/5.1.2-1/src/hed/libs/credential/CertUtil.cpp/#L71

https://sources.debian.net/src/canl-c/2.1.6-2/src/proxy/sslutils.c/#L1581

https://sources.debian.net/src/voms/2.0.13-1/src/sslutils/sslutils.c/#L1588

https://sources.debian.net/src/globus-gsi-callback/5.8-2/library/globus_gsi_callback.c/#L367

https://sources.debian.net/src/globus-gsi-callback/5.8-2/library/globus_gsi_callback.c/#L1059

https://sources.debian.net/src/globus-gsi-credential/7.9-2/library/globus_gsi_cred_handle.c/#L1997

And the following one actually uses the store and not the context:

https://sources.debian.net/src/globus-gssapi-gsi/12.1-1/library/globus_i_gsi_gss_utils.c/#L448

Mattias

[David Woodhouse]

Looking at the various places in the code where get_issuer
and check_issued are accessed, they mostly use the context rather than
https://sources.debian.net/src/nordugrid-arc/5.1.2-1/src/hed/libs/credential/CertUtil.cpp/#L71
https://sources.debian.net/src/canl-c/2.1.6-2/src/proxy/sslutils.c/#L1581
https://sources.debian.net/src/voms/2.0.13-1/src/sslutils/sslutils.c/#L1588
https://sources.debian.net/src/globus-gsi-callback/5.8-2/library/globus_gsi_callback.c/#L367
https://sources.debian.net/src/globus-gsi-callback/5.8-2/library/globus_gsi_callback.c/#L1059
https://sources.debian.net/src/globus-gsi-credential/7.9-2/library/globus_gsi_cred_handle.c/#L1997
https://sources.debian.net/src/globus-gssapi-gsi/12.1-1/library/globus_i_gsi_gss_utils.c/#L448

I was using store.get_issuer() in OpenConnect too, because I need to
manually build the trust chain to include it on the wire — because
even today the server might *still* suffer RT#1942 and fail to trust
our client cert unless we help it by providing the *right* chain.

I've worked around the lack of access to get_issuer() by doing a dummy
call to X509_verify_cert(), throwing away its result and then hoping
that we have something useful in store.chain (which we *can* still
access). That seems to work but I'm not stunningly happy with it; if we
can have an accessor I'd much rather go back to doing it the old way.

http://git.infradead.org/users/dwmw2/openconnect.git/commitdiff/0d635a0
(in workaround_openssl_certchain_bug() in the hunk around line 1306)

[David Woodhouse via RT]

Looking at the various places in the code where get_issuer
and check_issued are accessed, they mostly use the context rather than
https://sources.debian.net/src/nordugrid-arc/5.1.2-1/src/hed/libs/credential/CertUtil.cpp/#L71
https://sources.debian.net/src/canl-c/2.1.6-2/src/proxy/sslutils.c/#L1581
https://sources.debian.net/src/voms/2.0.13-1/src/sslutils/sslutils.c/#L1588
https://sources.debian.net/src/globus-gsi-callback/5.8-2/library/globus_gsi_callback.c/#L367
https://sources.debian.net/src/globus-gsi-callback/5.8-2/library/globus_gsi_callback.c/#L1059
https://sources.debian.net/src/globus-gsi-credential/7.9-2/library/globus_gsi_cred_handle.c/#L1997
https://sources.debian.net/src/globus-gssapi-gsi/12.1-1/library/globus_i_gsi_gss_utils.c/#L448

I was using store.get_issuer() in OpenConnect too, because I need to
manually build the trust chain to include it on the wire — because
even today the server might *still* suffer RT#1942 and fail to trust
our client cert unless we help it by providing the *right* chain.

I've worked around the lack of access to get_issuer() by doing a dummy
call to X509_verify_cert(), throwing away its result and then hoping
that we have something useful in store.chain (which we *can* still
access). That seems to work but I'm not stunningly happy with it; if we
can have an accessor I'd much rather go back to doing it the old way.

http://git.infradead.org/users/dwmw2/openconnect.git/commitdiff/0d635a0
(in workaround_openssl_certchain_bug() in the hunk around line 1306)
--
dwmw2

[Richard Levitte via RT]

I guess having a more restrictive accessor that only sets the
EXFLAG_PROXY bit could work. I suggested the more general solution of
having set/clear accessors for arbitrary flags since it was - well
more
general.

So let me ask this in a different manner, does OpenSSL 1.1 still not set the
EXFLAG_PROXY flag correctly? In what situations does that happen? That may be
worth a bug report of its own.

--
Richard Levitte
***@openssl.org

[Jan Just Keijser]

Hi Richard,

Post by Richard Levitte via RT

I guess having a more restrictive accessor that only sets the
EXFLAG_PROXY bit could work. I suggested the more general solution of
having set/clear accessors for arbitrary flags since it was - well
more
general.

So let me ask this in a different manner, does OpenSSL 1.1 still not set the
EXFLAG_PROXY flag correctly? In what situations does that happen? That may be
worth a bug report of its own.

this ties into my earlier question and example of verifying proxy
certificates. What if I want to explicitly *set* the EXFLAG_PROXY for a
stack of certificates? how would I do that? how can I ensure that
OpenSSL 1.1 will automagically trigger this flag for me? Is there a
'get_*' function to determine which flags were set during certificate
verification?

thanks for any pointers or advice,

JJK / Jan Just Keijser

[Jan Just Keijser via RT]

Hi Richard,

Post by Richard Levitte via RT

I guess having a more restrictive accessor that only sets the
EXFLAG_PROXY bit could work. I suggested the more general solution of
having set/clear accessors for arbitrary flags since it was - well
more
general.

So let me ask this in a different manner, does OpenSSL 1.1 still not set the
EXFLAG_PROXY flag correctly? In what situations does that happen? That may be
worth a bug report of its own.

this ties into my earlier question and example of verifying proxy
certificates. What if I want to explicitly *set* the EXFLAG_PROXY for a
stack of certificates? how would I do that? how can I ensure that
OpenSSL 1.1 will automagically trigger this flag for me? Is there a
'get_*' function to determine which flags were set during certificate
verification?

thanks for any pointers or advice,

JJK / Jan Just Keijser