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.

Sign up for GitHub

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

Jump to bottom

paserk value for k3.secret-wrap.pie-fail-1 is not valid RFC 4648 Base64 #18

Open
bannable opened this issue Nov 25, 2022 · 2 comments
Open

paserk value for k3.secret-wrap.pie-fail-1 is not valid RFC 4648 Base64 #18

bannable opened this issue Nov 25, 2022 · 2 comments

Comments

@bannable
Copy link

While the value can be parsed under RFC 2045, it is problematic when using a stricter RFC 4648 coder, such as Ruby's urlsafe_decode64 or strict_decode64, as the test vector cannot exercise the intended code path for an invalid authentication tag.

This appears to be because, when the test vector was written, the value was created simply by modifying the encoded Base64 value instead of modifying the underlying payload and re-encoding.

Even with an RFC 2045 coder, the final 8 bytes are dropped compared to k3.secret-wrap.pie-2 which it appears to be modified from.

Reproducer (in ruby)

val = '_9Npd2vzpHCHdGYDQ5QFMc6Nirlv3ubsPPb-xJnh6cLImLhpirH_PDQWRIkmb95m-fc1XpqwY2QQbUkkvb2-_tznGQBt0Yg2-0Lux2MNxObkxDIfxfz5lD_tToF58BIuTh9_Pny0dy27HmJPnUBgy7JP2Je-h4doq6i0V2Q5hRB'

puts Base64.decode64(val).bytesize
puts Base64.urlsafe_decode64(val).bytesize
puts Base64.urlsafe_decode64(val).unpack1('H*')

Expected

128
128
[some series of hex digits]

Actual

120
<internal:pack>:309:in `unpack1': invalid base64 (ArgumentError)

My suggestion is to change the value to a valid RFC 4648 string. For example, this is the value from k3.secret-wrap.pie-2 with the first byte of the tag replaced by \x00, and exercises the intended code path:

_9Npd2vzpHCHdGYDQ5QFMc6Nirlv3ubsPPb-xJnh6cLImLhpirH_PDQWRIkmb95m-fc1XpqwY2QQbUkkvb2-_tznGQBt0Yg2-0Lux2MNxObkxDIfxfz5lD_tToF58BIuTh9_Pny0dy27HmJPnUBgy7JP2Je-h3doq6i0V2Q5hRA
@bannable
Copy link
Author

It looks like k4.secret-wrap.pie-fail-1 has this same problem.

bannable added a commit to bannable/paseto that referenced this issue Nov 26, 2022
@bannable
Add test vectors for local-wrap and secret-wrap
ef77549 
I've modified the test vector for k3.secret-wrap.pie-fail-1 because,
in the course of implementing these vectors, I discovered that the
paserk value is not valid Base64.

Reported here: paseto-standard/paserk#18

In the mean time, I've replaced the badly encoded value with an encoded
value that replaces the first byte of the tag with \x00
@bannable
Copy link
Author

k3.secret-pw-fail-2 and k4.secret-pw-fail-2 have this same problem.

Suggested alternatives:

# k3.secret-pw-fail-2
AKjP_XFziwwUW8t0xppL3ecIgaOzfSEx-5-UQG36jJ8AACcQUeD46ydUwIkMOqkXWFvacyf_eaH1BTMlJsdCy6ZhemmaFMZTclOD9LrOwCVnmhlCDQEePilxQEfvPsRM5cL_yxx1bWL0wjS4GAQABQiCvGyQTi_LGlbMnYuiZfxWgpqNJpAI6jx71m6s4f6wZIg68Q

# k4.secret-pw-fail-2
AH54ImYiJUSDhTn7vzBI4QAAAAAQAAAAAAAAAwAAAAFBZythnpR02Zza64_y9DuKHeyZVEP_vZx0Y721aIry1rZc70cR08Jb2rgV4pcqR9in25TvA4pV7L4kT3r-0b-5a8Z7wk35D0zOnPLEJloAHf2XEYGleFReV2-tiV1T79G6OhlATgd-bJbXjRqlEOCsk_-pRdSsCeE

bannable added a commit to bannable/paseto that referenced this issue Nov 29, 2022
@bannable
Add support for local-pw and secret-pw
5c2543a 
- Versions have been refactored to be classes instead of modules, and
  things that are versioned are expected to respond to a .protocol method
  that returns an instance of a Interface::Version class. An instance of
  such a class is comparable with other instances of the same, and also
  with any string that matches the associated version, e.g. "v3".

- Enabled sorbet final checks and added final sigs to several classes

- IncorrectKeyType is gone in favor of LucidityError. A LucidityError is
  any violation of Algorithm Lucidity properties.

- Replaced most string concatenation operations with interpolation for
  improved performance

- Added local-pw and secret-pw support for both v3 and v4

- Made PIE version detection more robust against absense of RbNaCl

- PIE::Version3 -> PIE::PieV3, PIE::Version4 -> PIE::PieV4

- Util.secure_compare now uses libsodium primitives when available for
  significantly improved performance.

- Fixed pattern matching order in Token parsing to have the more specific
  case first

I'm not entirely happy with the interface for accessing PBKW at this point,
because it requires explicitly initializing with a version, but it works
well enough.

The tests for PBKW v4 are SLOW as hell due to the test vectors including
parameters with very high memlimit and opslimit values. I'm leaving them
enabled for now, but if it eats up too much CI time, they'll be changed
to run only on demand.

The test vectors have been modified as in ef77549
due to more buggy encoding. See: paseto-standard/paserk#18
@bannable bannable mentioned this issue Dec 2, 2022
Open
bannable added a commit to bannable/paseto that referenced this issue Dec 3, 2022
@bannable
Add test vectors for local-wrap and secret-wrap
a288af8 
I've modified the test vector for k3.secret-wrap.pie-fail-1 because,
in the course of implementing these vectors, I discovered that the
paserk value is not valid Base64.

Reported here: paseto-standard/paserk#18

In the mean time, I've replaced the badly encoded value with an encoded
value that replaces the first byte of the tag with \x00
bannable added a commit to bannable/paseto that referenced this issue Dec 3, 2022
@bannable
Add support for local-pw and secret-pw
4e70206 
- Versions have been refactored to be classes instead of modules, and
  things that are versioned are expected to respond to a .protocol method
  that returns an instance of a Interface::Version class. An instance of
  such a class is comparable with other instances of the same, and also
  with any string that matches the associated version, e.g. "v3".

- Enabled sorbet final checks and added final sigs to several classes

- IncorrectKeyType is gone in favor of LucidityError. A LucidityError is
  any violation of Algorithm Lucidity properties.

- Replaced most string concatenation operations with interpolation for
  improved performance

- Added local-pw and secret-pw support for both v3 and v4

- Made PIE version detection more robust against absense of RbNaCl

- PIE::Version3 -> PIE::PieV3, PIE::Version4 -> PIE::PieV4

- Util.secure_compare now uses libsodium primitives when available for
  significantly improved performance.

- Fixed pattern matching order in Token parsing to have the more specific
  case first

I'm not entirely happy with the interface for accessing PBKW at this point,
because it requires explicitly initializing with a version, but it works
well enough.

The tests for PBKW v4 are SLOW as hell due to the test vectors including
parameters with very high memlimit and opslimit values. I'm leaving them
enabled for now, but if it eats up too much CI time, they'll be changed
to run only on demand.

The test vectors have been modified as in ef77549
due to more buggy encoding. See: paseto-standard/paserk#18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@bannable