X509: Multiple OCSP Responders #5231
Conversation
atreiber94
force-pushed
the
feature/multiple_ocsp_responders
branch
from
5b9c3aa to
ad78ab4
Compare
atreiber94
force-pushed
the
feature/multiple_ocsp_responders
branch
from
ad78ab4 to
80da263
Compare
| if(m_ocsp_responders.empty()) { | ||
| return {}; | ||
| } | ||
| return m_ocsp_responders[0]; |
There was a problem hiding this comment.
If we were extremely nitpicky regarding semver this should be m_ocsp_responders.back(), judging from the now-replaced parsing code:
if(oid == OID::from_string("PKIX.OCSP")) {
const BER_Object name = info.get_next_object();
if(name.is_a(6, ASN1_Class::ContextSpecific)) {
m_ocsp_responder = ASN1::to_string(name);
}
}... that did repeatedly overwrite the std::string m_ocsp_responder until settling on the last entry.
There was a problem hiding this comment.
I'd be inclined to say it returned an arbitrary value that until now happened to always be the last one, this was not an intentional thing and also not afaik documented anywhere.
There was a problem hiding this comment.
Intuitively I'd expect the first value to be returned. Since the previous behavior was never documented I'd agree returning the first value would not be a notable semver issue. But I don't have a strong opinion on this, I can also understand returning the last value for consistent behavior.
| if(data().m_ocsp_responders.empty()) { | ||
| return {}; | ||
| } | ||
| return data().m_ocsp_responders[0]; |
There was a problem hiding this comment.
data().m_ocsp_responders.back() might be slightly better. See comment above.
Namely: * botan_x509_cert_view_binary_values * botan_x509_cert_view_string_values * botan_x509_crl_view_binary_values * botan_x509_crl_view_string_values This allows fetching standard information from X.509 objects such as the serial number of both a certificate and a CRL but also more specific information like OCSP responder URLs. Some getter types may have more than one value. For such cases, the getters have an index parameter to enumerate all elements.
These functions are currently implemented by trivially counting the number of elements their associated enumerator functions are actually producing. Given that most values are anyway viewed straight from the C++ objects' memory, the overhead should be negligible. The main advantage of this approach is its minimal maintenance burden.
atreiber94
force-pushed
the
feature/multiple_ocsp_responders
branch
from
80da263 to
1f55d01
Compare
|
Re-based onto the latest changes of #5188 which should hopefully turn the CI green. |
atreiber94
force-pushed
the
feature/multiple_ocsp_responders
branch
from
1f55d01 to
d4f21e0
Compare
Before, only a single OCSP responder URI was parsed and exposed via AIA extension and Botan::X509_Certificate. These now handle multiple OCSP responder URIs as per RFC 5280. Add corresponding test cases. Deprecate the single OCSP responder constructors and accessors. Note that the support for multiple OCSP responders is not propagated to `Botan::OCSP` methods yet.
atreiber94
force-pushed
the
feature/multiple_ocsp_responders
branch
from
d4f21e0 to
40273b6
Compare
4 participants
This addresses one of the limitations hindering integration of Botan's X509 parsing to strongSwan (see #5219), namely multiple OCSP responder URIs.
Before,
ocsp_responder()of the AuthorityInformationAccess extension or ofX509_Certificatewould return a single URI, even if the certificate parsed contains multiple URIs as OCSP responders.The single responder interfaces have been deprecated (e.g. it is not clear what "a OCSP responder" means in the now-deprecated methods - in my tests, it would return the last URI). The legacy interface is kept by returning the first URI (or the empty string if there are no URIs).
PR Dependencies
This is based on #5188 so I could test the FFI integration (also directly against the strongSwan X509 test suite).
OCSP
check_onlineThe changes here only expose multiple OCSP responder URIs to certificate parsing as well as the constructors/accessors of the AuthorityInformationAccess extension and
X509_Certificate. I did not include this into the OCSP features likeOCSP::check_onlinesince this code does not appear to be well tested (we should rather discuss what to do with this code going forward).