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

Signature format needs to be defined #799

Closed
yackermann opened this issue Feb 13, 2018 · 35 comments
Closed

Signature format needs to be defined #799

yackermann opened this issue Feb 13, 2018 · 35 comments

Comments

@yackermann
Copy link
Contributor

So currently we are only telling that you need to produce signature, but what format to provide it in we don't

Proposing adding requirement that signature must be DER encoded

@agl
Copy link
Contributor

agl commented Feb 13, 2018

The signature format is defined by the algorithm of a key, which is specified in the “alg” parameter during registration. So, for example, the newly registered RSA algorithms won't have a DER signature, but rather a big-endian RSA group element because that's what 8017 says.

(Although it is worth noting that ES256 is defined here as best I can tell and it says:

The signature is encoded by converting the integers into byte strings of the same length as the key size. The length is rounded up to the nearest byte and is left padded with zero bits to get to the correct length. The two integers are then concatenated together to form a byte string that is the resulting signature.

However, both Chrome and Firefox actually return a standard RFC 3279 ECDSA signature in this case.)

@yackermann
Copy link
Contributor Author

@agl Good point. Thanks for clarification

@equalsJeffH
Copy link
Contributor

equalsJeffH commented Feb 14, 2018

re-opened given discussion on webauthn call today Wed 14-Feb-2018
see also: https://lists.w3.org/Archives/Public/public-webauthn/2018Feb/0373.html

@samuelweiler samuelweiler added this to the CR milestone Feb 14, 2018
@akshayku
Copy link
Contributor

Microsoft has also implemented same as Firefox and Chrome which is same for CTAP1 and CTAP2 devices. No change required. Closing this.

@apowers313
Copy link
Contributor

Just for the sake of clarity, "the same as FF and Chrome" is ASN.1 / DER encoding the signature?

@agl
Copy link
Contributor

agl commented Feb 15, 2018

(I'm going to double-check that Chrome and Firefox are using ASN.1 DER (i.e. RFC 3279) encoding today.)

@apowers313
Copy link
Contributor

@agl if I understand your comments above ANS.1 DER encoding would be different than what is currently specified (i.e. - concatenating bytes together), is that right? Should we change the spec to reflect how people are actually encoding the signature field?

@akshayku
Copy link
Contributor

We should put some examples for clarity.

@agl
Copy link
Contributor

agl commented Feb 15, 2018

I'm not sure that I'm following the specifications correctly, but my best estimate is that the "correct" behaviour is to follow the COSE spec and just concatenate r & s. However, if all three current implementations independently got it "wrong", I consider that strong evidence that we should specify what is clearly the more obvious behaviour.

@apowers313
Copy link
Contributor

Maybe @jimsch could clarify that our reading of the COSE spec is correct?

@akshayku
Copy link
Contributor

Sample Ecc Signature:

"sig": h'3045022100d43908065193a1bf82a46e5db5e216fcfcb238a98d5ba00f62d59d45d3e46b4602203330d72eaea09786687e1e1b4875df94d1d430ec0998316320fda27d3a38a897'

@agl which particular line/section has this confusion in WebAuthN Spec.

@akshayku akshayku reopened this Feb 15, 2018
@selfissued
Copy link
Contributor

I agree with @apowers313 that the specification should unambiguously write down what we want the format to be, regardless of what we decide. Otherwise, the spec doesn't contain enough information to be implemented interoperably.

@akshayku
Copy link
Contributor

Reopening it till we get this corrected..

@akshayku
Copy link
Contributor

akshayku commented Feb 15, 2018

I agree with @agl here. Authenticators are generating (RFC 3279/RFC 8017) signatures and these needs to be reflected as it is to the RPs. There is no translation needed between standard obvious signatures schemes and COSE.

I will open a PR in few hours for correction.

@jimsch
Copy link

jimsch commented Feb 15, 2018

The COSE signature format is the same as the DER format sans the ASN.1

@agl
Copy link
Contributor

agl commented Feb 15, 2018

@akshayku's "sig" value, above is certainly DER encoded. For other browsers I'm testing the signature member of AuthenticatorAssertionResponse:

  1. From Firefox Nightly (build 20180215103933): DER (304502203da08…)
  2. From Chrome built this week: DER (3045022100e99cf4f33b092…)

The reason that I concluded that concatenating r & s might be "correct" is that, when creating a public key, the type of the key is specified as a COSEAlgorithmIdentifier. Specifically, I'm using -7 for ES256. The registry references RFC 8152 for this which says to concatenate to serialise a signature.

@selfissued
Copy link
Contributor

We should not include any ASN.1 prefixes in the signature values. If our implementations need to strip prefixes that the functions we are calling include, we should do that.

@agl
Copy link
Contributor

agl commented Feb 15, 2018

@herrjemand The context is that I'm squinting at the docs and wondering whether signatures are supposed to be COSE formatted, but we find that everyone has implemented ASN.1 DER for ECDSA so we should probably clarify the spec and make that official.

@yackermann
Copy link
Contributor Author

@agl Why do browsers re-encode signatures?

@yackermann
Copy link
Contributor Author

@agl Or is this only apply to CTAP1?

@akshayku
Copy link
Contributor

CTAP1 produces ASN1 DER signatures. So should CTAP2 for consistency. And it matches how we encode information in X5C certificates. As everyone has implemented the same, we are making this clarification in the spec.

@yackermann
Copy link
Contributor Author

@akshayku okay, cool

@yackermann
Copy link
Contributor Author

@akshayku so this applies to CTAP2 level? Not to webauthn?

@akshayku
Copy link
Contributor

Browsers are not re-encoding signatures. All CTAP2 authenticators are producing signatures as mentioned in RFC 3279/ RFC 8017. WebAuthN spec is not clear which one it is and we are clarifying it.

This applies to webauthn as CTAP2 authenticators already produces signatures as specified in RFC 3279/ RFC 8017.

@yackermann
Copy link
Contributor Author

yackermann commented Feb 15, 2018

@akshayku yes, you are correct. Sorry, I was a little bit confused *)

@selfissued
Copy link
Contributor

@akshayku - saying "RFC 8017 signatures" is ambiguous. For instance, look at https://tools.ietf.org/html/rfc8017#appendix-A.2.4 (RSASSA-PKCS-v1_5). It contains definitions for both DigestInfo, which includes an ASN.1 OID for the digest algorithm, and for "digest", which doesn't. Here's a snippet:

   DigestInfo ::= SEQUENCE {
       digestAlgorithm DigestAlgorithm,
       digest OCTET STRING
   }

It's my position that what we're calling "signature" should be the digest only and not require implementations to prefix the raw signature with ASN.1 information. If some implementations are doing that, that seems unnecessarily complicated.

@akshayku
Copy link
Contributor

So Repo is locked. @nadalin, Can you open repo for the clarification.?

@equalsJeffH
Copy link
Contributor

@nadalin: this is a technical issue.

@selfissued wrote:

look at https://tools.ietf.org/html/rfc8017#appendix-A.2.4 (RSASSA-PKCS-v1_5). It contains definitions for both DigestInfo, which includes an ASN.1 OID for the digest algorithm, and for "digest", which doesn't.

AFAICT, https://tools.ietf.org/html/rfc8017#appendix-A.2.4 is describing only the object identifier for RSASSA-PKCS-v1_5, and the digest is actually a hash value, not a resulting signature per se. E.g., see the definition for EMSA-PKCS1-v1_5 wherein DigestInfo is utilized.

WRT the actual signature values produced by the two "Signature Generation Operation"s given in RFC8017 (RSASSA-PSS-SIGN, RSASSA-PKCS1-V1_5-SIGN) , it appears their output is specified as a "plain" (aka "raw") octet string (i.e., not an ASN.1 OCTET STRING):

Output:

      S        signature, an octet string of length k, where k is the
               length in octets of the RSA modulus n

In the case of ECDSA in RFC8152, it appears the signature output is also a "plain" (aka "raw") octet string (as @agl effectively notes above):

Using the function [I2OSP] defined in [RFC8017], the signature is:
   Signature = I2OSP(R, n) | I2OSP(S, n)
   where n = ceiling(key_length / 8)

To note:

U2F clearly defines that the signature value "..is encoded in ANSI X9.62 format..." This aligns with RFC3279.

webauthn states only that: "A COSEAlgorithmIdentifier's value is a number identifying a cryptographic algorithm. The algorithm identifiers SHOULD be values registered in the IANA COSE Algorithms registry [IANA-COSE-ALGS-REG]..." These appear to produce "plain" (aka "raw") octet string signature values, as @agl notes above, and as @jimsh notes above.

In any case, webauthn ought to clarify the signature value encoding format in the case of specific signature algorithms. We may wish to also specify a set of recommended signature algs rather than leaving it open to any/all of the sig algs listed in the IANA COSE Algorithms registry, such that RPs have a constrained set of sig algs to implement.

For example, see how we did this in Token Binding Protocol.

@selfissued
Copy link
Contributor

It seems to me like the most straightforward thing to do is to say that the signature contains the same representation of raw bytes as would be used in the corresponding COSE signature for the corresponding algorithm. That way we don't have to define these for all algorithms - only possibly for new ones that we introduce ourselves.

The COSE signatures are all raw byte arrays and as Jim Schaad said, at least for ECDSA, correspond to the DER representation within the ASN.1.

@selfissued
Copy link
Contributor

P.S. We should include an example or two, or at least say things like what the byte lengths of signature values for common algorithms are. For instance, for ES256 the signature length is exactly 64 bytes. For RS256 with a 2048 bit key the signature length is exactly 256 bytes. Etc.

@akshayku
Copy link
Contributor

In spirit for keeping consistency with CTAP1/U2F authenticators, using exising crypto libraries developed over many years in the industry, and not breaking all the implementations done by everyone in FIDO group, its will be much better if we follow the existing standard RFCs.

@equalsJeffH Thanks for Token Binding link. That really helps :)

I added some examples I could find.

@akshayku
Copy link
Contributor

@agl , @equalsJeffH , @selfissued Please review.

@jyasskin
Copy link
Member

Just to be painfully clear, DER encodings of integers are never fixed-length (section 8.3.2 of https://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf), and https://tools.ietf.org/html/rfc3279#section-2.2.3 uses integers, while the COSE representations are fixed-length ("left padded with zero bits to get to the correct length").

@jimsch
Copy link

jimsch commented Feb 16, 2018

Not to mention that integers in DER are going to be extended by a byte if the leading byte of the integer has the high bit set.

@equalsJeffH
Copy link
Contributor

overall disclaimer: I do not have a personal dog in this race.

that said, there are trade-offs here worth noting...

its will be much better if we follow the existing standard RFCs

RFC8152 and RFC8230 are "existing standard RFCs". It seems that the issue you folks are having is that your existing toolset (libs etc) does not (as yet) directly support emitting signature values per the latter RFCs.

Yet, there are some advantages to the COSE-specified sig values. Meanwhile, it is debatable whether ASN.1 wrapping offers any intrinsic benefits.

So, just to note, this is yet another instance of ossification: pragmatic adherence to the status quo in the short term, but perhaps not advantageous in the long term.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants