search

krakenjs / passport-saml-encrypted

A strategy for Passport authentication that supports encrypted SAML responses
MIT License
14 stars 26 forks source link

Update the saml id length to match the spec #9

Closed wendlinga closed 9 years ago

wendlinga commented 9 years ago

Update generateUniqueID to conform to the spec length. Spec here https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf

1.3.4 ID and ID Reference Values This requirement MAY be met by encoding a randomly chosen value between 128 and 160 bits in length.

lmarkus commented 9 years ago

Hey, sorry I was on vacation away from electronics. So by my math, 8 bits = 1 character. 128 bits = 16 characters. 160 bits = 20 characters.

If I understood your PR correctly, we'd be overshooting the requirement, since you aim to go between 32 and 41 chars in length.

Could you clarify your intent?

wendlinga commented 9 years ago

No problem. My understanding is that since the characters are hex values, then each character only represents 4 bits.

var chars = "abcdef0123456789";

Based on that premise then 4 bits = 1 character. 128 bits = 32 characters. 160 bits = 40 characters.

lmarkus commented 9 years ago

I see your point. I don't think it's entirely accurate though. You certainly can represent an integer as 8 hex digits, 0xFFFFFFFF, but I don't think that the generated output of this function will be parsed as a number.

I think the string f will be intepreted as 01000110 (ascii 102), rather than 1111 (hex 0xF)

What do you think?

wendlinga commented 9 years ago

I agree that they will likely be stored as a string (and as such 8 bits), but the spec references the

probability of two randomly chosen identifiers being identical

Even if the value is stored as 8 bits, each position is only capable of representing 4 bits of information (0-F) It seems that the spec is concerned with the total number of possible combinations (2^128 to 2^160) instead of the underlying structure. Do you think this matches the documentations intent?

lmarkus commented 9 years ago

Good point.

lmarkus commented 9 years ago

Tagged, bumped and published. Thanks!