Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix 543: improve COSE_Key spec language and add COSE_Key examples #732

Merged
merged 10 commits into from
Jan 12, 2018

Conversation

equalsJeffH
Copy link
Contributor

@equalsJeffH equalsJeffH commented Dec 22, 2017

this builds on PR #731 to improve COSE_Key spec language and add COSE_Key examples

fix #543


Preview | Diff

Copy link
Contributor

@akshayku akshayku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

index.bs Outdated
-2: e ; e: RSA public exponent e byte string 3 bytes in length
e.g., in hex: 010001
}
</pre>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One example:

{1: 3, 3: -257, -1: h'ea9fbf0b9766e8e7412f86adeab7af8e11dd01308c1b71ac7c037f98a796b98f54253cf8b83f72b656340895608752af185e560695da06a77a655760ab28df3d44bfa417fe28978bfe34f1c93ad44508f4e6c56a4840f09aaf89481faa22098b41bb49082faad0ba94ffb529cd95cec0a56f3eebee9ff0f9cd95b22c48ef4854ce0f719cb06e76ba746e4129fbcfcb3f80b44bb2edd3908f468991a5249a1b8d9234316f245b1289f95d778a461733543612e3705c5ec2b78a9674305ca9fa464c3e95b0b67f058ff0cc6a84b0890551c413f1557a2a198c6d2629ade0f16ef9aec06ae89162f15a98ae49ce8b0ce8e07bafe1bd3b728bd1fc18f07e87a425f1', -2: h'010001'}

@equalsJeffH
Copy link
Contributor Author

the build errors just now were both "disk quota exceeded":
[0Koci runtime error: exec failed: container_linux.go:265: starting container process caused "could not create session key: disk quota exceeded"
[I built the spec successfully locally using cloud bikeshed]

@jcjones -- help?

Copy link
Member

@emlun emlun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not very familiar with these formats and encodings, so I'll abstain from reviewing.

index.bs Outdated
{
1: 3, ; kty: RSA key type
3:-37, ; alg: RS256
-1: n, ; n: RSA modulus n byte string 257 bytes in length
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this value (and below) be a 256 byte value? (There's no zero padding to avoid negative numbers in COSE.)

Copy link
Contributor Author

@equalsJeffH equalsJeffH Jan 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx hm. I dunno, you guys tell me. Here's how I generated the example RSA key pair, and openssl sez the modulus is 257 bytes:

 bash-3.2$ openssl genrsa -des3 -out private.pem 2048
Generating RSA private key, 2048 bit long modulus
.......................+++
..........................................................................................................+++
e is 65537 (0x10001)
Enter pass phrase for private.pem:
Verifying - Enter pass phrase for private.pem:
bash-3.2$ 
bash-3.2$ 
bash-3.2$ openssl rsa -in private.pem -outform PEM -pubout -out public.pem
Enter pass phrase for private.pem:
writing RSA key
bash-3.2$ 
bash-3.2$ 
bash-3.2$ PUBKEY=`grep -v -- ----- public.pem | tr -d '\n'`
bash-3.2$ 
bash-3.2$ 
bash-3.2$ echo $PUBKEY | base64 -D | openssl asn1parse -inform DER -i
    0:d=0  hl=4 l= 290 cons: SEQUENCE          
    4:d=1  hl=2 l=  13 cons:  SEQUENCE          
    6:d=2  hl=2 l=   9 prim:   OBJECT            :rsaEncryption
   17:d=2  hl=2 l=   0 prim:   NULL              
   19:d=1  hl=4 l= 271 prim:  BIT STRING        
bash-3.2$ 
bash-3.2$ 
bash-3.2$ 
bash-3.2$ echo $PUBKEY | base64 -D | openssl asn1parse -inform DER -i -strparse 19
    0:d=0  hl=4 l= 266 cons: SEQUENCE          
    4:d=1  hl=4 l= 257 prim:  INTEGER           :DB5F651550E3E99916CEE05C15D9322AD5AA6FF068FC63943A598F860635F413E84DC33CF72DCCE21FAE9FF9ED9DCA0EAD57DFF01921EF4DD1ADE1C22232D94BDA40DB66EF35855DC241D64496C70503FB4FE4CCAC05395DD9040E5C8168186FBFB61B3158BBBC8C7CD0F4309D8944C620522F5648AD0EDF544D318DD882BDE80CB1FE5F0C3924CD5F95616E66CA2B79D79C28B1192A01AA0A6D5614BF754D3085CEBE50F684AC2EDFD52B37BBB3740650EC4A736DD90F1455526AF3D8B3FA535795DB1A40175EF23E559DE7B0357CB050889C554BE149AF254DF147352276BD5F0453FAFCFEFD5AE019F69DFEB53AB2DD1063C4C4E4A6D8AD5B16DC6548ACC3
  265:d=1  hl=2 l=   3 prim:  INTEGER           :010001

is there any zero-padding in the above modulus value? Ah, is the leading : padding (that COSE_Key encoding omits)? Or is the : an artifact of ASN.1 encoding? I checked the modulus length and it is 256 bytes without the :. But the length of the exponent is given as 3 bytes and it is shown with the : byte/char, so I'm not sure what is going on.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2048 bits / 8 = 256, so we would expect a 256-byte value.

ASN.1 DER requires that an INTEGER with the most-significant bit set be padded with a leading zero to avoid it appearing to be negative. Since the leading byte is 0xdb, that will have happened in this case. In fact, it'll happen for all RSA keys since the MSB is required to be 1 (otherwise it would be a 2047-bit key).

COSE, however, doesn't have this leading padding byte. That is made explicit here:

The octet sequence MUST utilize the minimum number of octets needed to represent the value.

Copy link
Contributor Author

@equalsJeffH equalsJeffH Jan 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thx, so is openssl just not displaying a leading zero padding byte? ( I think yes )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, as it's not considered part of the value. If you base64-decode the public key and look for the modulus in a hex dump, it should be preceded by 0282010100.

index.bs Outdated
<pre class="example" highlight="json">
{
1: 3, ; kty: RSA key type
3:-37, ; alg: RS256
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: PS256, rather than RS256, I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, fixed, thx.

index.bs Outdated
<pre class="example" highlight="json">
{
1: 3, ; kty: RSA key type
3:-257, ; alg: RS256
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is algorithm -257 defined for COSE? I don't see it in the IANA registry. (In fact, I don't see PKCS #1 v1.5 at all.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's registered in WebAuthn, actually: https://w3c.github.io/webauthn/#sctn-cose-alg-reg

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we reach CR, I plan to submit the registration request to IANA.

@agl
Copy link
Contributor

agl commented Jan 4, 2018

(I see that I commented on the wrong subchange, but I think my point about s/257/256/ is still valid even if GitHub thinks it's outdated.)

@equalsJeffH
Copy link
Contributor Author

@agl

ok, you are referring to #732 (comment), i believe.

I think I understand, tho I remain unsure whether the correct modulus length is 256 or 257 bytes, please see: #732 (comment), thx.

@equalsJeffH
Copy link
Contributor Author

Ready to merge?

index.bs Outdated
@@ -2528,7 +2528,7 @@ to be used with the RSASSA-PSS w/ SHA-256 signature algorithm

Below is an example of a COSE_Key-encoded 2048-bit RSA public key (see [[RFC8230]]
[Section 4](https://tools.ietf.org/html/rfc8230#section-4))
to be used with the RSASSA-PKCS1-v1_5 w/ SHA-256 signature algorithm (see [[#sctn-cose-alg-reg]]):
to be used with the RS256 signature algorithm (RSASSA-PKCS1-v1_5 w/ SHA-256, see [[#sctn-cose-alg-reg]]):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Maybe replace "w/" with "with". Your call.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

index.bs Outdated
{
1: 3, ; kty: RSA key type
3:-257, ; alg: RS256
-1: n, ; n: RSA modulus n byte string 257 bytes in length
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think one extra space is needed after the colon here and below. (And then the continued comment lines would need an extra space too.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

index.bs Outdated
}
</pre>

Below is an example of a COSE_Key-encoded 2048-bit RSA public key (see [[RFC8230]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Could also just say that it's the same key because it's also RSA. Your call.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@equalsJeffH equalsJeffH merged commit 58e824a into master Jan 12, 2018
WebAuthnBot pushed a commit that referenced this pull request Jan 12, 2018
@emlun emlun deleted the jeffh-fix-543-COSE_KEY-examples branch June 12, 2019 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

musings wrt webauthn's profile of COSE_Key
6 participants