[openssl-dev] [openssl.org #4602] Missing accessors
41 views
Skip to first unread message
Kurt Roeckx via RT
Jul 2, 2016, 7:00:09 AM7/2/16
to
Hi,
I received the following bug in debian:
https://bugs.debian.org/829272
I got a lot of bugs filed about packages FTBFS with openssl 1.1.0.
I started to look at some of them, and many of them are due too
structures having been made opaque. In many cases accessors already
exists, but definitely not for all.
Here is a list of accessors I so far have identified as missing. The
filenames given in the "Add to ..." comments below are suggestions
based on where similar functions are defined and implemented.
/* 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);
/* Add to crypto/x509/x509_vfy.c : */
void X509_STORE_CTX_set_get_issuer(X509_STORE_CTX *ctx,
X509_STORE_CTX_get_issuer get_issuer)
{
ctx->get_issuer = get_issuer;
}
X509_STORE_CTX_get_issuer X509_STORE_CTX_get_get_issuer(X509_STORE_CTX *ctx)
{
return ctx->get_issuer;
}
void X509_STORE_CTX_set_check_issued(X509_STORE_CTX *ctx,
X509_STORE_CTX_check_issued check_issued)
{
ctx->check_issued = check_issued;
}
X509_STORE_CTX_check_issued X509_STORE_CTX_get_check_issued(X509_STORE_CTX *ctx)
{
return ctx->check_issued;
}
/* Add to include/openssl/x509v3.h */
void X509_set_extension_flags(X509 *x, uint32_t ex_flags);
void X509_clear_extension_flags(X509 *x, uint32_t ex_flags);
/* Add to crypto/x509v3/v3_purp.c */
void X509_set_extension_flags(X509 *x, uint32_t ex_flags)
{
x->ex_flags |= ex_flags;
}
void X509_clear_extension_flags(X509 *x, uint32_t ex_flags)
{
x->ex_flags &= ~ex_flags;
}
Regarding the new locking. Do I understand it correctly that e.g.
CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE);
should be replaced by something like
CRYPTO_THREAD_write_lock(X509_STORE_get_lock(ctx));
But then the accessors to get hold of the lock objects in the opaque
structs are missing. E.g.
/* Add to some header file */
CRYPTO_RWLOCK *X509_STORE_get_lock(X509_STORE *ctx);
/* Add to some implementation file */
/* Add to crypto/x509/x509_lu.c */
CRYPTO_RWLOCK *X509_STORE_get_lock(X509_STORE *v)
{
return v->lock;
}
Repeat for other relevant classes with locks.
Mattias
--
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4602
Please log in as guest with password guest if prompted
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
I received the following bug in debian:
https://bugs.debian.org/829272
I got a lot of bugs filed about packages FTBFS with openssl 1.1.0.
I started to look at some of them, and many of them are due too
structures having been made opaque. In many cases accessors already
exists, but definitely not for all.
Here is a list of accessors I so far have identified as missing. The
filenames given in the "Add to ..." comments below are suggestions
based on where similar functions are defined and implemented.
/* 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);
/* Add to crypto/x509/x509_vfy.c : */
void X509_STORE_CTX_set_get_issuer(X509_STORE_CTX *ctx,
X509_STORE_CTX_get_issuer get_issuer)
{
ctx->get_issuer = get_issuer;
}
X509_STORE_CTX_get_issuer X509_STORE_CTX_get_get_issuer(X509_STORE_CTX *ctx)
{
return ctx->get_issuer;
}
void X509_STORE_CTX_set_check_issued(X509_STORE_CTX *ctx,
X509_STORE_CTX_check_issued check_issued)
{
ctx->check_issued = check_issued;
}
X509_STORE_CTX_check_issued X509_STORE_CTX_get_check_issued(X509_STORE_CTX *ctx)
{
return ctx->check_issued;
}
/* Add to include/openssl/x509v3.h */
void X509_set_extension_flags(X509 *x, uint32_t ex_flags);
void X509_clear_extension_flags(X509 *x, uint32_t ex_flags);
/* Add to crypto/x509v3/v3_purp.c */
void X509_set_extension_flags(X509 *x, uint32_t ex_flags)
{
x->ex_flags |= ex_flags;
}
void X509_clear_extension_flags(X509 *x, uint32_t ex_flags)
{
x->ex_flags &= ~ex_flags;
}
Regarding the new locking. Do I understand it correctly that e.g.
CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE);
should be replaced by something like
CRYPTO_THREAD_write_lock(X509_STORE_get_lock(ctx));
But then the accessors to get hold of the lock objects in the opaque
structs are missing. E.g.
/* Add to some header file */
CRYPTO_RWLOCK *X509_STORE_get_lock(X509_STORE *ctx);
/* Add to some implementation file */
/* Add to crypto/x509/x509_lu.c */
CRYPTO_RWLOCK *X509_STORE_get_lock(X509_STORE *v)
{
return v->lock;
}
Repeat for other relevant classes with locks.
Mattias
--
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4602
Please log in as guest with password guest if prompted
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Richard Levitte via RT
Jul 7, 2016, 4:07:56 PM7/7/16
to
crypto/include/internal/x509_int.h, I can see a whole lot more function
pointers that are unreachable.
> Regarding the new locking. Do I understand it correctly that e.g.
>
> CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE);
>
> should be replaced by something like
>
> CRYPTO_THREAD_write_lock(X509_STORE_get_lock(ctx));
>
> But then the accessors to get hold of the lock objects in the opaque
> structs are missing. E.g.
>
> /* Add to some header file */
>
> CRYPTO_RWLOCK *X509_STORE_get_lock(X509_STORE *ctx);
>
> /* Add to some implementation file */
>
> /* Add to crypto/x509/x509_lu.c */
>
> CRYPTO_RWLOCK *X509_STORE_get_lock(X509_STORE *v)
> {
> return v->lock;
> }
>
> Repeat for other relevant classes with locks.
Cheers,
Richard
--
Richard Levitte
lev...@openssl.org
Richard Levitte via RT
Jul 7, 2016, 5:29:43 PM7/7/16
to
On Sat Jul 02 10:59:38 2016, ku...@roeckx.be wrote:
> /* 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
> /* 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);
Richard Levitte via RT
Jul 7, 2016, 5:40:48 PM7/7/16
to
On Sat Jul 02 10:59:38 2016, ku...@roeckx.be wrote:
> /* Add to include/openssl/x509v3.h */
>
> void X509_set_extension_flags(X509 *x, uint32_t ex_flags);
> void X509_clear_extension_flags(X509 *x, uint32_t ex_flags);
>
>
> /* Add to crypto/x509v3/v3_purp.c */
>
> void X509_set_extension_flags(X509 *x, uint32_t ex_flags)
> {
> x->ex_flags |= ex_flags;
> }
>
> void X509_clear_extension_flags(X509 *x, uint32_t ex_flags)
> {
> x->ex_flags &= ~ex_flags;
> }
This gives me the heebie jeebies. ex_flags is used a lot internally, and I
>
> void X509_set_extension_flags(X509 *x, uint32_t ex_flags);
> void X509_clear_extension_flags(X509 *x, uint32_t ex_flags);
>
>
> /* Add to crypto/x509v3/v3_purp.c */
>
> void X509_set_extension_flags(X509 *x, uint32_t ex_flags)
> {
> x->ex_flags |= ex_flags;
> }
>
> void X509_clear_extension_flags(X509 *x, uint32_t ex_flags)
> {
> x->ex_flags &= ~ex_flags;
> }
can't begin to imagine the consequences of letting external code manipulate
this. I understand that in some cases, it seems easy and quick, but...
So, if someone else wants to have a go at this and can make something sensible,
please be my guest. Me, I'm backing off from this particular idea.
Cheers,
Richard
Salz, Rich via RT
Jul 7, 2016, 5:41:59 PM7/7/16
to
I think we should ask kurt to ask the original reporter what they need to do.
Richard Levitte via RT
Jul 7, 2016, 6:00:36 PM7/7/16
to
On Sat Jul 02 10:59:38 2016, ku...@roeckx.be wrote:
> /* Add to some header file */
>
> CRYPTO_RWLOCK *X509_STORE_get_lock(X509_STORE *ctx);
>
> /* Add to some implementation file */
>
> /* Add to crypto/x509/x509_lu.c */
>
> CRYPTO_RWLOCK *X509_STORE_get_lock(X509_STORE *v)
> {
> return v->lock;
> }
https://github.com/openssl/openssl/pull/1295
>
> CRYPTO_RWLOCK *X509_STORE_get_lock(X509_STORE *ctx);
>
> /* Add to some implementation file */
>
> /* Add to crypto/x509/x509_lu.c */
>
> CRYPTO_RWLOCK *X509_STORE_get_lock(X509_STORE *v)
> {
> return v->lock;
> }
> Repeat for other relevant classes with locks.
to give out the lock. However, for opaque types that contain function pointers
that leads to user code, I can see scenarios where it does make sense.
For this case, I've only considered X509_STORE.
--
Richard Levitte
lev...@openssl.org
Kurt Roeckx
Jul 7, 2016, 6:42:55 PM7/7/16
to
On Thu, Jul 07, 2016 at 09:40:24PM +0000, Richard Levitte via RT wrote:
> On Sat Jul 02 10:59:38 2016, ku...@roeckx.be wrote:
> On Sat Jul 02 10:59:38 2016, ku...@roeckx.be wrote:
> > /* Add to include/openssl/x509v3.h */
> >
> > void X509_set_extension_flags(X509 *x, uint32_t ex_flags);
> > void X509_clear_extension_flags(X509 *x, uint32_t ex_flags);
> >
> >
> > /* Add to crypto/x509v3/v3_purp.c */
> >
> > void X509_set_extension_flags(X509 *x, uint32_t ex_flags)
> > {
> > x->ex_flags |= ex_flags;
> > }
> >
> > void X509_clear_extension_flags(X509 *x, uint32_t ex_flags)
> > {
> > x->ex_flags &= ~ex_flags;
> > }
>
> This gives me the heebie jeebies. ex_flags is used a lot internally, and I
> can't begin to imagine the consequences of letting external code manipulate
> this. I understand that in some cases, it seems easy and quick, but...
>
> So, if someone else wants to have a go at this and can make something sensible,
> please be my guest. Me, I'm backing off from this particular idea.
Mattias,
> >
> > void X509_set_extension_flags(X509 *x, uint32_t ex_flags);
> > void X509_clear_extension_flags(X509 *x, uint32_t ex_flags);
> >
> >
> > /* Add to crypto/x509v3/v3_purp.c */
> >
> > void X509_set_extension_flags(X509 *x, uint32_t ex_flags)
> > {
> > x->ex_flags |= ex_flags;
> > }
> >
> > void X509_clear_extension_flags(X509 *x, uint32_t ex_flags)
> > {
> > x->ex_flags &= ~ex_flags;
> > }
>
> This gives me the heebie jeebies. ex_flags is used a lot internally, and I
> can't begin to imagine the consequences of letting external code manipulate
> this. I understand that in some cases, it seems easy and quick, but...
>
> So, if someone else wants to have a go at this and can make something sensible,
> please be my guest. Me, I'm backing off from this particular idea.
Can you explain why this is needed, what the code is trying to do?
Kurt
Kurt Roeckx via RT
Jul 7, 2016, 6:43:13 PM7/7/16
to
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4602
Please log in as guest with password guest if prompted
Please log in as guest with password guest if prompted
Richard Levitte via RT
Jul 8, 2016, 2:08:58 AM7/8/16
to
> > /* 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
> >
> > 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
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?
Richard Levitte via RT
Jul 11, 2016, 8:14:42 AM7/11/16
to
On Mon Jul 11 11:34:35 2016, mattias...@physics.uu.se wrote:
>
> 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.
> fre 2016-07-08 klockan 00:42 +0200 skrev Kurt Roeckx:
> > Mattias,
> >
> > Can you explain why this is needed, what the code is trying to do?
> >
> >
> > Kurt
> >
>
> Hi!
> > Mattias,
> >
> > Can you explain why this is needed, what the code is trying to do?
> >
> >
> > Kurt
> >
>
>
> 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.
David Woodhouse
Jul 11, 2016, 9:34:54 AM7/11/16
to
On Mon, 2016-07-11 at 13:08 +0000, Mattias Ellert via RT wrote:
>
>
> 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
>
>
> 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
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
David Woodhouse via RT
Jul 11, 2016, 10:04:49 AM7/11/16
to
--
dwmw2
--
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4602
Please log in as guest with password guest if prompted
--
Richard Levitte via RT
Jul 20, 2016, 11:11:51 AM7/20/16
to
On Mon Jul 11 14:04:22 2016, dw...@infradead.org wrote:
> 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 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.
Is this still true with OpenSSL 1.1? If so, please file a report.
> 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)
https://github.com/openssl/openssl/pull/1294 currently provides a setter for
get_issuer in X509_STORE.
--
Richard Levitte
lev...@openssl.org
--
Richard Levitte via RT
Jul 20, 2016, 11:14:31 AM7/20/16
to
On Mon Jul 11 11:34:35 2016, mattias...@physics.uu.se wrote:
> 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
> 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.
EXFLAG_PROXY flag correctly? In what situations does that happen? That may be
worth a bug report of its own.
Jan Just Keijser
Jul 20, 2016, 12:58:45 PM7/20/16
to
Hi Richard,
On 20/07/16 17:14, Richard Levitte via RT wrote:
> On Mon Jul 11 11:34:35 2016, mattias...@physics.uu.se wrote:
>> 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
On 20/07/16 17:14, Richard Levitte via RT wrote:
> On Mon Jul 11 11:34:35 2016, mattias...@physics.uu.se wrote:
>> 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.
>
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
Jul 20, 2016, 12:59:04 PM7/20/16
to
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4602
Please log in as guest with password guest if prompted
Please log in as guest with password guest if prompted
Richard Levitte via RT
Jul 20, 2016, 1:11:15 PM7/20/16
to
On Wed Jul 20 16:58:20 2016, jan...@nikhef.nl wrote:
> Hi Richard,
>
> On 20/07/16 17:14, Richard Levitte via RT wrote:
> > On Mon Jul 11 11:34:35 2016, mattias...@physics.uu.se wrote:
> >> 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?
I assume you only want that flag set for actual proxy certs a no other. If you
> Hi Richard,
>
> On 20/07/16 17:14, Richard Levitte via RT wrote:
> > On Mon Jul 11 11:34:35 2016, mattias...@physics.uu.se wrote:
> >> 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?
simply want to make sure the certs in a stack are properly flagged by OpenSSL,
call X509_check_purpose for each of them.
> 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,
call that for each X509*.
Incidently, this function calls X509_check_purpose to make sure the caches are
properly built up...
--
Richard Levitte
lev...@openssl.org
Richard Levitte via RT
Jul 21, 2016, 5:52:03 AM7/21/16
to
On Thu Jul 21 08:18:30 2016, mattias...@physics.uu.se wrote:
> ons 2016-07-20 klockan 15:14 +0000 skrev Richard Levitte via RT:
> > On Mon Jul 11 11:34:35 2016, mattias...@physics.uu.se wrote:
> > >
> > > 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
> > lev...@openssl.org
> >
>
> The answer to this is related to Mischa's reply, which unfortunately
> was only sent to the Debian BTS and not the the OpenSSL RT. I quote it
> below. As indicated in the answer, setting the EXFLAG_PROXY allows
> handling non-RFC proxies in OpenSSL.
>
> mån 2016-07-11 klockan 14:53 +0200 skrev Mischa Salle:
> > Hi Richard, Mattias, others,
> >
> > I agree with you that it would be nice if OpenSSL could figure out
> > itself whether a cert needs to be treated as a proxy, but currently
> > that
> > doesn't work reliably as far as I know.
> > The flag is certainly needed in the case of non-RFC3820 proxies, also
> > known as legacy proxies. Unfortunately these are still very widely
> > used
> > (majority of the proxies actually) and hence our code must be able to
> > handle them correctly.
> >
> > Best wishes,
> > Mischa Sallé
> >
> ons 2016-07-20 klockan 15:14 +0000 skrev Richard Levitte via RT:
> > On Mon Jul 11 11:34:35 2016, mattias...@physics.uu.se wrote:
> > >
> > > 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
> > lev...@openssl.org
> >
>
> The answer to this is related to Mischa's reply, which unfortunately
> was only sent to the Debian BTS and not the the OpenSSL RT. I quote it
> below. As indicated in the answer, setting the EXFLAG_PROXY allows
> handling non-RFC proxies in OpenSSL.
>
> mån 2016-07-11 klockan 14:53 +0200 skrev Mischa Salle:
> > Hi Richard, Mattias, others,
> >
> > I agree with you that it would be nice if OpenSSL could figure out
> > itself whether a cert needs to be treated as a proxy, but currently
> > that
> > doesn't work reliably as far as I know.
> > The flag is certainly needed in the case of non-RFC3820 proxies, also
> > known as legacy proxies. Unfortunately these are still very widely
> > used
> > (majority of the proxies actually) and hence our code must be able to
> > handle them correctly.
> >
> > Best wishes,
> > Mischa Sallé
> >
Ok... From looking at the voms code that was linked to earlier, I can see that
legacy proxy certs are recognised by an older OID (called PROXYCERTINFO_V3 in
the code), 1.3.6.1.4.1.3536.1.222. Is there a spec for the extensions in that
version, whether they are critical or not and so on, that I can reach? Or is
the OID the only actual difference? If it's easy enough (and it currently does
look quite easy), I can certainly see adding some code in OpenSSL to recognise
those...
Richard Levitte via RT
Jul 22, 2016, 5:53:56 AM7/22/16
to
On Fri Jul 22 07:38:25 2016, mattias...@physics.uu.se wrote:> As far as I know there are three different kinds of proxies, usually
> called "legacy", "draft" and "rfc", or sometimes version 2, 3 and 4
> respectively.
>
> For example see "grid-proxy-init -help":
>
> -draft Creates a draft (GSI-3) proxy
> -old Creates a legacy globus proxy
> -rfc Creates a RFC 3820 compliant proxy
>
> The really tricky one is the old legacy version 2 proxy I think.
> called "legacy", "draft" and "rfc", or sometimes version 2, 3 and 4
> respectively.
>
> For example see "grid-proxy-init -help":
>
> -draft Creates a draft (GSI-3) proxy
> -old Creates a legacy globus proxy
> -rfc Creates a RFC 3820 compliant proxy
>
> The really tricky one is the old legacy version 2 proxy I think.
Ok, so after doing quite a bit of searching, I found some references to GT2
(old) in a 3.0 document:
http://toolkit.globus.org/toolkit/docs/3.0/gsi/GSI-message-specification-02.doc
(section 5)
As I understand it, the GT2 format is a simple EE cert, with just two
differences:
1. the issuer is also a EE (so it has the basic constraint CA set to false)...
or it's another GT2 proxy, which amounts to the same thing.
2. the subject name is the issuer name plus an appended 'CN=Proxy' or
'CN=LimitedProxy'
Good so far?
My main problem here is GT3 (draft) proxy certs. If I look at
https://tools.ietf.org/html/draft-ietf-pkix-proxy-10, they look exactly the
same as RFC3820 proxy certs, down to the extension OID. If I look at a really
old draft
(http://sigillum.pl/upload/pdf/Internet%20X.509%20Public%20Key%20Infrastructure.%20Proxy%20Certificate%20Profile.pdf),
the difference is *wild*, and if look at a random shell script
(https://www.nikhef.nl/~janjust/proxy-verify/genproxy) I found while searching
for OID 1.3.6.1.4.1.3536.1.222, I find a third variant that has a slightly
different proycertinfo extension...
Btw, this should really become a new ticket, along the lines of "OpenSSL
doesn't recognise earlier proxy certs".
Richard Levitte via RT
Jul 22, 2016, 11:26:46 AM7/22/16
to
In addition to github PR 1294, there's now also PR 1339 which adds the function to set the EXFLAG_PROXY flag on a given certificate.
Also, PR 1295 has been updated. Instead of a function that returns a lock, there is now a lock and an unlock function.
To me, it seems that that covers what's being asked for. Perhaps not exactly (the setters are for X509_STORE only), but should be workable.
(writing this from my mobile, sorry for the lack of github links)
Also, PR 1295 has been updated. Instead of a function that returns a lock, there is now a lock and an unlock function.
To me, it seems that that covers what's being asked for. Perhaps not exactly (the setters are for X509_STORE only), but should be workable.
(writing this from my mobile, sorry for the lack of github links)
Richard Levitte via RT
Jul 22, 2016, 11:56:24 AM7/22/16
to
Hi,
Good point, I'll look into that. Also, thanks for the reminder, that HOWTO needs a rewrite, badly.
Cheers
Richard
On Fri Jul 22 15:51:16 2016, msa...@nikhef.nl wrote:
> Hi,
>
> unless I didn't look careful enough I think we might still be missing
> the current_cert (and current_issuer) from the X509_STORE_CTX, as
> advertised in
> https://github.com/openssl/openssl/blob/master/doc/HOWTO/proxy_certificates.txt#L204
> and used in e.g.
> https://github.com/italiangrid/voms/blob/master/src/sslutils/sslutils.c
> and many other places for verifying the proxy chain or is there a
> better/other solution for that?
>
> Best wishes,
> Mischa
Good point, I'll look into that. Also, thanks for the reminder, that HOWTO needs a rewrite, badly.
Cheers
Richard
On Fri Jul 22 15:51:16 2016, msa...@nikhef.nl wrote:
> Hi,
>
> unless I didn't look careful enough I think we might still be missing
> the current_cert (and current_issuer) from the X509_STORE_CTX, as
> advertised in
> https://github.com/openssl/openssl/blob/master/doc/HOWTO/proxy_certificates.txt#L204
> and used in e.g.
> https://github.com/italiangrid/voms/blob/master/src/sslutils/sslutils.c
> and many other places for verifying the proxy chain or is there a
> better/other solution for that?
>
> Best wishes,
> Mischa
>
> On Fri, Jul 22, 2016 at 03:26:26PM +0000, Richard Levitte via RT
> wrote:
> > In addition to github PR 1294, there's now also PR 1339 which adds
> > the function to set the EXFLAG_PROXY flag on a given certificate.
> >
> > Also, PR 1295 has been updated. Instead of a function that returns a
> > lock, there is now a lock and an unlock function.
> >
> > To me, it seems that that covers what's being asked for. Perhaps not
> > exactly (the setters are for X509_STORE only), but should be
> > workable.
> >
> > (writing this from my mobile, sorry for the lack of github links)
> >
> > --
> > Richard Levitte
> > lev...@openssl.org
> > --
> > Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4602
> > Please log in as guest with password guest if prompted
> >
> > --
> > To unsubscribe, send mail to 829272-un...@bugs.debian.org.
> On Fri, Jul 22, 2016 at 03:26:26PM +0000, Richard Levitte via RT
> wrote:
> > In addition to github PR 1294, there's now also PR 1339 which adds
> > the function to set the EXFLAG_PROXY flag on a given certificate.
> >
> > Also, PR 1295 has been updated. Instead of a function that returns a
> > lock, there is now a lock and an unlock function.
> >
> > To me, it seems that that covers what's being asked for. Perhaps not
> > exactly (the setters are for X509_STORE only), but should be
> > workable.
> >
> > (writing this from my mobile, sorry for the lack of github links)
> >
> > --
> > Richard Levitte
> > lev...@openssl.org
> > --
> > Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4602
> > Please log in as guest with password guest if prompted
> >
> > --
Richard Levitte via RT
Jul 23, 2016, 5:44:43 AM7/23/16
to
To get current_cert, it's X509_STORE_CTX_get_current_cert().
To get current_issuer, it's X509_STORE_CTX_get0_current_issuer()
Those functions are already present in pre-1.1 OpenSSL (at least in the 1.0.2
series)
To get current_issuer, it's X509_STORE_CTX_get0_current_issuer()
Those functions are already present in pre-1.1 OpenSSL (at least in the 1.0.2
series)
Richard Levitte via RT
Jul 25, 2016, 7:47:13 AM7/25/16
to
On Mon Jul 25 11:32:17 2016, msa...@nikhef.nl wrote:
>
> yes, those I know, but the problem is the *setting* of the failing
> cert.
> Since we need to walk the whole chain for the proxy pathlength
> verification, we need to be able to indicate which cert is failing.
> See
> e.g.
> https://github.com/globus/globus-
>
toolkit/blob/globus_6_branch/gsi/callback/source/library/globus_gsi_callback.c#L1691
> and further, in particular line 1731.
> VOMS is basically using the same code
> https://github.com/italiangrid/voms/blob/master/src/sslutils/sslutils.c#L2236
> and further, in particular line 2255.
Is that code to cope with pathlen checking bugs? That's what it looks to me. In
that case, it might no longer be needed with OpenSSL 1.1, along with some other
stuff (the subject checking stuff comes to mind). Quite frankly, I think the
grid source needs a good and hard look over, quite a bit of it shouldn't be
needed any more. The exception is recognising pre-3820 proxy certs.
> Jan Just also sets the current_issuer in his grid-proxy-verify.c,
> http://www.nikhef.nl/~janjust/proxy-verify/
> line 346, but I'm not sure that's necessary.
> Mischa
> On Sat, Jul 23, 2016 at 09:44:18AM +0000, Richard Levitte via RT
> wrote:
> > To get current_cert, it's X509_STORE_CTX_get_current_cert().
> > To get current_issuer, it's X509_STORE_CTX_get0_current_issuer()
>
> Hi Richard,
> wrote:
> > To get current_cert, it's X509_STORE_CTX_get_current_cert().
> > To get current_issuer, it's X509_STORE_CTX_get0_current_issuer()
>
>
> yes, those I know, but the problem is the *setting* of the failing
> cert.
> Since we need to walk the whole chain for the proxy pathlength
> verification, we need to be able to indicate which cert is failing.
> See
> e.g.
> https://github.com/globus/globus-
>
toolkit/blob/globus_6_branch/gsi/callback/source/library/globus_gsi_callback.c#L1691
> and further, in particular line 1731.
> VOMS is basically using the same code
> https://github.com/italiangrid/voms/blob/master/src/sslutils/sslutils.c#L2236
> and further, in particular line 2255.
Is that code to cope with pathlen checking bugs? That's what it looks to me. In
that case, it might no longer be needed with OpenSSL 1.1, along with some other
stuff (the subject checking stuff comes to mind). Quite frankly, I think the
grid source needs a good and hard look over, quite a bit of it shouldn't be
needed any more. The exception is recognising pre-3820 proxy certs.
> Jan Just also sets the current_issuer in his grid-proxy-verify.c,
> http://www.nikhef.nl/~janjust/proxy-verify/
> line 346, but I'm not sure that's necessary.
> Mischa
Salz, Rich
Jul 25, 2016, 8:48:15 AM7/25/16
to
> That's exactly what we currently do, we provide a verification callback, but
> we do need to be able to set the failing cert in a chain for that.
Stick it in EXDAT?
Salz, Rich via RT
Jul 25, 2016, 8:48:35 AM7/25/16
to
> That's exactly what we currently do, we provide a verification callback, but
> we do need to be able to set the failing cert in a chain for that.
Stick it in EXDAT?
--
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4602
Please log in as guest with password guest if prompted
Please log in as guest with password guest if prompted
Salz, Rich via RT
Jul 25, 2016, 9:27:43 AM7/25/16
to
Perhaps the GRID folks can just write their own validation routine completely?
Salz, Rich
Jul 25, 2016, 9:27:44 AM7/25/16
to
Perhaps the GRID folks can just write their own validation routine completely?
--
--
Salz, Rich
Jul 25, 2016, 9:44:46 AM7/25/16
to
I am not sure what to suggest. This conversation is bouncing across two ticket systems and is all about a legacy certificate format that is, what, outdated since 2002?
I am hard-pressed to see why OpenSSL 1.1 has to do anything other than what Richard proposed.
I am hard-pressed to see why OpenSSL 1.1 has to do anything other than what Richard proposed.
Salz, Rich via RT
Jul 25, 2016, 9:45:06 AM7/25/16
to
I am not sure what to suggest. This conversation is bouncing across two ticket systems and is all about a legacy certificate format that is, what, outdated since 2002?
I am hard-pressed to see why OpenSSL 1.1 has to do anything other than what Richard proposed.
--
I am hard-pressed to see why OpenSSL 1.1 has to do anything other than what Richard proposed.
--
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4602
Please log in as guest with password guest if prompted
Please log in as guest with password guest if prompted
Richard Levitte via RT
Jul 25, 2016, 10:28:28 AM7/25/16