This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 26465 - Algorithm normalization doesn't allow arbitrary operations for AlgorithmIdentifier fields
Summary: Algorithm normalization doesn't allow arbitrary operations for AlgorithmIdent...
Status: RESOLVED FIXED
Alias: None
Product: Web Cryptography
Classification: Unclassified
Component: Web Cryptography API Document (show other bugs)
Version: unspecified
Hardware: PC All
: P2 normal
Target Milestone: ---
Assignee: Mark Watson
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-30 01:23 UTC by Mark Watson
Modified: 2014-09-26 18:21 UTC (History)
1 user (show)

See Also:


Attachments

Description Mark Watson 2014-07-30 01:23:32 UTC
Our algorithm normalization rules assume that the target Dictionary type is dependent on the ( algorithm name, operation ) pair.

Algorithm normalization is invoked recursively for member entries of AlgorithmIdentifier type. The name to be used is just the value specified in the input data, but the operation is "hard-coded" in the specification as follows:
- if the type is HashAlgorithmIdentifier, then "digest"
- otherwise, use the same value as the outer normalization

This seems restrictive. 

I suggest that the "leaf" values in the [[supportedAlgorithms]] structure be changed from just a type to a ( type, DOMString? ) pair, with the rule that the DOMString must be non-null if and only if the type is or is derived from AlgorithmIdentifier. Then this would be the operation used for recursive invokation of algorithm normalization.

The 'define an algorithm' procedure and the invokations of it would need amending as well.
Comment 1 Ryan Sleevi 2014-07-30 01:44:35 UTC
Mark,

I've read through this bug several times, and your other comment, and it's still entirely opaque to me what you see the problem as, nor what you're proposing as the solution.

Your comment about (type, DOMString?) makes me think you're wanting to introduce a new interface, which surely doesn't sound right. The nullability of DOMString? alone raises concerns.

Perhaps you could try restating the problem as you see it.

If you're objecting to the specialization of HashAlgorithmIdentifier, well, another solution is to simply allow digest algorithms to be used for all operations, since they are 'leaves' (if I understand what you're referring to) in nearly all supported operations.

Still, clarity desired.
Comment 2 Mark Watson 2014-07-30 02:36:12 UTC
(In reply to Ryan Sleevi from comment #1)
> Mark,
> 
> I've read through this bug several times, and your other comment, and it's
> still entirely opaque to me what you see the problem as, nor what you're
> proposing as the solution.

I thought what I said was pretty clear. I'll write a reply to my own post with additional clarification.

> 
> Your comment about (type, DOMString?) makes me think you're wanting to
> introduce a new interface, which surely doesn't sound right. The nullability
> of DOMString? alone raises concerns.

No, I didn't say anything about an interface.

> 
> Perhaps you could try restating the problem as you see it.
> 
> If you're objecting to the specialization of HashAlgorithmIdentifier, well,
> another solution is to simply allow digest algorithms to be used for all
> operations, since they are 'leaves' (if I understand what you're referring
> to) in nearly all supported operations.
> 
> Still, clarity desired.
Comment 3 Mark Watson 2014-07-30 02:56:04 UTC
Ok, I am missing a layer. We need to be a able to associate operations with member values *inside* the target dictionary type, not with the target dictionary type itself.


(In reply to Mark Watson from comment #0)
> Our algorithm normalization rules assume that the target Dictionary type is
> dependent on the ( algorithm name, operation ) pair.

In the algorithm normalization rules, we start with the [[supportedAlgorithms]] associative container, which maps from the operation to an aliasesAndAlgorithms associative container which contains an "algorithms" entry which is an associative container, which maps from algorithm names to a single leaf value which is an IDL Dictionary type.

Aliases aside, this maps from a pair ( algorithm name, operation ) to an IDL Dictionary type which is the target type for the normalized algorithmIdentifier value.

> 
> Algorithm normalization is invoked recursively for member entries of
> AlgorithmIdentifier type.

If we find a member, X, of the target IDL type which has type AlgorithmIdentifier, we recursively invoke the algorithm normalization algorithm. The input to this is the algorithm identifier value itself (X) and an operation value.

> The name to be used is just the value specified in
> the input data, but the operation is "hard-coded" in the specification as
> follows:
> - if the type is HashAlgorithmIdentifier, then "digest"
> - otherwise, use the same value as the outer normalization

Step 12.5 in section 2.4.5 defines the rules for the operation value to be used. The only options are "digest" or the same as the original value.

So, for example, if I am performing a 'sign' operation then any parameters to this of type AlgorithmIdentifier (or subclass) are normalized with operation name 'sign' or 'digest' depending on whether the the type is a HashAlgorithmIdentifier or not.

> 
> This seems restrictive. 

Because I cannot have an member in an algorithm identifier that identifies an algorithm to be used with an operation other than 'digest' or the same as the outer algorithm identifier.

Suppose I have a new signature algorithm which must be parameterized by the identity and parameters of an encryption algorithm which is somehow used in the signature process.

Dictionary NewSignatureAlgorithm : AlgorithmIdentifier
{
   AlgorithmIdentifier enc;
}

example:

{ name : "NewSignature", enc: { name : "AES-GCM" } }

I need enc to be normalized with operation set to "encrypt", not "sign".

> 
> I suggest that the "leaf" values in the [[supportedAlgorithms]] structure be
> changed from just a type to a ( type, DOMString? ) pair,

Instead of the values of the associative container found at the "algorithms" member of the associative container at the "sign" member of [[supportedAlgorithms]] being just an IDL type, they would be a pair ( type, DOMString? ).

To make it more abstruse we could say this is another associative container with members "type" and "operation".

Here I made a mistake in the proposal, we need to associate operations with values *inside* this target type. Instead of a DOMString we could do one of the following:

- a container which mirrors the structure of the target type, specifying for each AlgorithmIdentifier member what the operation is to be used to normalize it (would need to support members which are themselves Dictionaries).
- a container which maps from sub-types of AlgorithmIdentifier to the operation name, so that extension specifications can define that NewHashAlgorithmIdentifier should use "digest" and EncryptAlgorithmIdentifier should use "encrypt" etc.

Of course this is not needed for any dictionary member which is not an AlgorithmIdentifier (which was the reason for the nullability before).

> with the rule that
> the DOMString must be non-null if and only if the type is or is derived from
> AlgorithmIdentifier.
>
> Then this would be the operation used for recursive
> invokation of algorithm normalization.
> 
> The 'define an algorithm' procedure and the invokations of it would need
> amending as well.
Comment 4 Ryan Sleevi 2014-07-31 18:06:50 UTC
I apologize, but I'm still missing something / still find your definition a bit vague, because it seems like you're mixing solution with problem.

(In reply to Mark Watson from comment #3)
> Because I cannot have an member in an algorithm identifier that identifies
> an algorithm to be used with an operation other than 'digest' or the same as
> the outer algorithm identifier.

To confirm, your problem is simply that the normalized algorithm of an AlgorithmIdentifier is constrained to the operation type of the parent.

Can you describe an actual algorithm or problem with this? At least with our other API concerns, there are real and practical algorithms that demonstrate this issue. This seems a bit even more... impractical?

> - a container which mirrors the structure of the target type, specifying for
> each AlgorithmIdentifier member what the operation is to be used to
> normalize it (would need to support members which are themselves
> Dictionaries).

Apologies, but I still cannot make heads nor tails of what you're proposing. It sounds, which surely can't be correct, that you're proposing duplicating the interface definitions.

> - a container which maps from sub-types of AlgorithmIdentifier to the
> operation name, so that extension specifications can define that
> NewHashAlgorithmIdentifier should use "digest" and
> EncryptAlgorithmIdentifier should use "encrypt" etc.

I'm not sure the value of "don't monkeypatch" is worth the value here, especially when there's no concrete, real-world use case, and it's just an academic concern.
Comment 5 Mark Watson 2014-07-31 19:13:02 UTC
(In reply to Ryan Sleevi from comment #4)
> I apologize, but I'm still missing something / still find your definition a
> bit vague, because it seems like you're mixing solution with problem.
> 
> (In reply to Mark Watson from comment #3)
> > Because I cannot have an member in an algorithm identifier that identifies
> > an algorithm to be used with an operation other than 'digest' or the same as
> > the outer algorithm identifier.
> 
> To confirm, your problem is simply that the normalized algorithm of an
> AlgorithmIdentifier is constrained to the operation type of the parent.
> 
> Can you describe an actual algorithm or problem with this? At least with our
> other API concerns, there are real and practical algorithms that demonstrate
> this issue. This seems a bit even more... impractical?

I don't have a non-hypothetical example. But we are talking about extensibility here, so we should allow for all kinds of extension (within reason). Unless there is some reason to believe that this form of extension is unlikely to be needed.

My hypothetical example was a new signature algorithm which needs to be parameterized by a choice of encryption algorithm.

If one algorithm makes internal use of another algorithm like this, there is no reason to expect the inner operation to match the outer one. Indeed, the existing concrete example is of signature algorithms which internally make use of hash algorithms, where the inner operation (digest) does not match the outer one (sign).

> 
> > - a container which mirrors the structure of the target type, specifying for
> > each AlgorithmIdentifier member what the operation is to be used to
> > normalize it (would need to support members which are themselves
> > Dictionaries).
> 
> Apologies, but I still cannot make heads nor tails of what you're proposing.

I think you need to read what I said more carefully, then.

> It sounds, which surely can't be correct, that you're proposing duplicating
> the interface definitions.

Indeed, that's not correct. What on earth in what I said makes you think that ?

Suppose the IDL Dictionary for my hypothetical signature algorithm is as in my example:

Dictionary NewSignatureAlgorithm : AlgorithmIdentifier
{
   AlgorithmIdentifier enc;
}

and suppose I have a value, again as in my example:

{ name : "NewSignature", enc: { name : "AES-GCM" } }

and furthermore, [[supportedAlgorithms]] is equal in part to:

{ "sign" : { "aliases" : ...,
             "algorithms" : { "NewSignature" : NewSignatureAlgorithm } } }

Now, in following the normalization rules, I will look up "sign" and then "NewSignature" in [[supporedAlgorithms]] and come up with a 'desiredType' of NewSignatureAlgorithm (step 8 in 20.4.5)

Then I will perform the IDL conversion to that desiredType. Finally, I'll traverse NewSignatureAlgorithm and discover that the member "enc" has type AlgorithmIdentifier. In step 12.5 in 20.4.5 the third choice is triggered and I will: "Set the dictionary member on normalizedAlgorithm with key name key (="enc") to the result of normalizing an algorithm, with the alg set to idlValue (={ name : "AES-GCM" }) and the op set to op (="sign").

So, we are constrained to normalizing the "enc" member using operation "sign".

If we want to allow the flexibility for the "enc" member to be normalized using operation "encrypt", then instead if just looking up 'desiredType' in [[supportedAlgorithms]] we would want also to be provided with an object like this:

{ "enc" : "encrypt" }

which specifies the operations to be used for the AlgorithmIdentifier members of desiredType. 

> 
> > - a container which maps from sub-types of AlgorithmIdentifier to the
> > operation name, so that extension specifications can define that
> > NewHashAlgorithmIdentifier should use "digest" and
> > EncryptAlgorithmIdentifier should use "encrypt" etc.
> 
> I'm not sure the value of "don't monkeypatch" is worth the value here,
> especially when there's no concrete, real-world use case, and it's just an
> academic concern.

Well, as I said, the constraint that the inner operation must be the same as the outer one makes no sense, especially as in the only concrete example ("sign" vs "digest") they don't match.

If you want to allow future flexibility without going as far as specifying the inner algorithm normalization rules then in that third option of step 12.5 of 20.4.5 we can just say:

"Set the dictionary member on normalizedAlgorithm with key name key to the result of normalizing an algorithm, with the alg set to idlValue and the op set to the value specified in the specification of algName."
Comment 6 Ryan Sleevi 2014-07-31 20:24:53 UTC
(In reply to Mark Watson from comment #5)
> I don't have a non-hypothetical example. But we are talking about
> extensibility here, so we should allow for all kinds of extension (within
> reason). Unless there is some reason to believe that this form of extension
> is unlikely to be needed.

Which I believe, and am having trouble seeing this as "within reason", thus trying to come up with a more useful scheme.

> If we want to allow the flexibility for the "enc" member to be normalized
> using operation "encrypt", then instead if just looking up 'desiredType' in
> [[supportedAlgorithms]] we would want also to be provided with an object
> like this:
> 
> { "enc" : "encrypt" }

I specifically avoided this because it de-generates into the same problem as wrap/unwrap with key-specified attributes (the PKCS#11 problem with CKA_WRAP_TEMPLATE/CKA_UNWRAP_TEMPLATE) in that we need to support infinite layers of recursion, such that

{ "enc" : { "param1": "encrypt", "param2": { "subparam": "encrypt", "anothersubparam": "sign" } } }

I agree that there is a hypothetical use case, but to me, the practical existence of such a hypothetical strikes at a strong design smell. That is, I think there are going to be far more issues with such a hypothetical "NewSignatureAlgorithm", and I don't think supporting a non-monkeypatched approach with algorithm identification are going to solve those issues.

On a more practical concern, since we're dealing with hypotheticals, I will note that none of the other platform APIs need to deal with / treat this problem. Considering the lead time for any sort of successful peer review on the suitability of NewSignatureAlgorithm, I'm also not convinced that we should try to solve this problem now.
Comment 7 Mark Watson 2014-08-01 15:53:47 UTC
Ok, so at least we are clear on the problem.

I think that a future algorithm for operation X which needs to be parameterized by with an algorithm for operation Y, with Y != X is far far more likely than the case where Y == X.

Can you give me any examples of an algorithm where Y == X ? A signature algorithm which needs to be parameterized with another signature algorithm ? An encryption algorithm which needs to be parameterized with another encryption algorithm ?

If we are not going to solve this problem now (which I agree is an option), we should remove the recursive normalization except for the explicit digest case, because there are even fewer potential applications for that. It's also confusing because there is no rationale for supporting that case and not the more general one.

I agree with your point about the complexity of defining a full solution in this specification. The alternative is to defer to the extension specification, when/if it happens, by simply saying that the operation to be used will be defined in the specification of the new algorithm.
Comment 8 Mark Watson 2014-08-26 00:54:01 UTC
So, just to be clearer what I am now proposing.

In section 20.4.5. "Normalizing an algorithm", second sub-clause ("If alg is an IDL object") step 12.5, third sub-option (" If member is of the type AlgorithmIdentifier")

REPLACE:

Set the dictionary member on normalizedAlgorithm with key name key to the result of normalizing an algorithm, with the alg set to idlValue and the op set to op.

WITH:

Set the dictionary member on normalizedAlgorithm with key name key to the result of normalizing an algorithm, with the alg set to idlValue and the op set to the operation specified by the specification that definines algorithm.
Comment 9 Mark Watson 2014-09-22 18:04:44 UTC
Any objections to my proposal in Comment #8 ?