Dns filtering#5
Conversation
| dnsServerIp: string, | ||
| readonly databaseManager: DatabaseManager, | ||
| readonly logger: NetworkEventLogger<DnsEvent> | ||
| ) { |
There was a problem hiding this comment.
keep private fields that don't need to be access from another class
| ) { | ||
| this.resolver.setServers([dnsServerIp]); | ||
| } | ||
| async logQuery({ name, type, query: { _client: { address } } }: Request, { |
There was a problem hiding this comment.
This should not be accessed directly here: the object DnsMessenger should completely isolate the business logic here from the underlying library structures.
Instead, modify DnsMessage to have clientIp part of the "request" event
| try { | ||
| switch (type) { | ||
| case "A": | ||
| await this.handleARecord(request, name); |
There was a problem hiding this comment.
Since the filter is just on the name and not on the type of the request, you can filter all requests (move the check before the switch)
| this.dnsMessenger.sendSrvRecords(request, (await this.resolver.resolveSrv(name))); | ||
| break; | ||
| case "PTR": | ||
| this.dnsMessenger.sendPtrRecords(request, (await this.resolver.resolvePtr(name))); |
There was a problem hiding this comment.
Better to keep the Proxy logic separate from the filter. Create a new class DNSFilter that takes a generic DNSRequestHandler as an argument.
This will make everything more simple and easy to modify later.
| const isBlocked = await this.checkBlocked(request); | ||
| if (isBlocked) { | ||
| throw 'blocked' | ||
| } |
There was a problem hiding this comment.
Avoid using exception to handle business logic: exception execution is slower than normal code and it is harder to follow the code
| <Iterate array={logs.sort((loga, logb) => logb.timestamp - loga.timestamp)}>{ | ||
| (log) => | ||
| <If condition={log.device_id === id && log.service === 'DNS'}> | ||
| <If key={log?.timestamp} condition={log.device_id === id && log.service === 'DNS'}> |
There was a problem hiding this comment.
Use _id for the key, you are sure this one is unique. timestamp is not guarantied to be unique
| <TableCell>{log?.event?.type}</TableCell> | ||
| <TableCell>{log?.event?.name}</TableCell> | ||
| <TableCell sx={this.highlighRed(log.event)}>{dayjs(log?.timestamp).format('YYYY-MM-DD hh:mm:ss a')}</TableCell> | ||
| <TableCell sx={this.highlighRed(log.event)}>{log?.event?.type}</TableCell> |
| <TableCell>{dayjs(log?.timestamp).format('YYYY-MM-DD hh:mm:ss a')}</TableCell> | ||
| <TableCell>{log?.event?.type}</TableCell> | ||
| <TableCell>{log?.event?.name}</TableCell> | ||
| <TableCell sx={this.highlighRed(log.event)}>{dayjs(log?.timestamp).format('YYYY-MM-DD hh:mm:ss a')}</TableCell> |
There was a problem hiding this comment.
Isn't color inherented?
You can just set it on TableRow instead of on every individual field
| type State = {}; | ||
|
|
||
| export default class DnsLogs extends Component<Props, State> { | ||
| highlighRed(event: DnsEvent): { color?: string } { |
| highlighRed(event: DnsEvent): { color?: string } { | ||
| return event?.blocked ? {color: 'red'} : {} | ||
| } | ||
| getStatus(event: DnsEvent): string { |
2 participants
No description provided.