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.

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

Review SessionID Security #4

Closed
helje5 opened this issue Jun 29, 2019 · 2 comments
Closed

Review SessionID Security #4

helje5 opened this issue Jun 29, 2019 · 2 comments
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@helje5
Copy link
Member

helje5 commented Jun 29, 2019

The way session IDs are generated is probably not status quo. Security experts please ;-)

@helje5 helje5 added help wanted Extra attention is needed question Further information is requested labels Jun 29, 2019
@mattt
Copy link

mattt commented Jun 30, 2019

I'm not a security expert, but reading the current implementation, I think the small number of random bits (2~10 for 1337 + 264 for random Int) is problematic; the addition of a timestamp doesn't really do much to mitigate an attack, and MD5 isn't cryptographically secure.

Probably the simplest replacement with reasonable security is to use Foundation UUID, whose values are generated on macOS and Linux from 122 random bits read from /dev/urandom (+6 fixed bits for UUIDv4 format) [1], [2], [3].

Here's what this would look like:

import struct Foundation.UUID

extension NIOHostingSession {
  
  // MARK: - Session ID
    
  static func createSessionID() -> String {
    return UUID().uuidString
  }
}

However, something to consider: According to this article, a motivated attacker may compromise 122 random bits in under an hour, so UUID isn't suitable for most production environments. As an alternative, the article suggests Base64-encoding 160 random bits. And I'm pretty sure you could append / prepend a fixed-format timestamp, atomic counter, and / or namespace if you wanted to be able to determine the age or provenance of a given session ID without compromising security.

I'm not familiar with the current best-practices for cross-platform generation of random data in Swift, but I'd be happy to investigate and submit a PR if that's something you'd be interested in.

helje5 added a commit that referenced this issue Jul 4, 2019
@helje5
Copy link
Member Author

helje5 commented Jul 4, 2019

OK, did the UUID thing, much better, thanks.

I'm still interested in the current best practices, but only mildly :-) So for now I keep that open just in case someone else has extra suggestions.

BTW: We could also couple a session to a WebSocket connection, i.e. use no session-id at all. Though that would forbid reconnects, though we could use rolling reconnect-ids for that. I suppose.

@helje5 helje5 closed this as completed Jul 8, 2019
helje5 added a commit to Macro-swift/MacroExpress that referenced this issue Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants