On Mon, Mar 06, 2023 at 04:35:05PM +0100, Theo Buehler wrote:
> > 3) Signatures (outside the TBS) in a .cer must be RSA (TODO: also
> > check mod + (e))
>
> I'd prefer to skip this for now. This does not really buy us much, it
> is independent and I see it as some polish that doesn't need to go in
> at the same time.
OK, let's approach that part separately as per below.
> Some comments/questions about this inline
>
> RFC 7935 explicitly allows NID_rsaEncryption. I seem to recall it was
> an issue in cms.c. Why is not an issue here?
I think there is a potential for confusion in that RFC 7935
differentiates between locations: 'in the certificate' and 'CMS SignedData'.
In the CMS context both rsaEncryption or sha256WithRSAEncryption can
appear (and both *do* appear in the wild). This is why we have to that
that 'nid is NID_rsaEncryption or NID_sha256WithRSAEncryption' check in
cms.c line 202.
However, RFC 7935 section 2 fourth paragraph starting with "In
certificates, CRLs, ..." mandates that sha256WithRSAEncryption is used:
"The Object Identifier (OID) sha256WithRSAEncryption from [RFC4055] MUST
be used in these products."
I take that to mean that the algorithmIdentifier OID (outside the TBS)
on .cer and .crl files MUST be sha256WithRSAEncryption; it seems all
deployed RPKI CAs concluded the same.
The below changeset adds a check to rpki-client to ensure that arbitrary
.cer and .crl files were signed with the RFC-mandated
sha256WithRSAEncryption algorithm.
OK?
Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.103
diff -u -p -r1.103 cert.c
--- cert.c 6 Mar 2023 16:04:52 -0000 1.103
+++ cert.c 6 Mar 2023 16:28:41 -0000
@@ -647,9 +647,12 @@ cert_parse_pre(const char *fn, const uns
size_t i;
X509 *x = NULL;
X509_EXTENSION *ext = NULL;
+ const X509_ALGOR *palg;
+ const ASN1_OBJECT *cobj;
ASN1_OBJECT *obj;
EVP_PKEY *pkey;
struct parse p;
+ int nid;
/* just fail for empty buffers, the warning was printed elsewhere */
if (der == NULL)
@@ -673,6 +676,20 @@ cert_parse_pre(const char *fn, const uns
/* Cache X509v3 extensions, see X509_check_ca(3). */
if (X509_check_purpose(x, -1, -1) <= 0) {
cryptowarnx("%s: could not cache X509v3 extensions", p.fn);
+ goto out;
+ }
+
+ /* RFC 7935 section 2 */
+ X509_get0_signature(NULL, &palg, x);
+ if (palg == NULL) {
+ cryptowarnx("%s: X509_get0_signature", p.fn);
+ goto out;
+ }
+ X509_ALGOR_get0(&cobj, NULL, NULL, palg);
+ if ((nid = OBJ_obj2nid(cobj)) != NID_sha256WithRSAEncryption) {
+ warnx("%s: RFC 6488: wrong signature algorithm %s, want %s",
+ fn, OBJ_nid2ln(nid),
+ OBJ_nid2ln(NID_sha256WithRSAEncryption));
goto out;
}
Index: crl.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/crl.c,v
retrieving revision 1.22
diff -u -p -r1.22 crl.c
--- crl.c 21 Feb 2023 10:18:47 -0000 1.22
+++ crl.c 6 Mar 2023 16:28:41 -0000
@@ -20,6 +20,8 @@
#include <string.h>
#include <unistd.h>
+#include <openssl/x509.h>
+
#include "extern.h"
struct crl *
@@ -27,8 +29,10 @@ crl_parse(const char *fn, const unsigned
{
const unsigned char *oder;
struct crl *crl;
+ const X509_ALGOR *palg;
+ const ASN1_OBJECT *cobj;
const ASN1_TIME *at;
- int rc = 0;
+ int nid, rc = 0;
/* just fail for empty buffers, the warning was printed elsewhere */
if (der == NULL)
@@ -44,6 +48,19 @@ crl_parse(const char *fn, const unsigned
}
if (der != oder + len) {
warnx("%s: %td bytes trailing garbage", fn, oder + len - der);
+ goto out;
+ }
+
+ X509_CRL_get0_signature(crl->x509_crl, NULL, &palg);
+ if (palg == NULL) {
+ cryptowarnx("%s: X509_CRL_get0_signature", fn);
+ goto out;
+ }
+ X509_ALGOR_get0(&cobj, NULL, NULL, palg);
+ if ((nid = OBJ_obj2nid(cobj)) != NID_sha256WithRSAEncryption) {
+ warnx("%s: RFC 6488: wrong signature algorithm %s, want %s",
+ fn, OBJ_nid2ln(nid),
+ OBJ_nid2ln(NID_sha256WithRSAEncryption));
goto out;
}
