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

Add ability to mark webirc users and forbid marked users #320

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

skizzerz
Copy link
Contributor

Marked connections (via set_mark in the webirc auth block) will be
checked against the forbid_mark flag in the final auth block after
resolving the user's spoofed IP. This allows for additional levels of
security from webirc operators so that they are unable to spoof their
way into privileged auth blocks -- an auth block with forbid_mark will
reject any connections from marked users.

The mark is not automatic because some webirc servers may be fully
trusted to allow privileged users to connect (such as a webchat hosted
by the network itself).

Marked connections (via set_mark in the webirc auth block) will be
checked against the forbid_mark flag in the final auth block after
resolving the user's spoofed IP. This allows for additional levels of
security from webirc operators so that they are unable to spoof their
way into privileged auth blocks -- an auth block with forbid_mark will
reject any connections from marked users.

The mark is not automatic because some webirc servers may be fully
trusted to allow privileged users to connect (such as a webchat hosted
by the network itself).
@edk0 edk0 self-requested a review March 29, 2022 15:51
Copy link
Contributor

@edk0 edk0 left a comment

Choose a reason for hiding this comment

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

I don't feel unambiguously great about reusing the flag bit, but I think it's probably better than the alternative.

@@ -325,6 +325,14 @@ verify_access(struct Client *client_p, const char *username)
return (NOT_AUTHORISED);
}

if(IsMarked(client_p))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if(IsMarked(client_p))
if (IsMarked(client_p))

Copy link
Contributor Author

@skizzerz skizzerz Apr 1, 2022

Choose a reason for hiding this comment

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

I just stuck with the same formatting as all of the other (existing) ifs in the file (which lacked spaces before the parenthesis). Should I change all of them?

Copy link
Contributor

Choose a reason for hiding this comment

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

change as many as you can be bothered to without feeling excessive, I guess. I don't always change surrounding ones (though I do think it's good to); I just figure if we stop introducing new if( eventually it'll be gone.

ircd/s_conf.c Show resolved Hide resolved
Copy link
Contributor

@edk0 edk0 left a comment

Choose a reason for hiding this comment

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

also I clicked the wrong button oops

@jillest
Copy link
Contributor

jillest commented Apr 2, 2022

I would prefer adding a new flag bit; reusing an existing one seems rather cryptic and there are plenty of bits left.

The flags in the configuration could perhaps be named more clearly as well, for example "unprivileged_webirc" and "forbid_unprivileged_webirc" (or "untrusted" instead of "unprivileged").

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.

3 participants