-
Notifications
You must be signed in to change notification settings - Fork 41
Arbitrary alphabet and radix support #2
Comments
Two questions:
Thanks a lot for making this project public! |
|
It would be great if the change wouldn't require using the latest Go version (specially using tip), but have that as a planned migration. What I mean is, add it as a subpackage, so this can be used soon, and from the start making it clear that once Go updates |
Agreed - I'd prefer it to not be dependent on the Go version. I like the migration option long term, but as it is that's dependent on upstream Go. I tested the base62 support ( #1 ) in a branch by copying the standard Further, I think for now what I will do is use only the parts of |
@fmpwizard discussion is here: golang/go#21558 |
I see that the linked ticket was closed, but they didn't go with the change that would allow arbitrary alphabets, right? |
@fmpwizard that is correct, they added base62 support which basically fixes #1 , which is nice; but not full arbitrary alphabets. The Go team's comments on the linked issue explains the reasoning behind that. I do still want to add arbitrary alphabet support, especially seeing your use case above. I may just have to add separate If you have any thoughts on how to do this effectively, please let me know! |
@anitgandhi thanks for following up on this topic. I can more or less follow the ideas around implementing this, but not enough to propose an actual changeset, let alone doing it effectively. |
friendly ping |
Sadly I haven't had the bandwidth to work on this issue 😢 As a note to my future self (or anyone who wants to take this up), implementing this will have to limit to ASCII strings so the |
I was looking at implementing this but had a question:
I pulled the go repo, checked out go1.4.3 and I didn't find any |
Whoops, it was called That code already has a check for effectively ensuring ASCII/base256 which should get the job done. |
Thanks! I'll take a look and post back with results |
any update on this |
I think I finally figure out what I'm supposed to do here, should have some time to work on this over the weekend |
I went the path of copying the
Is the license issue something that capitalone's lawyers may bring up or are you worried about the golang side? if the golang side, I can ask on the mailing list, you'll know best from the capitalone side. From my reading of the math/big code, it looks pretty well optimized already or at least I don't think I could come up with something better than what's there already (where I could say, this is my original code) |
In hindsight, the licensing aspect is probably more trivial as compared to the issue of maintaining the separate copy from the upstream. Personally I don't like it when things get out of sync like that. What about a character substitution model? I suppose it would still be limited to base62 but a utility that takes an input and output alphabet, and just does a straight character substitution could be a first step. |
the out of sync is a real issue, I was worried about how to make sure we would port any bug fixes, etc, it's possible, but at times may not be the best way to use time. The character substitution sounds interesting. With the base62 model, we can now support
in my particular use case, I can limit it to either all upper or lowercase letter, plus 0-9 and then ideally these:
if I urlencode the string I get from my source, mixed with only really using 0-9 and A-B, base62 would be enough I think Is this what you had in mind? |
Also iirc in the 1.9 release, math/big started using some math/bits which I think uses internal/cpu . Might be wrong on that tho And yes thats exactly what I was thinking. For every n characters you don't use out of base62, you get a different n "special" chars. Which I suppose doesn't even have to be in this package since it's just a character map |
Another take on this is not get involved in the alphabet at all. Instead, define fpe encrypt as a function that takes an array of uint16 - so supporting radix 1..65536. Instead of using Int.SetString() to build the binary string, use the NUMradix(X) function defined in https://nvlpubs.nist.gov/nistpubs/specialpublications/nist.sp.800-38g.pdf Something like:
This example also calculates max which is radix to power of len(s), for use later in encrypt to compute the modulus. A corresponding function is required to convert the encryption result back to an array of uint16. The NIST paper defines a STRradix function to do that. With these encryption interfaces in place, it is trivial to convert strings in fairly large alphabets (65536 members) into arrays of uint16 and back again. I'll prepare a PR when I get time for you look at. |
@martinwaite interesting, that looks like a good way to solve the problem. I'd be happy to review and merge a PR whenever. Since your solution still uses the Really my only conditions for the PR is there isn't a significant performance regression, but we can tackle that as issues come up 👍 |
Description of Enhancement
Expanding on #1 , in theory, this package should support arbitrary alphabets as well. It will require either major changes to
math/big
or a new sub-package.This may or may not have to include non-ASCII characters.
Further, there is a chance it will require a breaking change in the interface, if this alphabet input is added to
NewCipher
. Alternatively, a new function onCipher
calledSetAlphabet
could be added.The text was updated successfully, but these errors were encountered: