[9front] letsencrypt vs rsa2csr

[9front] letsencrypt vs rsa2csr

[ori]

Earlier today, sirjofri reported a bug with
acmed:

	sirjofri | ori: resp={
	sirjofri | "type": "urn:ietf:params:acme:error:malformed",
	sirjofri | "detail": "Error parsing certificate request:
		asn1: structure error: tags don't match (0 vs {class:2 tag:0
		length:0 isCompound:false}) {optional:false explicit:false
		application:false private:false defaultValue:\u003cnil\u003e
		tag:0xc0026c5548 stringType:0 timeType:0 set:false
		omitEmpty:false} @323"

After some investigation into what was different between us
and openssl, it looks like openssl was setting empty containers
as constructed, but we werent:

	$ diff -u ok.dump sad.dump                                                                                                       
	--- ok.dumpSun Aug  8 08:21:57 2021
	+++ sad.dumpSun Aug  8 08:27:49 2021
	@@ -5,13 +5,13 @@
	    13:d=3  hl=2 l=  23 cons:    SET               
	    15:d=4  hl=2 l=  21 cons:     SEQUENCE          
	    17:d=5  hl=2 l=   3 prim:      OBJECT            :commonName
	-   22:d=5  hl=2 l=  14 prim:      UTF8STRING        :eigenstate.org
	+   22:d=5  hl=2 l=  14 prim:      PRINTABLESTRING   :eigenstate.org
	    38:d=2  hl=4 l= 290 cons:   SEQUENCE          
	    42:d=3  hl=2 l=  13 cons:    SEQUENCE          
	    44:d=4  hl=2 l=   9 prim:     OBJECT            :rsaEncryption
	    55:d=4  hl=2 l=   0 prim:     NULL              
	    57:d=3  hl=4 l= 271 prim:    BIT STRING        
	-  332:d=2  hl=2 l=   0 cons:   cont [ 0 ]        
	+  332:d=2  hl=2 l=   0 prim:   cont [ 0 ]        
	   334:d=1  hl=2 l=  13 cons:  SEQUENCE          
	   336:d=2  hl=2 l=   9 prim:   OBJECT            :sha256WithRSAEncryption
	   347:d=2  hl=2 l=   0 prim:   NULL              
	
Here's a patch that changes this. I'm not
a native asn.1 speaker, so I'm not entirely
sure that this is correct. Do we only want
to do this for some containers, or do we want
it for all of them?

--- //.git/fs/object/3909b83a90ff0c820ef7c903a03fc12b043ebfea/tree/sys/src/libsec/port/x509.c
+++ sys/src/libsec/port/x509.c
@@ -1025,8 +1025,8 @@
 			el = e.val.u.setval;
 		else
 			err = ASN_EINVAL;
+		*pconstr = CONSTR_MASK;
 		if(el != nil) {
-			*pconstr = CONSTR_MASK;
 			for(; el != nil; el = el->tl) {
 				err = enc(&p, el->hd, lenonly);
 				if(err != ASN_OK)

Re: [9front] letsencrypt vs rsa2csr

[Michael Forney]

On 2021-08-08, ori@eigenstate.org <ori@eigenstate.org> wrote:
> Here's a patch that changes this. I'm not
> a native asn.1 speaker, so I'm not entirely
> sure that this is correct.

I think it is correct, it doesn't really make sense to have a
primitive set/sequence.

The basic encoding rules (of which DER is a subset) in X.690 say[0]:

8.9.1 The encoding of a sequence value shall be constructed.
8.10.1 The encoding of a sequence-of value shall be constructed.
8.11.1 The encoding of a set value shall be constructed.
8.12.1 The encoding of a set-of value shall be constructed.

> //.git/fs/object/3909b83a90ff0c820ef7c903a03fc12b043ebfea/tree/sys/src/libsec/port/x509.c
> +++ sys/src/libsec/port/x509.c
> @@ -1025,8 +1025,8 @@
>  			el = e.val.u.setval;
>  		else
>  			err = ASN_EINVAL;
> +		*pconstr = CONSTR_MASK;
>  		if(el != nil) {
> -			*pconstr = CONSTR_MASK;
>  			for(; el != nil; el = el->tl) {
>  				err = enc(&p, el->hd, lenonly);
>  				if(err != ASN_OK)

You could go a step further and remove the if-statement as well. It is
already taken care of by the initial for-loop condition.

[0] https://www.itu.int/rec/T-REC-X.690-202102-I/en

Re: [9front] letsencrypt vs rsa2csr

[ori]

Quoth Michael Forney <mforney@mforney.org>:
> 
> You could go a step further and remove the if-statement as well. It is
> already taken care of by the initial for-loop condition.

Good point. Updated.

--- //.git/fs/object/3909b83a90ff0c820ef7c903a03fc12b043ebfea/tree/sys/src/libsec/port/x509.c
+++ sys/src/libsec/port/x509.c
@@ -1025,13 +1025,11 @@
 			el = e.val.u.setval;
 		else
 			err = ASN_EINVAL;
-		if(el != nil) {
-			*pconstr = CONSTR_MASK;
-			for(; el != nil; el = el->tl) {
-				err = enc(&p, el->hd, lenonly);
-				if(err != ASN_OK)
-					break;
-			}
+		*pconstr = CONSTR_MASK;
+		for(; el != nil; el = el->tl) {
+			err = enc(&p, el->hd, lenonly);
+			if(err != ASN_OK)
+				break;
 		}
 		break;