PKCS#11 URI query attribute parsing broken

[lkundrak]

According to the grammar on page 5 of RFC 7512, the p11-value attribute is a query attribute, not a path attribute.

These are correct:

pkcs11:token=test-token;object=client;type=private?pin-value=123456
pkcs11:token=test-token;object=client;type=private?vendor-attribute=666&pin-value=123456

This is not (well, technically it might be considered a vendor specific attribute...):

pkcs11:token=test-token;object=client;type=private;pin-value=123456

Therefore, the URI parser should look for it in the query part. Moreover, it should preferrably reject it in the path, as unlike the path part of the URI it's potentially sensitive information and applications may be assuming that it's okay to handle the path as non-sensitive (e.g. print it into log files). Not sure how feasible would that be though, since it will probably break user configurations.

[mtrojnar]

@dwmw2 What do you think?
The RFC 7512 parser was your contribution: OpenSC/engine_pkcs11@853a087...

[mouse07410]

Hmm... I for one see no problem with this:

pkcs11:token=test-token;object=client;type=private;pin-value=123456

and don't see why it should stop working.

[mtrojnar]

We don't have to (and probably we should not) detect invalid URIs. We just need to accept valid ones. Accepting "?" and "&" as alternative separators besides ";" should do the job.
Does it make sense?

[mouse07410]

Makes perfect sense, thank you!

[dwmw2]

By this point I'm quite keen on ditching my own implementation of the parser altogether, and just using libp11-kit instead...

[lo1ol]

Sorry for too late comment. I think it's was not a good decicion to accept ";?&" as common seporators.

The problem may become when pk11-pattr attributes contains "&" inside values. For example, take a "pkcs11:token=token&serial=12345678?pin-value=1234567" URI.

1. RFC parser parses for this:
   token="token&serial=12345678"
   pin-value="1234567"
2. Your parser parses for this:
   token="token"
   serial="12345678"
   pin-value="1234567"

So, what do your think about changing your parser to the right RFC version in future?

[dwmw2]

I still think we should ditch it and use the p11-kit one.

In fact since OpenSSL has deprecated ENGINEs entirely, I think there's a lot of scope for building a new OpenSSL 'provider' around p11-kit directly.