Skip to content

Normalize malformed nested key errors - #79

Open
OskarEichler wants to merge 2 commits into
cedarcode:masterfrom
OskarEichler:codex/malformed-key-errors
Open

OskarEichler wants to merge 2 commits into
cedarcode:masterfrom
OskarEichler:codex/malformed-key-errors

Conversation

@OskarEichler

Copy link
Copy Markdown

Loads URI explicitly and converts deeply nested CBOR stack failures into COSE::MalformedKeyError alongside the other malformed-input exceptions.

Verified with 86 upstream examples plus focused malformed-key, WebAuthn, and Rails integration models.

@santiagorodriguez96 santiagorodriguez96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this. I think this PR fixes two separate bugs – I've written them up as #82 and #83.

I know there's no coverage of this rescue today – but would it be possible to add a spec for this CBOR::StackError?

Comment thread lib/cose/key.rb
def self.cbor_decode(data)
CBOR.decode(data)
rescue CBOR::MalformedFormatError, EOFError, FloatDomainError, RegexpError, TypeError, URI::InvalidURIError
rescue CBOR::MalformedFormatError, CBOR::StackError, EOFError, FloatDomainError, RegexpError, TypeError,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
rescue CBOR::MalformedFormatError, CBOR::StackError, EOFError, FloatDomainError, RegexpError, TypeError,
rescue CBOR::UnpackError, EOFError, FloatDomainError, RegexpError, TypeError,

https://github.com/cabo/cbor-ruby/blob/89e94f3b059deb58478eab51996f764fcc6ecd82/doclib/cbor/error.rb#L3-L10

Comment thread lib/cose/key.rb
require "cose/key/rsa"
require "cose/key/symmetric"
require "openssl"
require "uri"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, worth calling out: this does more than load a constant. The cbor gem only calls URI.parse for CBOR tag 32 if URI is already defined, so this require turns tag-32 parsing on process-wide, which changes how CBOR data decodes everywhere in the app – not just inside cose-ruby. Two things change for any CBOR.decode call:

  • tag-32 values come back as URI objects instead of CBOR::Tagged
  • input that decoded fine before can now raise URI::InvalidURIError
require "cbor"
bytes = CBOR::Tagged.new(32, "https://example.com").to_cbor
CBOR.decode(bytes).class  # => CBOR::Tagged
require "uri"
CBOR.decode(bytes).class  # => URI::HTTPS

Low impact in practice, since Rails and webauthn-ruby load uri anyway. But I think it needs a CHANGELOG entry, and it makes this a minor bump rather than a patch.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CBOR::StackError is not rescued by COSE::Key.cbor_decode COSE::KEY.cbor_decode raises NameError when unlisted error is raised and uri is not required

2 participants